allthingssecurity opened a new pull request, #29123: URL: https://github.com/apache/camel/pull/29123
# Description [CAMEL-26540](https://issues.apache.org/jira/browse/CAMEL-26540) `MasterRoutePolicy` (camel-zookeeper-master) starts the route consumer from the `CHANGED` group event (group thread) and stops it from `DISCONNECTED` (Curator connection-state thread, and the closing thread in `ZooKeeperGroup.close()`), with nothing ordering the two: - `ZookeeperGroupListenerSupport.groupEvent` checks `isConnected()` and `isMaster()`, then `onLockOwned` sets `masterConsumer` and starts the consumer. A disconnect handled between the check and the start sets `masterConsumer` to false and stops nothing (the consumer is not started yet), and the consumer is then started. When the session expires another node becomes the master and starts its consumer, while this one keeps consuming, also after it reconnects as a standby (standby events stop nothing); - a failed start (`startConsumer` throws, for example the broker is not reachable when the node is elected) is only logged and leaves `masterConsumer` true, so no later event starts the consumer again: the node holds the leadership and nobody consumes until it loses its connection. These are the defects that CAMEL-25315 fixed in `MasterConsumer`. This change starts and stops the consumer under a lock of the policy. A start checks again under it that the group is connected, this node is the master and the policy is running. A failed start leaves `masterConsumer` false, goes to the route policy's exception handler (as in `MasterConsumer`), and is retried after 5 seconds while this node is the master; a disconnect or a stop cancels the retry. A start interrupted by the stop of the policy is only logged at DEBUG. One difference from `MasterConsumer`, which creates a new delegate and starts it outside its lock with a generation counter: the policy starts the route's own consumer while holding the lock. A `DISCONNECTED` already waited for such a start on the consumer's lifecycle lock, so it gets no slower. The stop of the policy first shuts down the retry thread and closes the group, which interrupts the group thread as before, and only then takes the lock, so it does not wait for a start that hangs. `groupListener` is now volatile (read by the group, connection and retry threads). The defects were found with a TLA+ model of the policy (two nodes, each with the group thread and the connection thread; ZooKeeper sessions that are suspended, expire and reconnect; a failed start). "At most one consumer" and "a consumer only runs on the master" are violated in 12 steps (check, `compareAndSet`, connection suspended, disconnect handled, start, session expired, the other node elected), and "the master consumes" in 5 steps with one failed start. With the change all three hold (up to 5 network events and a failed start). The model does not cover the stop of the policy; the tests do. Tests: new `MasterRoutePolicyLeadershipTest`, no ZooKeeper: a `ManagedGroupFactory` bound in the registry returns a fake group (as `MasterConsumerLeadershipTest`) that can hold the group thread right after its `isMaster()` check, and interrupts that thread when it is closed, as `ZooKeeperGroup` does. - `testDisconnectedWhileElected` delivers `DISCONNECTED` while the group thread is held after its check. It shows the stale check; it cannot tell the lock from a plain second check, which the model covers. - `testFailedStartIsRetried`: the first start fails, the master must start the consumer later. - `testDisconnectedCancelsTheRetry`, `testStopCancelsTheRetry`: a pending retry must not start the consumer after a disconnect or a stop (they also pass without the change, which has no retry). - `testStopWhileStarting`: the policy stops while a consumer start hangs; the stop must not wait for it. Without the main-code change (the tests that set the retry delay compiled against a no-op setter; `testStopWhileStarting` and the two retry-cancel tests pass there too, as main has no retry and its stop closed the group first): ``` MasterRoutePolicyLeadershipTest.testDisconnectedWhileElected A node that lost the leadership must not consume ==> expected: <0> but was: <1> MasterRoutePolicyLeadershipTest.testFailedStartIsRetried » ConditionTimeout ... The master must start its consumer after a failed start ==> expected: <1> but was: <0> within 20 seconds. ``` With the change the camel-zookeeper-master unit tests pass, and the new tests and `MasterConsumerLeadershipTest` pass 20 times in a row. The integration tests need Docker and were not run. Not changed here, a follow-up JIRA will be filed: `doStart` calls `StringHelper.notEmpty("groupName", groupName)` with the arguments swapped, so a missing group name is not rejected. Affected: main, 4.23.x, 4.22.x, 4.18.x and 4.14.x (the same code; the test uses camel-test-junit6, so the backport to the junit5 branches needs the import changed). # 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 module, 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]
