gnodet-bot commented on code in PR #27645:
URL: https://github.com/apache/camel/pull/27645#discussion_r4236774368
##########
components/camel-minio/src/main/java/org/apache/camel/component/minio/MinioConsumer.java:
##########
@@ -279,31 +315,82 @@ public void onDone(Exchange exchange) {
});
}
} catch (Exception e) {
+ if (isNoSuchKey(e)) {
+ // deleted (for example by another consumer) after it
was listed
+ LOG.debug("Skipping object {} as it no longer exists",
srcObjectName);
+ inProgress.remove(srcObjectName);
+ skipped++;
+ continue;
+ }
LOG.warn("Error getting MinioObject due: {}",
e.getMessage());
+ // this and the remaining exchanges of the batch are not
processed
+ releaseInProgress(srcObjectName, exchanges);
+ if (ready != null) {
+ routeExchange(ready, routed, total - skipped, false,
0);
+ }
throw e;
}
}
- // add on completion to handle after work when the exchange is done
- exchange.getExchangeExtension().addOnCompletion(new
Synchronization() {
- public void onComplete(Exchange exchange) {
- processCommit(exchange);
+ if (ready != null) {
+ routeExchange(ready, routed++, total - skipped, false,
exchanges.size() + 1);
Review Comment:
💡 **`BATCH_SIZE` inconsistency when a GET-phase skip follows an
already-queued exchange**
`total - skipped` is evaluated *at the time this `ready` exchange is
routed*, not at the end of the batch. In the scenario where the **next** object
fails with `NoSuchKey`, the current `ready` exchange receives an inflated
`BATCH_SIZE`:
Example — 4 queued objects `[b, c, d, e]` where `b` and `e` fail on GET:
- `b` → NoSuchKey → `skipped=1`, continue
- `c` → OK → `ready=c`
- `d` → OK → routes `c` with `BATCH_SIZE = 4−1 = 3` ← **wrong**; `ready=d`
- `e` → NoSuchKey → `skipped=2`, continue
- after loop → routes `d` with `BATCH_SIZE = 4−2 = 2` ← correct
`c` receives `BATCH_SIZE=3` but the true final count is `2`. Any consumer
reading `BATCH_SIZE` on the first exchange for progress reporting will see
stale data.
`MinioConsumerDeletedObjectTest.deletedObjectsAreSkipped` asserts
`BATCH_SIZE=2` only on the **last** received exchange (`d.txt`), so this
inconsistency on `c.txt` goes undetected.
The fix requires knowing the final skip count before routing any exchange —
one approach is a two-pass strategy (collect all, filter skipped, then route),
but that means holding all object bodies in memory simultaneously. A lighter
alternative is to not expose `BATCH_SIZE` until the last exchange (set it to
`0` or `-1` for intermediate ones) and only set it correctly on
`BATCH_COMPLETE=true`. Given the rarity of the race condition (object deleted
between list and GET), this is low-severity but worth noting.
--
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]
