allthingssecurity opened a new pull request, #29532: URL: https://github.com/apache/camel/pull/29532
# Description [CAMEL-26784](https://issues.apache.org/jira/browse/CAMEL-26784) `HazelcastRoutePolicy` implements the leadership with a thread that takes the Hazelcast lock and then sleeps while holding it; the lock is released only when that thread is interrupted, which today happens only when the policy stops (with the CamelContext). Two route lifecycle callbacks were not handled: - **A removed route kept the leadership.** The policy did not override `onRemove`. A route removed on the leader (`removeRoute`, or a route reload, which removes the route and adds it again) left the lock held, so the other nodes waited for it forever and the route ran nowhere in the cluster. A route reloaded with a new policy instance (for example one created in the route builder) did not run either: the new policy waited for the lock its predecessor still held. - **A follower started its consumer when its route was resumed.** On a node that is not the leader the policy stops the consumer in `onStart`. `onSuspend` takes the route out of the routes to start on takeover, but `onResume` was not overridden, so a `suspendRoute` / `resumeRoute` on a follower (JMX, controlbus, a scheduled route policy) resumed the consumer. Depending on the consumer the follower then consumed next to the leader, or (SEDA, whose resume does not restart the threads its stop ended) the consumer was reported started without consuming. When the follower took the leadership while its route was suspended, the resume did not really start the consumer either, so no node consumed. Change: - the policy counts its routes; `onRemove` of the last one ends the leadership. The leadership thread now waits on a latch of its own instead of sleeping; the latch is counted down, the thread releases the lock and ends, and the node is no longer the leader. It is deliberately not interrupted there: a Hazelcast client call made by an interrupted thread fails at once, so on a client instance the `remove`/`unlock` in the `finally` block would fail and the lock would stay held. When the CamelContext or the policy stops the thread is still interrupted, as before (on a client its wait in `tryLock` ends at once, on a member with `tryLockTimeout`), and the lock is now released with the interrupt flag cleared, which also fixes a policy on a client instance that stopped while leader; - the leadership is set under the policy lock after checking that the routes were not removed meanwhile, so a thread that got the lock just as the last route was removed does not take the leadership; - `onInit` starts a new leadership thread when a route is added to a started policy that has none (a route added again with the same policy instance). `doStart` starts the thread only when the policy has routes; - `onResume` stops the consumer on a follower, as `onStart` does, so it is started again on takeover; and when the node took the leadership while the route was suspended, it restarts the consumer that the policy had stopped (a resume does not start it); - "Leadership lost" is now logged when the node loses the leadership (the condition was inverted, so it was never logged), as the leadership now moves on a route removal. What stays as it was: stopping a route (without removing it) keeps the leadership; the policy still stops a follower's consumer after the route started it, and on resume the consumer of a follower runs for a moment before `onResume` stops it, as on start. On a follower whose last route is removed while the CamelContext keeps running, the leadership thread ends when its current `tryLock` returns (within `tryLockTimeout`, 10 seconds by default); a route added again meanwhile gets a new thread after it. Left for the follow-up on the same file (opens after this one): a failed consumer start at takeover is not retried, a takeover can start the consumer of a route that is being stopped, and `onStart` still reads the leader flag outside the policy lock. `MasterRoutePolicy` in camel-zookeeper-master overrides only `onInit` too (no `onRemove`/`onResume`); not changed here. Tests, new `HazelcastRoutePolicyRouteLifecycleTest` (two CamelContexts on one embedded Hazelcast member, bound to the loopback address; the lock is per leadership thread; a plain `*Test`, no test-infra service, Awaitility and mock waits only): - `removedRouteGivesUpTheLeadership`: the route is removed on the leader, the other node must take over and consume; - `removedRouteGivesUpTheLeadershipWithAClientInstance`: the same, the leader uses a Hazelcast client; - `policySharedByTwoRoutesKeepsTheLeadershipUntilBothAreRemoved`: one policy on two routes, the leadership stays until both are removed; - `routeReloadedWithANewPolicyRunsOnOneNode`: the route is removed and added again on the leader with a new policy: exactly one node must lead and consume; - `routeAddedAgainWithTheSamePolicyRunsOnOneNode`: the same with the same policy instance (worked before, guards the restart of the leadership thread); - `resumedRouteOnAFollowerKeepsItsConsumerStopped`: suspend and resume on the follower, its consumer must stay stopped, and it must consume once it takes over; - `routeResumedAfterTheFollowerTookOverConsumes`: the follower takes over while its route is suspended, and must consume after the resume. On main 6 of the 7 fail (all but the same-policy re-add); against the first version of this change, which interrupted the thread on remove and only stopped the consumer on resume, the client and the resume-after-takeover tests failed. With the change the 7 pass, 140/140 over 20 runs in one JVM, and the camel-hazelcast unit tests pass (262 run, 0 failures; 1 error in `HazelcastUserConfigSerializationFilterWarningTest.warnsForUserClientConfigWithoutFilter`, whose client cannot reach the member's non-loopback address in my sandbox; it does not use the route policy). Upgrade guide: a 4.24 entry, as a route reloaded on the leader can now move to another node. The file is the same on the maintenance branches (4.14.x, 4.18.x, 4.22.x, 4.23.x), so the change cherry-picks; I leave it to the reviewers whether a backport is wanted. Found with Specula (TLA+ model checked against traces of the real code) and confirmed with the tests above. # 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) filed for the change (usually before you start working on it): [CAMEL-26784](https://issues.apache.org/jira/browse/CAMEL-26784). # 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 camel-hazelcast with the formatter and import-sort plugins and ran the camel-hazelcast tests; 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]
