allthingssecurity commented on PR #29667:
URL: https://github.com/apache/camel/pull/29667#issuecomment-6819864069
Thanks for the review. All four points are in 34d8a63b6623.
1. Fixed here, as you suggested. `KafkaTransactionSynchronization` marks the
source `KafkaExactlyOnceMetadata` (`markOffsetsCommittedAfterHandledFailure`)
only after the offsets-only `commitTransaction` succeeded.
`completeExactlyOnceProcessing` then skips the rollback-only branch when that
mark is set and `TRANSACTION_ABORTED_PROPERTY` is not. The rollback-only flag
of the exchange is not cleared. The mark sits on the metadata object, which is
shared with copies of the exchange and is new for every polled record, so it
cannot leak into a replay. One correction to the two paths in your note:
exactlyOnce requires `breakOnFirstError=true`
(`KafkaConsumer.validateExactlyOnceConfiguration`), so the
`breakOnFirstError=false` ERROR path cannot happen. The symptom was the WARN
"will seek" plus a reconnect and group rejoin, and that is now gone.
Unit tests:
- `KafkaTransactionSynchronizationTest`: the mark is set after the
offsets-only commit; it is not set when that commit fails (new test); it is not
set in the rollback controls.
- `KafkaRecordStreamingProcessorTest`, with `breakOnFirstError` and a
real exchange: rollback only plus the mark gives no exception, no break, and
the offset is recorded; the control without the mark is reported as rolled back
and breaks.
With the production classes of 7f7080642e63, the mark test and the
no-report test fail; with the change they pass.
2. Done: "once CAMEL-26447 is merged" is gone, and the comment covers both
doCatch cases.
3. Renamed to `handledRejectedSend`.
4. Added
`KafkaExactlyOnceIT.rejectedRedeliveryCaughtByDoCatchOfCallingRouteSkipsTheRecord`,
built on your template:
- Route: a `doTry` calls a `direct` route with `maximumRedeliveries(1)`,
whose sink has `maxRequestSize=1024`, and the poison records of the other cases
go through it.
- Asserts: committed offset 3; output `small-0, small-2`; one attempt for
`big-1`; one catch.
- Point 1 check: the consumer is created only once, counted through a
`DefaultKafkaClientFactory` subclass on the component, since a record reported
as rolled back reconnects the consumer.
It compiles, but I could not run it here (no Docker). The PR build runs
`install` on the changed modules without skipping failsafe for camel-kafka, so
it should run there; I could not confirm that from the last job log, because
the Maven output goes to an artifact.
Tests: camel-kafka unit tests 332 run, 0 failures, 1 skipped
(camel-kafka-common 11); the two unit test classes 840/840 over 20 runs in one
JVM. The description is updated.
_Claude Code on behalf of allthingssecurity_
--
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]
