allthingssecurity commented on PR #29583:
URL: https://github.com/apache/camel/pull/29583#issuecomment-6793879904
Thanks for the review. All three points are in 6e9237d7cd6c.
1. A failed join now undoes itself. `joinGroup` wraps the listener start and
the publishing of the node state. On a failure it stops and shuts down the
listener, removes it from the `CuratorFramework` it registered on, clears
`groupListener` and `thisNodeState`, and rethrows. On the `RouteAddedEvent`
path the failure still goes to the exception handler, since an event notifier
cannot fail the add. The policy then is not connected, `isMaster()` is false,
and it joins again when the route is added again or the policy is started
again. New test `failedJoinLeavesNoHalfJoinedState`: a user `CuratorFramework`
and a `ContainerIdFactory` that fails once, so the join fails after the
listener started, on a route added to a running context. It checks that the
policy is not connected and that no group listener is left on the client, then
that the route joins and consumes once it is added again. It fails on the first
commit (the started listener is kept and still registered). There is also `fa
iledJoinAtStartLeavesNoListenerOnTheCurator` for the join at CamelContext
start. It is a control: there the failed start removes the route, and
`onRemove` cleaned up before as well.
2. `doStop` now runs the same cleanup as `leaveGroup`, through one shared
`closeGroup`: the listener is removed from the client and cleared, and a
consumer started as master is stopped. A failure there is logged at WARN, as in
`leaveGroup`. The client is the policy's curator or else the `curator` bean
from the registry, which are the only clients the listener registers on
(`ZookeeperGroupSupport.setCurator`). Before, it asked the listener's group
factory, whose client can differ when a `ManagedGroupFactory` bean is used. New
test `policyStoppedOnItsOwnLeavesTheGroupAndJoinsAgainWhenStarted`: after
`stopService(policy)` no listener is left on the client, the policy is neither
connected nor master, and the consumer is stopped. After `startService` it is
master again and consumes. It fails on the first commit.
3. `assertConsumes` now uses `MockEndpoint.assertIsSatisfied(context, 20,
TimeUnit.SECONDS)`.
On the NPE: yes, this PR fixes it. On main, a route added to a running
context with a new `MasterRoutePolicy` failed in `doStart` with a
`NullPointerException` ('this.route' is null), and the route was not created.
The description says so ("A route could not be added again"), and
`routeAddedAgainWithANewPolicyRunsAgain` covers it.
Not changed here: `MasterConsumer` in the same module has a similar gap. Its
`doStop` does not remove its group listener from the component's curator. I can
file a follow-up if you want one.
Tests: camel-zookeeper-master 35 run, 0 failures.
`MasterRoutePolicyRouteLifecycleTest` 240/240 over 20 runs in one JVM,
`MasterRoutePolicyLeadershipTest` 100/100. The description is updated.
_Claude Code on behalf of allthingssecurity_
--
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]
