Repository navigation
CAMEL-25527: camel-mybatis, camel-sql - do not run onConsume for a failed or rollback only exchange - #27677
Conversation
…iled or rollback only exchange The camel-mybatis consumer ran the onConsume statements after every exchange, also when the exchange failed (a failed route sets the exception on the exchange instead of throwing it) or was marked rollback only, so the row was marked as consumed and was not polled again. It now runs them only for an exchange that completed successfully; the row of a failed or rollback only exchange is consumed again by the next poll. Handle the exception in the route, for example with onException(...).handled(true), to consume it anyway. The camel-sql consumer picked onConsumeFailed only for an exchange with an exception, so a rollback only exchange got onConsume. It now gets onConsumeFailed. With transacted=true both consumers now also break out of the batch at a rollback only exchange, as they do at a failed one. That path created RollbackExchangeException without the exchange, which threw a NullPointerException; it is now created with the exchange, before the exchange is released. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
davsclaus
left a comment
There was a problem hiding this comment.
Thanks! This matches the documented contract ("after successful processing") and lines up with the merged camel-jooq fix (CAMEL-25511), including the handled(true) escape hatch. Good tests, including the pooled exchange factory variants.
Two small optional points inline. Note: camel-jpa has the same rollback-only gap, as you mention in the description, so a follow-up JIRA would be welcome.
Claude Code on behalf of davsclaus. This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying. It is a static review against the project conventions and does not replace static analysis or specialized review tools.
…ade note as breaking, own message for a rollback only exchange The exception that breaks out of a transacted batch for a rollback only exchange no longer says there was an error processing the exchange. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 12 of 704 tested, 25 compile-only — current: 12 all testedMaveniverse Scalpel detected 12 affected modules (current approach: 12). Skip-tests mode would test 12 modules (4 direct + 10 downstream), skip tests for 25 (generated code, meta-modules) Modules Scalpel would test (12)
Modules with tests skipped (25)
All tested modules (39 modules, 6m 22s total)Total reactor time: 6m 22s
Top 20 slowest modules:
|
Description
CAMEL-25527
MyBatisConsumer.processBatchran theonConsumestatements after every exchange, also when the exchange failed (a failed route leaves the exception on the exchange instead of throwing it) or was marked rollback only. The row was then marked as consumed (in the documented exampleupdate ACCOUNT set PROCESSED = true) and was not polled again, although the javadoc says the statements run "after successful processing".SqlConsumer.processBatchpickedonConsumeFailedfromexchange.isFailed()only, so a rollback only exchange (no exception) gotonConsume. This is the sibling of the camel-jooq fix CAMEL-25511.Now:
onConsumeonly when the exchange is neither failed nor rollback only. The row of a failed exchange is consumed again by the next poll, so a row that always fails is processed at every poll;onException(...).handled(true)consumes it anyway (docs and catalog copy say so).onConsumeFailedfor a rollback only exchange, and no statement whenonConsumeFailedis not set (as for a failed exchange), so the row is consumed again by the next poll.transacted=true, both consumers now break out of the batch at a rollback only exchange, as they do at a failed one. That code path createdRollbackExchangeExceptionwith anullexchange, which throws aNullPointerException(it had never run, sinceisFailed()implies an exception); it is now created with the exchange, before the exchange is released.Both consumers already create their exchanges with
createExchange(false)and callreleaseExchange(exchange, false)after reading the outcome, so the change also holds with the pooled exchange factory; the tests run with it too. No extra per-message cost (one more flag read). An upgrade guide note is added for 4.23 under its own heading next to the existing camel-mybatis entry.Tests:
MyBatisOnConsumeFailedTest: one poll (scheduler not started) where the route fails for one account, marks one exchange rollback only, or (withtransacted=true) marks the second of three rollback only.SqlConsumerRollbackOnlyTest: one poll over the three projects ofcreateAndPopulateDatabase.sqlwhere the AMQ exchange is marked rollback only: withonConsumeFailed, without it, and withtransacted=true.MyBatisOnConsumeFailedPooledExchangeTestandSqlConsumerRollbackOnlyPooledExchangeTestrun the same tests withPooledExchangeFactory.expected: <[2]> but was: <[]>,expected: <[DONE, BAD, DONE]> but was: <[DONE, DONE, DONE]>,expected: <[DONE, ASF, DONE]> but was: <[DONE, DONE, DONE]>andExpected org.apache.camel.RollbackExchangeException to be thrown, but nothing was thrown.SqlFunctionDataSourceTestwas excluded locally: it needs MariaDB4j, which does not install on my machine, and fails the same way without the change.Not changed here:
JpaConsumerruns the@Consumeddelete handler whenexchange.getException() == null, so a rollback only exchange is consumed there as well; that can be a follow-up if wanted.Target
mainbranch)Tracking
Apache Camel coding standards and style
mvn clean install -DskipTestslocally from root folder and I have committed all auto-generated changes.(I built and tested camel-mybatis and camel-sql with
install, including the formatter and import-sort plugins. I did not run the full root build.)AI-assisted contributions
Co-authored-bytrailers) and the PR description identifies the AI tool used.This PR was prepared with Claude Code (Claude Opus 5.5). The commit carries a
Co-Authored-Bytrailer.Claude Code on behalf of allthingssecurity
🤖 Generated with Claude Code