shashank created CAMEL-25300:
--------------------------------
Summary: Throttle EIP - the clean of the total requests throttler
can hand out a second set of permits, so more exchanges than allowed pass in a
time period
Key: CAMEL-25300
URL: https://issues.apache.org/jira/browse/CAMEL-25300
Project: Camel
Issue Type: Bug
Components: camel-core
Reporter: shashank
{{TotalRequestsThrottler}} keeps a {{ThrottlingState}} (a {{DelayQueue}} of
permits) per correlation key. Every returned permit schedules {{clean()}} 10
periods later ({{cleanPeriodMillis = timePeriodMillis * 10}}, computed once in
the constructor), which does {{states.remove(key)}}.
* An exchange that looked up the state just before the clean runs still polls a
permit from the removed state, while the next exchanges create a new state with
all {{maximumRequests}} permits available at once. So in that period more
exchanges than allowed pass (with 2 per period: 3).
* The permit of that exchange is returned to the removed state, which schedules
the removed state's clean again. 10 periods later that clean does
{{states.remove(key)}}, which removes whatever state is mapped then, also the
new one while it is in use: its delayed permits are dropped, the next exchange
gets a fresh state with all permits, and the clean of the dropped state repeats
this. Under continuous traffic with concurrent senders the throttler can keep
resetting every 10 periods.
* {{setTimePeriodMillis}} (JMX) does not change the clean period. After
increasing the period to more than 10 times its initial value, the clean
removes states whose permits are still delayed, and the next exchanges get new
permits at once.
CAMEL-24926 fixed the same pattern in {{ConcurrentRequestsThrottler}}: its
clean only removes its own state when no permit is out, and an exchange that
took a permit from a removed state takes it from the current state instead.
h3. Reproduction
Test {{TotalRequestsThrottlerCleanTest}} (camel-core, the same technique as
{{ConcurrentRequestsThrottlerCleanTest}} from CAMEL-24926: an executor that
keeps the scheduled clean tasks, and the {{maximumRequests}} expression, which
the throttler evaluates after it looked up the state, runs them for a given
exchange). Throttle 2 per hour, {{rejectExecution(true)}}:
* {{testCleanAfterExchangeLookedUpState}}: "first", then "race" (the clean runs
after it looked up the state, as if 10 periods had passed), then "second" must
pass and the next exchange must be rejected. On main it passes too (3 exchanges
in the period after the clean):
{noformat}
TotalRequestsThrottlerCleanTest.testCleanAfterExchangeLookedUpState:64 Expected
org.apache.camel.CamelExecutionException to be thrown, but nothing was thrown.
{noformat}
* {{testCleanFollowsTimePeriod}}: after {{setTimePeriodMillis(2 hours)}} the
clean is scheduled in 20 hours. On main:
{noformat}
TotalRequestsThrottlerCleanTest.testCleanFollowsTimePeriod:94 expected:
<[72000000]> but was: <[36000000]>
{noformat}
* {{testCleanRemovesUnusedState}} (control): the clean still removes an unused
state (passes on main).
The defect was found with a TLA+ model of the throttler states, the clean
tasks, exchanges (lookup, poll, enqueue as separate steps) and time periods:
"at most maximumRequests permits are handed out per period" is violated in 12
steps (an exchange looks up the state, the clean removes it, the exchange takes
a permit, another exchange creates a new state and takes a permit), and "the
clean of a replaced state never removes the current state" in 13 steps. With
the fix below both hold (2-3 exchanges, 1-2 permits, 2-4 periods).
h3. Proposed fix
As in CAMEL-24926:
* {{clean()}}: {{states.computeIfPresent(key, (k, s) -> s == this &&
markRemovedIfUnused() ? null : s)}}; a state is unused when all its permits are
back in the queue (checked and marked {{removed}} under a lock of its own).
* Unlike CAMEL-24926, that lock is not the state's lock: in this mode a
decrease of the maximum requests (a dynamic {{maximumRequests}} expression,
such as a header) takes permits with {{DelayQueue.take()}} while it holds the
state's lock, and waits when none is available. An exchange that took a permit
must be able to check the flag and return its permit meanwhile, otherwise the
decrease and every exchange of the key wait forever. (The concurrent requests
throttler decreases with {{Semaphore.reducePermits}}, which does not wait.)
* An exchange that polled (or took) a permit from a state that is marked
removed puts the permit back and takes one from the current state.
* The clean is scheduled with the current time period ({{10 x
getTimePeriodMillis()}}, saturated). This belongs to the same fix: "unused"
counts the permits in the queue, delayed or not, so a clean must not run while
a permit is still delayed; with the period fixed at the initial value it could,
after the period was increased.
{{TotalRequestsThrottlerRateDecreaseTest}} (regression test for the lock
above): {{throttle(header("max")).timePeriodMillis(2000)}}; two exchanges take
both permits, two more wait in {{take()}} (the test waits until their threads
are parked), then an exchange with {{max=1}} decreases the maximum and waits
for a permit to discard. All five must get through. It passes on main; with the
first version of the fix (the flag under the state's lock) it hung: the thread
dump shows the two exchanges parked in {{isRemoved()}} on the state's lock and
the decreasing one in {{DelayQueue.take()}} inside
{{calculateAndSetMaxRequestsPerPeriod}}.
With the fix the new tests and the throttle tests
({{org/apache/camel/processor/throttle/**}}, {{Throttl*Test}}: 60 tests) pass.
Severity: the lookup race needs an exchange at the instant the clean runs
(after 10 idle periods); once it happened, the reset can repeat under
continuous traffic; the JMX case is deterministic. Minor.
Affected: 4.14.x, 4.18.x and main (same code).
Duplicate check (2026-10-03): JIRA text "TotalRequestsThrottler" (CAMEL-24928
asyncDelayed NPE, CAMEL-24227 volatile JMX fields), summary
"throttle"/"throttler" since 2023 (CAMEL-24926, CAMEL-20357, CAMEL-20267, ...:
none about the clean of the total requests mode). GitHub pull requests
"TotalRequestsThrottler", "throttler clean": #26775 (CAMEL-24928), #24985,
#24713; the open #26871 touches {{ConcurrentRequestsThrottler}} interrupt
handling only.
_Filed with Claude Code on behalf of allthingssecurity._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
