allthingssecurity opened a new pull request, #27391: URL: https://github.com/apache/camel/pull/27391
# Description [CAMEL-25315](https://issues.apache.org/jira/browse/CAMEL-25315) `MasterConsumer` (camel-zookeeper-master) starts the delegate consumer from the `CHANGED` group event (group thread) and stops it from `DISCONNECTED` (Curator connection-state thread, and the closing thread in `ZooKeeperGroup.close()`) and from `doStop`, with no synchronisation and only `if (delegate == null)`: - a delegate whose start failed (broker or server not reachable when the node is elected) stays in the field, so no later event starts it again: the node keeps the leadership and nobody consumes until it loses its connection or the route is restarted; - a disconnect or a stop between the null check and the assignment of the new delegate stops nothing, and the delegate is then created and started: it keeps consuming on a node that is no longer the master (also after reconnecting as a standby), or after the route stopped (when the start takes longer than the 5 s `close()` waits for the group thread). This change adds a `leadershipLock` and a generation counter incremented by `onDisconnected` and `doStop`. A start is marked under the lock after checking again that the group is connected and this node is the master; the consumer is created and started without holding the lock; it is published under the lock only if the generation did not change, otherwise stopped. A failed start is retried after 5 seconds by a scheduled task of the same generation (cancelled by a disconnect or stop). The started state is published once per leadership term, so a retry does not trigger another group event. It follows camel-master's `MasterConsumer` (CAMEL-24583, CAMEL-24626) in its locking and its lost-leadership checks, with two differences: camel-zookeeper-master has no `backOffDelay`/`backOffMaxAttempts` options, so the retry delay is fixed at camel-master's default of 5 seconds and the retries do not stop while this node is the master (camel-master gives up after 10 attempts and stays leader w ithout consuming); and each attempt creates a new delegate consumer (a failed `start()` already stopped the previous one), where camel-master reuses one. Each failed attempt is logged at WARN with its stack trace. The defects were found with a TLA+ model of the consumer (group thread, connection thread, stopping thread, each statement one step, `close()` waiting at most 5 s): "a master that received a leadership event consumes" is violated in 3 steps with one failed start; "no delegate runs on a stopped route or a node that is not the master" in 5 steps (event, connection lost, stopConsumer, create, start). With the fix all properties hold (up to 3 events, 2 failed starts, a disconnect and a stop, deadlock check on); a fix without the generation check, or without the retry, violates them. The model does not cover reconnecting as master after a disconnect. As @oscerd pointed out on the ticket, the old code only got stuck when the failure came at or after the assignment of `delegate` (`updateState` or `startService(delegate)`, which is where most consumers connect); here the consumer is only published after a successful start, and every failure is retried. He also noted that `delegate` and `delegateService` were plain fields written by two threads: all writes and the start-path reads are now under the lock, and both fields are `volatile` because `doSuspend`/`doResume` read them without it. Affected: main, 4.22.x and 4.18.x (same code; 4.14.x is end of life). Tests: new `MasterConsumerLeadershipTest`, no ZooKeeper: a `ManagedGroupFactory` bound in the registry returns a fake group whose events the test fires, and the delegate endpoint fails a start or holds `createConsumer` on a latch (latches, Awaitility). Without the main-code change: ``` MasterConsumerLeadershipTest.testFailedStartIsRetried:73 » ConditionTimeout ... The master must start its consumer after a failed start ==> expected: <1> but was: <0> within 20 seconds. MasterConsumerLeadershipTest.testDisconnectedWhileCreatingConsumer:93 A node that lost the leadership must not consume ==> expected: <0> but was: <1> MasterConsumerLeadershipTest.testStoppedWhileCreatingConsumer:110 A stopped route must not consume ==> expected: <0> but was: <1> ``` With the change the camel-zookeeper-master unit tests pass (14). The integration tests need Docker and were not run. # 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). # 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 and tested the affected modules, including the formatter and import-sort plugins. 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]
