allthingssecurity commented on code in PR #27340:
URL: https://github.com/apache/camel/pull/27340#discussion_r4181333212
##########
core/camel-core-processor/src/main/java/org/apache/camel/processor/errorhandler/RedeliveryErrorHandler.java:
##########
@@ -1214,10 +1214,46 @@ private void runAsynchronousRedelivery() {
LOG.trace("Scheduling redelivery task to run in {} millis for
exchangeId: {}", redeliveryDelay,
exchange.getExchangeId());
}
- executorService.schedule(() ->
reactiveExecutor.schedule(this::redeliver), redeliveryDelay,
+ if (currentRedeliveryPolicy.isAllowRedeliveryWhileStopping()) {
+ executorService.schedule(() ->
reactiveExecutor.schedule(this::redeliver), redeliveryDelay,
+ TimeUnit.MILLISECONDS);
+ } else {
+ // the redelivery is not allowed while stopping, so wake up
every second to check whether we are
+ // preparing for shutdown, the same as the synchronous
redelivery does (see sleep())
+ scheduleAsynchronousRedelivery(new StopWatch());
+ }
+ }
+
+ private void scheduleAsynchronousRedelivery(StopWatch watch) {
+ long delay = Math.max(0, Math.min(1000, redeliveryDelay -
watch.taken()));
Review Comment:
The extra cost applies only with `allowRedeliveryWhileStopping=false`; the
default (`true`) still schedules one task
for the full delay, as before. With `false`, each waiting asynchronous
redelivery gets one scheduler task per second of
its delay, and each task does one reactive-executor hop that checks
`preparingShutdown` and the stopwatch. That is
the same one-second cadence as the synchronous path's `sleep()`. The
synchronous path also blocks a whole thread for
the entire delay, which is much more expensive per waiting exchange. So with
N exchanges waiting there are N short
tasks per second on the error handler's scheduler. I think that is
acceptable for an opt-in setting.
I did consider tracking the scheduled futures and firing them from
`prepareShutdown()`. I chose polling because:
- it keeps the state inside the redelivery task: no shared registry of
pending redeliveries, and no add/remove on a
concurrent structure for every redelivery while nothing is shutting down;
- firing from `prepareShutdown()` races with a task whose delay is just
expiring. Each task would then need a
compare-and-set so it either redelivers or is rejected exactly once, and
the registry needs cleanup on every path
(redelivered, rejected, scheduler rejected, error handler stopped);
- polling gives the asynchronous path exactly the behaviour of the
synchronous one (rejection within about one second
of `prepareShutdown`), so the two paths stay identical.
If you prefer the registry (rejection at once instead of within a second,
and no wake-ups), I can switch to it.
_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]
