[
https://issues.apache.org/jira/browse/CAMEL-25300?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Claus Ibsen resolved CAMEL-25300.
---------------------------------
Resolution: Fixed
> 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
> Assignee: shashank
> Priority: Minor
> Fix For: 4.23.0
>
>
> {{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)
