allthingssecurity commented on code in PR #26775:
URL: https://github.com/apache/camel/pull/26775#discussion_r4084416935
##########
core/camel-core-processor/src/main/java/org/apache/camel/processor/TotalRequestsThrottler.java:
##########
@@ -184,7 +184,11 @@ protected boolean processAsynchronously(
exchange.setProperty(PROPERTY_EXCHANGE_QUEUED_TIMESTAMP,
System.nanoTime());
}
exchange.setProperty(PROPERTY_EXCHANGE_STATE, State.ASYNC);
- long delay = throttlingState.peek().getDelay(TimeUnit.NANOSECONDS);
+ ThrottlePermit next = throttlingState.peek();
+ // there is no permit in the queue when the rate is 0, or when the
only permits are taken by other
+ // exchanges and not yet returned, so try again after one period
+ long delay = next != null
+ ? next.getDelay(TimeUnit.NANOSECONDS) :
TimeUnit.MILLISECONDS.toNanos(getTimePeriodMillis());
Review Comment:
Yes, it waits longer. The rescheduled run calls `process()` again with state
`ASYNC`. If `poll()` still returns no permit, it does not go async again (that
is only done for state `SYNC`). It falls through to `throttlingState.take()`,
which blocks on the async-delayed thread until a permit is available, then
continues as usual. That is also what already happens when the permit that
`peek()` returned was taken by another exchange during the delay, so the
one-period delay is only the point at which the retry starts, not a deadline.
With `throttle(0)` it therefore waits like the synchronous mode does (blocked
in `take()`) instead of failing with the NPE.
A follow-up could reschedule instead of blocking the async thread in
`take()`, but that would change the existing async-delayed behaviour, so I kept
it out of this fix.
_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]
