Events/CDC (TRUNK-6429/6515/6516) sign-off: 13 verified findings across core + artemis/camel/debezium

[dkayiwa]

Went through the events/CDC work (core 2.9.x and master, plus artemis/camel/debezium). I confirmed each item below by running it on a 2.9.x install against MySQL, or against the real classes, jars, and bytecode; the file:line references are from the code. Each item notes how I confirmed it.

Confirmed

1. Outbox events are never delivered when the @OutboxEventListener lives in a module, which is the normal case. On a running server I registered a module @OutboxEventListener, fired a matching event, and the outbox_event row was written but stayed PENDING with the listener never called; bc2739ac-5bf3-4ecd-8e11-d383d53695bb was absent from jobrunr_recurring_jobs. In the same run OutboxEventRegistry.hasOutboxListeners() returned true (so the registry does see the module listener), and when I force-scheduled the poller myself the very same row was delivered and marked COMPLETED. So the poller logic is fine; OutboxTaskSchedulerInitializer just makes the schedule-or-not decision once at core startup, before modules register listeners, and never revisits it. Same code on master.

2. The outbox poller can't load module-defined event types. Same run: a module-defined OutboxableEvent was persisted to the outbox, then the poller's Class.forName(item.getEventType()) (OutboxPollingTaskHandler.java:101; line 107 on master) threw java.lang.ClassNotFoundException, the row's error_count kept climbing, and the listener was never called. Use OpenmrsClassLoader/Context.loadClass. (This is what PR TRUNK-6429: core Events/CDC sign-off fixes (#6289) #6296 changes.)

3. CDCEvent on 2.9.x cannot be serialized by the outbox/broker mapper. I serialized the real org.openmrs.event.CDCEvent with the real JacksonConfig.objectMapper() and it threw InvalidDefinitionException: Direct self-reference leading to cycle (... CDCEvent["resolvableType"] -> ResolvableType["componentType"] ...), because line 12 imports org.codehaus.jackson.annotate.JsonIgnore and FasterXML ignores it. The sibling SaveServiceEvent (same getResolvableType() getter, FasterXML @JsonIgnore) serialized fine in the same test. master already uses the FasterXML import.

4. outbox_event.payload is TEXT (64 KB) though the field is @Lob, and ordinary saves overflow it. The column is text(65535), sql_mode is STRICT_TRANS_TABLES. This is not just a large-payload edge case: a routine savePerson (one Person, a couple of names) rolled back on me with MysqlDataTruncation: Data too long for column 'payload', because the serialized SaveServiceEvent<Person> measured about 101 KB. The size comes from JacksonConfig enabling Hibernate5Module.FORCE_LAZY_LOADING, so serializing the event eagerly pulls the whole reachable graph (person -> creator -> roles -> privileges -> ...). Since that insert runs synchronously inside the business transaction (the same publish path that failed a save in item 7), the clinical write rolls back. So with an outbox listener registered, common saves fail outright. Make payload (and completed_listeners) MEDIUMTEXT/LONGTEXT, and reconsider FORCE_LAZY_LOADING (it also means extra lazy-load queries per event). Same on master.

5. An Artemis @BrokerEventListener typed BrokerIncomingEvent<?> dead-letters every message. Publishing a String threw JsonParseException, a JSON object threw InvalidTypeIdException: missing '@class', both were redelivered to the DLQ, and the publisher saw success. A concrete BrokerIncomingEvent<Foo> worked (verified with a ProbeDto). Handle the Object/<?> case or reject it at registration.

6. Outgoing broker sends aren't transactional. I published a BrokerOutgoingEvent inside a transaction that then rolled back, and the consumer still received it, because handleEvent (ArtemisEventListener.java:172) is a plain @EventListener that sends synchronously. So a send can fire for a change that later rolls back, or be lost if the process dies before the send, and there's no retry. Decide whether this should be @OutboxEventListener or @TransactionalEventListener(AFTER_COMMIT).

7. A synchronous @EventListener that throws fails the service call that produced the event. A listener that threw (I called getId() on a GlobalProperty, which is unsupported) propagated out of the publish and failed the saveGlobalProperty that triggered it. The advice publishes before proceed() (OpenmrsServiceEventAdvice.java:109) with no isolation around listeners. Isolate listener exceptions or document the constraint.

