davsclaus commented on code in PR #27677:
URL: https://github.com/apache/camel/pull/27677#discussion_r4240436403
##########
docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc:
##########
@@ -863,6 +863,15 @@ comma and the statement ran once for each part (once for
each byte of a `byte[]`
Collections, iterators and other arrays (including `int[]`, `long[]` and other
primitive arrays) still run the statement
once per element; send a `List` to run it once per value.
+=== camel-mybatis, camel-sql - onConsume does not run for a failed or rollback
only exchange
Review Comment:
For camel-mybatis this changes runtime behaviour: a row that always fails is
now polled again on every poll instead of being marked processed. The
camel-mybatis entry just above is tagged as breaking; suggest the same here:
```suggestion
=== camel-mybatis, camel-sql - onConsume does not run for a failed or
rollback only exchange (Breaking change)
```
##########
components/camel-mybatis/src/main/java/org/apache/camel/component/mybatis/MyBatisConsumer.java:
##########
@@ -136,18 +136,21 @@ public int processBatch(Queue<Object> exchanges) throws
Exception {
try {
getProcessor().process(exchange);
- if (onConsume != null) {
+ // only consume the data when the exchange was processed
successfully: a failed or rollback only
+ // exchange leaves the data as it is, so the next poll
consumes it again
+ if (onConsume != null && !exchange.isFailed() &&
!exchange.isRollbackOnly()) {
endpoint.getProcessingStrategy().commit(endpoint,
exchange, data, onConsume);
}
} catch (Exception e) {
handleException(e);
}
- if (getEndpoint().isTransacted() && exchange.isFailed()) {
+ if (getEndpoint().isTransacted() && (exchange.isFailed() ||
exchange.isRollbackOnly())) {
// break out as we are transacted and should rollback
cause = exchange.getException();
if (cause == null) {
- cause = new RollbackExchangeException("Rollback
transaction due error processing exchange", null);
+ // rollback only (the exception must be created before the
exchange is released)
+ cause = new RollbackExchangeException("Rollback
transaction due error processing exchange", exchange);
Review Comment:
Minor: for a rollback-only exchange there was no error, so this message is a
bit misleading (and it ends up as a WARN from the poll strategy on each such
poll). Maybe distinguish the two cases, e.g. `"Rollback transaction due
exchange marked as rollback only"`. Same in `SqlConsumer`.
##########
components/camel-sql/src/main/java/org/apache/camel/component/sql/SqlConsumer.java:
##########
@@ -278,20 +278,23 @@ public int processBatch(Queue<Object> exchanges) throws
Exception {
exchange.setException(e);
}
- if (getEndpoint().isTransacted() && exchange.isFailed()) {
+ // a rollback only exchange (without an exception) did not
complete successfully either
+ boolean failed = exchange.isFailed() || exchange.isRollbackOnly();
+
+ if (getEndpoint().isTransacted() && failed) {
// break out as we are transacted and should rollback
Exception cause = exchange.getException();
+ if (cause == null) {
+ // rollback only (the exception must be created before the
exchange is released)
+ cause = new RollbackExchangeException("Rollback
transaction due error processing exchange", exchange);
Review Comment:
Same as in `MyBatisConsumer`: the message says *error* also for a
rollback-only exchange.
--
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]
