[
https://issues.apache.org/jira/browse/CAMEL-26291?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18132246#comment-18132246
]
Claus Ibsen commented on CAMEL-26291:
-------------------------------------
Fixed by https://github.com/apache/camel/pull/28467, merged to main as
295b8d648cf2014ee671f264634a31e60a027386 (Camel 4.24.0).
_Claude Code on behalf of davsclaus_
> camel-core - ThrottlingExceptionRoutePolicy counts the failures since the
> circuit last closed instead of those within failureWindow: old failures open
> the circuit, and a successful exchange in the half open state opens it again
> -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: CAMEL-26291
> URL: https://issues.apache.org/jira/browse/CAMEL-26291
> Project: Camel
> Issue Type: Bug
> Components: camel-core
> Reporter: shashank
> Assignee: shashank
> Priority: Major
> Fix For: 4.24.0
>
>
> {{ThrottlingExceptionRoutePolicy}} (camel-support) documents
> {{failureThreshold}} as "How many failed messages within the window would
> trigger the circuit breaker to open" and {{failureWindow}} as the "Sliding
> window for how long time to go back (in millis) when counting number of
> failures". It counts every failure since the circuit last closed (or since
> the route started) instead, and only checks that the *last* failure is within
> the window: {{failures.get() >= failureThreshold && lastFailure >=
> System.currentTimeMillis() - failureWindow}} (main a090e27eb02a,
> {{isThresholdExceeded}} line 304; the counter is only reset in
> {{closeCircuit}}, line 390).
> Two consequences:
> # *Old failures open the circuit.* With {{failureThreshold=2}} and
> {{failureWindow=100}}, a failure, then a second failure more than 100 ms
> later, opens the circuit and suspends the consumer, although only one failure
> is within the window. With the defaults (50 failures, 60 s), a route with an
> occasional failure is suspended on a single failure once it has collected 50
> since it started.
> # *A successful exchange in the half open state opens the circuit again.* In
> the half open state ({{calculateState}}, line 262) the policy opens the
> circuit when the threshold is exceeded and closes it otherwise. The failures
> that opened the circuit are still counted, so as long as the last of them is
> within {{failureWindow}}, the successful trial exchange opens the circuit
> again (the consumer is suspended again). With the defaults ({{halfOpenAfter}}
> 30 s, {{failureWindow}} 60 s) a successful trial within 30 s of the first
> half open check therefore opens the circuit again: the route stays suspended
> for about 60 s instead of 30 s, and with a larger window (say 10 minutes,
> retry every 30 s) until the failures leave the window, after the backend has
> recovered, while each check resumes the consumer until the first exchange is
> done. The existing half open tests all use a window (30 ms) shorter than
> {{halfOpenAfter}}, where the old failures have left the window.
> Reproduced with a route {{from("direct:...").routePolicy(policy)}} and
> {{ProducerTemplate}} (latches, no sleeps): {{new
> ThrottlingExceptionRoutePolicy(1, 60000, 100, null)}}: a failure opens the
> circuit, the timer half opens it, a successful exchange: state "opened",
> consumer suspended. {{new ThrottlingExceptionRoutePolicy(2, 1000, 60000,
> null)}}: failure, wait until the window has passed, failure: state "opened".
> *Fix*: the policy keeps the times of the last {{failureThreshold}} failures;
> the threshold is exceeded when the oldest of them is within {{failureWindow}}
> (a sliding window, as documented). In the half open state the exchange
> decides: a failure opens the circuit again, a success closes it. The half
> open check of the timer leaves a half open circuit to the next exchange: an
> exchange that was in flight when the circuit opened can be done when the
> check is due, find the circuit still open and half open it itself, and the
> timer's check must then not open it again because of the failure that opened
> it. The check of an open circuit and {{keepOpen}} are unchanged. The counters
> shown by JMX and the dev console ({{failures}}, {{success}}, {{lastFailure}})
> keep their meaning. The failure times have their own lock, as the policy lock
> is held while the consumer is suspended or resumed. The class javadoc, which
> said the half open state checks the failure threshold, now says that the next
> exchange decides. Upgrade guide entry for 4.24. The existing tests that open
> the circuit with two failures used a 30 ms window; as both failures must now
> be within it, they use 60 s.
> *Tests*: new {{ThrottlingExceptionRoutePolicyFailureWindowTest}} (camel-core,
> latches): {{testSuccessInHalfOpenStateClosesTheCircuit}} and
> {{testFailuresOutsideTheWindowDoNotOpenTheCircuit}} fail on main and pass
> with the fix; the second also checks that two failures within the window
> still open the circuit.
> {{testHalfOpenTimerDoesNotReopenACircuitHalfOpenedByAnExchange}} holds the
> timer's check with a latch until the straggler has half opened the circuit;
> it fails on main and on the first version of the fix (state "opened") and
> passes with the fix. The other tests of the policy (camel-core,
> camel-management, camel-console) pass.
> *How it was found*: first by reading the class against the documented meaning
> of {{failureThreshold}}/{{failureWindow}} while writing a driver for the Rocq
> JVM model (a JVM semantics in Rocq that executes the real class files). Its
> thread layer (threads, locks, atomics, a clock, {{java.util.Timer}}) now runs
> the production class with its own timer, and kernel-checked theorems on the
> class files of main a090e27eb02a state both consequences:
> {{main_failures_outside_window_open}} (threshold 2, window 100 ms: two
> failures 201 ms apart open the circuit) and
> {{main_half_open_success_reopens}} (threshold 1, window 60 s, half open after
> 30 s: the successful trial opens the circuit again). The same theorems on the
> fix show the documented behaviour. A schedule search over the model then
> found the timer race above in the first version of the fix
> ({{fix_straggler_first_timer_reopens}}, a two-thread schedule replayed in the
> kernel); the fix handles it ({{fix2_straggler_first_stays_half_open}}). Every
> model result was also checked on HotSpot with the same class files, and the
> race with the test above.
> *Affected*: main, camel-4.22.x, camel-4.18.x and camel-4.14.x (same
> {{isThresholdExceeded}} and half open decision in {{calculateState}}; read,
> not run). Users see the change (upgrade guide), so main only is suggested;
> 4.18.x and 4.14.x do not have CAMEL-25014 ({{reopenCircuit}}).
> *Related*: PR #28382 (CAMEL-25911, {{TimeSource}}) changes the same lines of
> {{onExchangeDone}} and {{isThresholdExceeded}} (small rebase for whichever is
> merged second); CAMEL-26104 (the half open timer through the
> ExecutorServiceManager) touches the timer task.
> _Filed with Claude Code on behalf of allthingssecurity._
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