8. Camel never gets the Elasticsearch client. CamelConfig.java:82 does .backend().unwrap(RestClient.class); against the Hibernate Search 6.2.4 bytecode, unwrap returns only for ElasticsearchBackend and otherwise throws, and the catch returns null. So on an Elasticsearch backend the route indexes nothing while looking healthy. Should be .unwrap(ElasticsearchBackend.class).client(RestClient.class) with a warning when the client is unavailable.

9. Camel hawtio hardening. The built omod bundles geronimo-jms_2.0_spec-1.0-alpha-2.jar (its own copy of javax.jms); CamelLoginModule.java:56 compares the console password with String.equals (not constant-time); and the console WAR is never re-extracted on upgrade (CamelConfig.java:152, if (!consoleWar.exists())).

10. Artemis wires broker listeners only once, so a consumer module started at runtime never receives. setupListeners is guarded by if (initialized) return;, so it builds JMS containers only from the listeners present at the first context refresh. I confirmed this with two consumers using the same concrete-typed @BrokerEventListener: one in the startup batch received its message, and one in a module I started at runtime with ModuleFactory.startModule received nothing (the message reached the broker with no consumer container, and its listener was never invoked). OpenMRS starts modules at runtime, so this is a real path.

11. Camel's jms component is left unwired by the camel-jms classloader split, so jms: routes can't work even when a broker is present. In the running server with artemis loaded, the jms component bean resolved to Spring's NullBean (that is, CamelConfig.jms() returned null) while two broker ConnectionFactory beans were visible through core's javax.jms interface. Camel's @Autowired ConnectionFactory binds the javax.jms.ConnectionFactory from the geronimo-jms jar bundled in the omod (item 9), which is a different class from the core javax.jms-api the broker factories implement, so nothing matched and the component came back null. Excluding the geronimo spec so the module shares core's javax.jms-api fixes it. (This also means the earlier "two ConnectionFactory beans cause NoUniqueBeanException" theory is wrong: Camel sees zero, not two, which is why boot doesn't crash.)

12. Debezium floods consumers with framework-table churn. The engine captures the whole database and table.exclude.list excludes only debezium's own three tables, so it emits CDCEvents for non-domain tables like jobrunr_jobs, jobrunr_backgroundjobservers, jobrunr_recurring_jobs, liquibasechangeloglock, and the outbox_event table itself. In one short run the CDCEvents broke down as 210 jobrunr_jobs, 37 outbox_event, 23 other jobrunr_*, 2 liquibasechangeloglock, and only 12 for the real domain change (global_property). So the large majority of the stream is framework churn with entityType=null that every @EventListener(CDCEvent) consumer has to handle, plus the outbox is re-captured as CDC. Exclude the infrastructure tables or document that consumers must filter.

13. A single poison event blocks the whole outbox until it exhausts the retry limit. The poller processes strictly in id order and, on any listener failure, resets the row to PENDING (until errorCount >= outboxevent.retry.limit, default 16) and throws, aborting the cycle. So one event that keeps failing (for example item 2's ClassNotFoundException, or a transient broker outage) blocks every later event for up to 16 retries before it goes FAILED and the queue drains. I watched this happen: the failing row climbed to error_count=16 and flipped to FAILED, and the six SaveServiceEvent rows queued behind it stayed PENDING at error_count=0 (never attempted) the whole time, then all drained to COMPLETED the moment the poison row went FAILED. Transient failures count toward the same limit, so a temporary downstream outage can also push an otherwise-good event to FAILED (then needing a manual retryFailedOutboxEvent).

@rkorytkowski flagging for you. The three per-module review issues (artemis#4, camel#4, debezium#1) are closed and folded in here; debezium is otherwise clean.

[Tarekchehahde]

This sign-off ticket is a cross-module review gate rather than a single code change. From the contributor side, the actionable core event-coverage instances called out in the related issues are being addressed separately:

• Order stop/discontinue does not fire a *ServiceEvent for the stopped order (event-driven consumers see it as still active) #6197 → TRUNK-6429: publish SaveServiceEvent when an order is stopped (#6197) #6293 (stopOrder publishes SaveServiceEvent)
• No dedicated patient-merge event; consumers must rely on the incidental SaveServiceEvent<PersonMergeLog> #6195 → TRUNK-6429: publish MergePatientsServiceEvent after patient merge (#6195) #6294 (MergePatientsServiceEvent)
• Non-proxied / non-pattern service mutators bypass #6084 *ServiceEvents (transferEncounter, stopVisits, removeAllergy, setAllergies, …) — systemic gap #6199 (stopVisits instance) → TRUNK-6429: publish SaveServiceEvent from stopVisits (#6199) #6295 (proxied saveVisit)

Remaining items in this review (Artemis outbox delivery, Camel POC wiring, Debezium notes, multi-node docs) look like maintainer/sign-off scope rather than a standalone external PR. Happy to help on any of those if maintainers want them split into trackable issues.

[Tarekchehahde]

Core sign-off PR for the items addressable in openmrs-core:

PR: #6296

• MEDIUMTEXT for outbox_event.payload / completed_listeners (Events/CDC (TRUNK-6429/6515/6516) sign-off: 13 verified findings across core + artemis/camel/debezium #6289 item 2)
• Context.loadClass in outbox poller (Events/CDC (TRUNK-6429/6515/6516) sign-off: 13 verified findings across core + artemis/camel/debezium #6289 item 5)
• Javadoc on typed vs DB event streams and pre-proceed() timing (Events/CDC (TRUNK-6429/6515/6516) sign-off: 13 verified findings across core + artemis/camel/debezium #6289 items 3–4)

Event-coverage instances (#6195/#6197/#6199): #6293, #6294, #6295.

Artemis/Camel/Debezium and deployment-doc items remain out of scope for this core PR.

[rkorytkowski]

Went through all 13 findings. PR #6298 addresses the ones that belong in openmrs-core; the rest are either in module repos, already fixed on master, documented as intentional, or out of scope for this repo.

#	Finding	Disposition
1	Outbox poller never starts when @OutboxEventListener is in a module	Fixed in #6298 — OutboxEventRegistry now triggers scheduling when it finds listeners (at startup or via new registerBean() for module runtime registration); OutboxTaskSchedulerInitializer no longer implements SmartInitializingSingleton
2	Outbox poller ClassNotFoundException for module-defined event types	Fixed in #6298 — Class.forName → Context.loadClass in OutboxPollingTaskHandler
3	CDCEvent wrong @JsonIgnore import causes serialization cycle	Already fixed on master (FasterXML import in place), Fixed in #6299 on 2.9.x
4	outbox_event.payload is TEXT (64 KB), overflows on ordinary saves	Fixed in #6298 — payload and completed_listeners changed to MEDIUMTEXT in update changelog, schema-only snapshot, and OutboxEvent entity
5	BrokerIncomingEvent<?> dead-letters every message	Fixed in #6298 — BrokerEventListenerFactory now fails fast at startup with a clear error when a wildcard/raw type is used
6	Outgoing broker sends not transactional	Expected and documented — see openmrs-module-artemis#6
7	Synchronous @EventListener that throws aborts the service call	Documented in #6298 — added Javadoc to OpenmrsServiceEventAdvice clarifying this is intentional (listener acts as precondition guard) and pointing to @TransactionalEventListener / @OutboxEventListener for non-blocking use
8	Camel wrong unwrap call — Elasticsearch client never wired	Camel module (openmrs-module-camel)
9	Camel hawtio: bundled geronimo jar, non-constant-time password, WAR not re-extracted on upgrade	Camel module (openmrs-module-camel)
10	Artemis wires broker listeners only once — runtime module consumers never receive	Artemis module (openmrs-module-artemis)
11	Camel jms component null due to classloader split with bundled geronimo jar	Camel module (openmrs-module-camel)
12	Debezium floods consumers with framework-table churn (jobrunr_*, outbox_event, etc.)	Debezium module (openmrs-module-debezium) — extend table.exclude.list
13	Single poison event blocks the whole outbox queue until retry limit exhausted	Expected by design — strict chronological ordering is intentional