ramu11 commented on PR #28730: URL: https://github.com/apache/camel/pull/28730#issuecomment-6574778831
> **TL;DR:** Thanks @ramu11, the asynchronous hand-off detection now covers `vm`, `disruptor`, `disruptor-vm` and `reactive-streams`, and the wireTap wait has a proportional margin, so I resolved gnodet-bot's two threads. Point 1 of my previous review is still open, and it blocks: a Kafka send after a `doCatch` (or after `onException(...).continued(true)`) is still treated as a dead letter send (`KafkaProducer.java:790-796`). `isFailureHandlerDelivery` is new code in this PR, so this needs a fix here rather than a follow-up JIRA. > > Checked on [4adad62](https://github.com/apache/camel/commit/4adad620d7c44c8ab68d69d9bc54595662bd1eb1): > > * The `ExchangeSendingEvent` notifier works when it is added to a running context: `addEventNotifier` starts it and sets `eventNotificationApplicable`, and `SendProcessor` reads that flag on every send (`SendProcessor.java:226`). `recordsAnEarlierAsynchronousSendAfterALaterSynchronousSend` covers this. > > New notes on this commit (not blocking): > > 1. **One exactly-once consumer turns on event notification for the whole CamelContext.** After `ensureAsynchronousHandoffTracking`, every `to()` in every route allocates a `StopWatch` and a new callback, and walks the notifier list (`SendProcessor.java:226-250`), also in routes that have nothing to do with Kafka. Please say this in the doc. If a cheaper hook fits, use it instead. For example, the check could run only for exchanges that carry `KafkaExactlyOnceMetadata` by returning early in `notify` before the URI check. That does not remove the per-send cost, but it keeps the property off unrelated exchanges. > 2. **The mark is set on the exchange that performs the send.** A `seda:` send from a copy (`split`, `multicast`, `enrich` with a copy, a `wireTap` copy) marks the copy, not the source exchange, so `hasAsynchronousHandoff` does not see it. Either document that limit or record the hand-off on the `KafkaExactlyOnceMetadata` that the copies share, as `hasAsynchronousKafkaHandoff` already does. > 3. **The notifier is never removed.** It stays registered after the Kafka route is removed or stopped. Removing it when the consumer stops would keep the context clean, together with the entry in the static map. > 4. **Docs:** `kafka-component.adoc:637-638` still only names `SEDA`. Please list the five schemes that are detected (gnodet-bot noted this too). > > Still open from my previous review: point 1 (`EXCEPTION_CAUGHT`, blocking), and point 2 (the reflection-based wireTap instrumentation and the blocking wait after the source transaction has committed). > > _Claude Code on behalf of Claus Ibsen. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying._ Thanks for the review. I’ve pushed the updates in commit **599871dfcf6e** addressing both blocking points: 1. **Failure-handler classification:** Removed the `EXCEPTION_CAUGHT` heuristic. Failure-handler delivery is now tracked using an explicit exchange property, set and cleared through the Dead Letter Channel lifecycle events. This prevents normal Kafka sends after `doCatch` or `continued(true)` from being incorrectly classified as DLQ delivery. 2. **Asynchronous WireTap handling:** Removed the reflection-based WireTap instrumentation and the blocking wait after source transaction completion. Kafka-directed WireTap targets are now rejected during route startup. Supported asynchronous handoffs (`seda`, `vm`, `disruptor`, `disruptor-vm`, and `reactive-streams`) retain the EOS metadata through the shared metadata object. I also addressed the event-notification overhead feedback: the documentation now explicitly explains that the metadata fast path avoids URI inspection and Kafka-specific bookkeeping, but does not eliminate Camel’s per-send event-notification dispatch cost. **Validation:** The full Maven build completed successfully. `KafkaExactlyOnceIT` (23 tests), `KafkaExactlyOnceRegressionIT` (23 tests), and `KafkaExactlyOnceRebalanceIT` (1 test) all passed. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
