allthingssecurity opened a new pull request, #28467: URL: https://github.com/apache/camel/pull/28467
# Description [CAMEL-26291](https://issues.apache.org/jira/browse/CAMEL-26291) `ThrottlingExceptionRoutePolicy` documents `failureThreshold` as the number of failed messages *within the window* that opens the circuit, and `failureWindow` as the sliding window used to count them. It counted every failure since the circuit last closed instead, and only checked that the last failure was within the window (`failures.get() >= failureThreshold && lastFailure >= now - failureWindow`). Two consequences: 1. Failures spread over a long time open the circuit on a single new failure: with `failureThreshold=2` and `failureWindow=100`, two failures more than 100 ms apart open it. With the defaults a route that fails now and then is suspended on one failure once it has 50 since it started. 2. In the half open state the circuit is opened again when the threshold is exceeded, and closed otherwise. The failures that opened the circuit are still counted, so a successful trial exchange opens the circuit again as long as they are within the window. With the defaults (half open after 30 s, window 60 s) a successful trial within 30 s of the first half open check opens the circuit again; with a long window the route stays suspended after the backend has recovered until those failures leave the window. The existing half open tests used a 30 ms window, shorter than `halfOpenAfter`, which hid this. Changes: - The policy keeps the times of the last `failureThreshold` failures (an `ArrayDeque`, with its own lock: the policy lock is held while the consumer is suspended or resumed). The threshold is exceeded when the oldest of them is within `failureWindow`. A threshold of 0 or less still opens the circuit on one failure within the window, as before. - In the half open state the done exchange decides: a failure opens the circuit again, a success closes it. The check of an open circuit after `halfOpenAfter` (the timer) and `keepOpen` are unchanged. - The timer's half open check leaves a half open circuit to the next exchange. An exchange that was in flight when the circuit opened can be done just when the check is due: it finds the circuit still open and half opens it itself, and the timer's check, which runs at the same time, then finds the circuit half open. A first version of this fix decided there with the failure threshold: the failure that opened the circuit was still within the window, so the circuit was opened again and the consumer suspended for another `halfOpenAfter`, although no exchange had failed since. - The class javadoc said that in the half open state the policy would "start the consumer and check the failure threshold"; it now says that the next exchange decides. - The `failures`, `success` and `lastFailure` values shown by JMX and the dev console keep their meaning. - Upgrade guide (4.24): an entry for the two behaviour changes, with what to change to open the circuit sooner. - The existing tests that open the circuit with two failures (`ThrottlingExceptionRoutePolicyTest`, `...HalfOpenTest`, `...HalfOpenHandlerTest`, `...HalfOpenHandlerSedaTest`) used `failureWindow=30`. Both failures must now be within the window, which two sends in a row do not guarantee on a busy CI machine, so they use 60 s. Nothing else in them depends on the window. - Backport: camel-4.22.x, 4.18.x and 4.14.x have the same `isThresholdExceeded` and half open decision (read, not run). As users see the change (upgrade guide), I suggest main only. The code cherry-picks onto 4.22.x without conflict (the guide entry stays on main); 4.18.x and 4.14.x do not have CAMEL-25014 (`reopenCircuit`) and need adapting. Tests: - New `ThrottlingExceptionRoutePolicyFailureWindowTest` (camel-core, direct routes, latches, no sleeps): `testSuccessInHalfOpenStateClosesTheCircuit` and `testFailuresOutsideTheWindowDoNotOpenTheCircuit` fail on main (state `opened` instead of `closed`) 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 a straggler exchange 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 first test holds the timer's check the same way until it has checked that the circuit is open. The only wait that is not a latch is for the clock to pass the 1 s failure window (Awaitility). - camel-support: 140 tests, 0 failures. camel-core, the `*Throttl*`, `*RoutePolicy*` and `org.apache.camel.support` tests: 614 tests, 1 failure, `RouteWatcherReloadChangedRoutesTest.theSummarySaysWhichRoutesTheReloadChanged`, which fails the same way on an unchanged origin/main in this offline environment (older snapshot jars of the modules outside the core reactor). camel-management `ManagedThrottling*RoutePolicyTest` and camel-console `CircuitBreakerDevConsoleTest`: 3 tests, 0 failures. The formatter and import-sort plugins ran. How it was found: first by reading the class against the documented meaning of `failureThreshold`/`failureWindow`, while writing a driver for a JVM model in Rocq that executes the real class files. The model's thread layer (threads, locks, atomics, a clock and `java.util.Timer`) now runs this class with its own timer, and kernel-checked theorems on the class files of main state both consequences (`main_failures_outside_window_open`, `main_half_open_success_reopens`) and the corrected behaviour with this change. A schedule search over the model found the timer race above in the first version of the fix (`fix_straggler_first_timer_reopens`); with this change that schedule leaves the circuit half open (`fix2_straggler_first_stays_half_open`). The results were checked on HotSpot with the same class files, and the race with the test above. Note: rebased onto main after PR #28382 (CAMEL-25911) was merged; the policy and the new test read the clock through `TimeSource.currentTimeMillis()`, like the rest of the class. CAMEL-26104 (the half open timer through the `ExecutorServiceManager`) must keep the timer task's `calculateState(route, null)`. # Target - [x] I checked that the commit is targeting the correct branch (Camel 4 uses the `main` branch) # Tracking - [x] If this is a large change, bug fix, or code improvement, I checked there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL-26291) filed for the change (usually before you start working on it): [CAMEL-26291](https://issues.apache.org/jira/browse/CAMEL-26291). # Apache Camel coding standards and style - [x] I checked that each commit in the pull request has a meaningful subject line and body. - [ ] I have run `mvn clean install -DskipTests` locally from root folder and I have committed all auto-generated changes. (I built the core modules and ran the tests above, including the formatter and import-sort plugins. No generated file changes. I did not run the full root build.) # AI-assisted contributions - [x] If this PR includes AI-generated code, commits have proper co-authorship attribution (e.g., `Co-authored-by` trailers) and the PR description identifies the AI tool used. This PR was prepared with Claude Code (Claude Opus 5.5). The commit carries a `Co-Authored-By` trailer. _Claude Code on behalf of allthingssecurity_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
