[
https://issues.apache.org/jira/browse/CAMEL-24457?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18108881#comment-18108881
]
mustafa kamal commented on CAMEL-24457:
---------------------------------------
Some information from Claude Code that might help
What actually releases the leadership thread
AbstractCamelContext.doStop() (sources line 3289):
// Stop the route controller
camelContextExtension.stopAndShutdownRouteController();
→ ClusteredRouteController.doStop():200 → clusterService.stop() →
AbstractCamelClusterService.doStop():129 → holder.get().stop() → your doStop()
→ selector.close().
That happens before routes are shut down.
InternalServiceManager.shutdownServices(this, list, false) - the call that
leads to onRemove → releaseClusterView - is ~35 lines further down.
So I had the order wrong earlier: removeEventListener does not run before
doStop(). The view is stopped first, by the route controller, while all N
policy listeners are still registered. The Curator thread is released right
there and fires into a fully-populated listener list, seconds before the hook
reaches releaseClusterView.
That's why this reproduces at all - it isn't a lucky race on the trigger, it's
the designed order.
1. Why isRunAllowed() solves it
BaseService.stop():
status = STOPPING; // line 173
try (AutoCloseable ignored = doLifecycleChange()) {
doStop(); // ← your close() runs here
and
public boolean isRunAllowed() {
return status >= STARTING && status <= SUSPENDED; // STOPPING = 8, outside
}
Status is already STOPPING when doStop() calls close(). The leadership thread
unwinds into your finally and evaluates isRunAllowed() at a point where it
cannot be anything but false - same for the polling predicate () ->
!isRunAllowed(), which is the same condition that ends the task.
So the guard isn't narrowing a window, it's deterministic on this path. The
Curator thread returns without entering fireLeadershipChangedEvent, never takes
the StampedLock read, never asks any policy for its ReentrantLock. The hook's
removeEventListener write lock is uncontended. One edge of the ABBA cycle is
simply gone.
Genuine leadership loss while running is unaffected - status is STARTED, the
event fires as before.
2. Does anything still get told, and does it matter?
For ClusteredRoutePolicy: nothing is lost. Before the guard, the policies
really were receiving leadership-lost at controller-stop time and running
stopManagedRoutes(). After the guard they don't - but every route is about to
be shut down by the very next phase of doStop() anyway, and each policy still
clears its own flag:
// releaseClusterView() finally
setLeader(false);
Same end state, reached by the shutdown thread instead of the Curator thread.
setLeader is an AtomicBoempotent either way.
For third-party Leadership listeners: it is a real behavior change, and worth
stating explicitly in the PR. They no longer get a leadership-lost callback
when the view stops.
The defensible argument for that: the event they'd receive is already useless.
Your doStop() nulls the selector before closing it -
leaderSelector = null;
if (selector != null) \{ leader = false; selector.close(); }
- so a listener called from that path sees getLeader() → Optional.empty()
(guarded by isStoppingOrStopped()) and getMembers() → Collections.emptyList().
It's a notification about a view that no longer exists. Camel's general
ServiceSupport contract is that a stopping service doesn't emit events, so
skipping it is consistent; listeners should treat view stop as implicit
leadership loss and hook the view's lifecycle if they need cleanup.
My suggestion for the PR: keep the guard, and add a comment on the finally
saying leadership-lost is intentionally not fired once the view is stopping,
because the view is already torn down and every listener is about to be removed.
The fix worth proposing separately
The guard removes the cycle from the ZK view, but the inversion itself lives in
core and any cluster view can still hit it:
ClusteredRoutePolicy.releaseClusterView():239 calls
clusterView.removeEventListener(...) while holding the
policy's lock, and the view calls back into setLeader() → that same lock from
under its read lock. Moving removeEventListener outside the lock.lock() block
would fix it at the source for every implementation.
> Camel Zookeeper Cluster Split brain issue when leader is isolated
> -----------------------------------------------------------------
>
> Key: CAMEL-24457
> URL: https://issues.apache.org/jira/browse/CAMEL-24457
> Project: Camel
> Issue Type: Bug
> Components: camel-zookeeper, camel-zookeeper-master
> Affects Versions: 4.18.3
> Environment: 3 Linux RHEL machines
> All 3 has ZK on it
> And 2 Has the the camel application in cluster mode Active/Passive
> Reporter: mustafa kamal
> Priority: Major
> Labels: bug, split-brain
> Fix For: 4.22.1, 4.23.0, 4.18.5
>
>
> ------------------------------------------------------------------------------------------------------------------
> Problem Description:
> ------------------------------------------------------------------------------------------------------------------
> When the ZooKeeper cluster becomes unavailable, the Camel route managed by
> "ClusteredRoutePolicy" never stops, even though the node has lost its
> ZooKeeper session. Beyond the route not stopping, this also introduces a
> split-brain scenario: if one node becomes isolated from ZooKeeper while
> others remain connected, a new leader election runs on the healthy side and a
> second node wins leadership and starts the same route — now two nodes are
> running the same route simultaneously with no coordination, which can cause
> data corruption, duplicate processing, or conflicting writes depending on
> what the route does.
>
> ------------------------------------------------------------------------------------------------------------------
> Root Cause:
> ------------------------------------------------------------------------------------------------------------------
> The root cause is a timing issue between two threads in
> "ZooKeeperClusterView.CamelLeaderElectionListener.takeLeadership()". When
> ZooKeeper goes down, Curator's "ConnectionStateManager" thread fires a
> "SUSPENDED"/"LOST" state change, which causes "LeaderSelectorListenerAdapter"
> to interrupt the thread blocked inside "takeLeadership()". The "BlockingTask"
> handles the interrupt correctly and exits its loop. Execution then reaches
> the leadership-lost event at:
>
> fireLeadershipChangedEvent(getLeader().orElse(null)); //
> ZooKeeperClusterView.java line 155
>
> This fires "ClusteredRoutePolicy.leadershipChanged()", which, regardless of
> the argument passed, always calls back into:
> setLeader(clusterView.getLocalMember().isLeader()); //
> ClusteredRoutePolicy.java line 376
>
> Which resolves to: leaderSelector.hasLeadership() //
> CuratorLocalMember.isLeader(), line 162
> This returns "true" at this point because Curator's guarantees that
> "hasLeadership()" only becomes "false" after "takeLeadership()" returns to
> the "LeaderSelector" internals. The event fires from inside
> "takeLeadership()", so the answer is always "true", "ClusteredRoutePolicy"
> sees no leadership change, and the route keeps running indefinitely.
>
> ------------------------------------------------------------------------------------------------------------------
> Consequences:
> ------------------------------------------------------------------------------------------------------------------
> The consequence is:
> - The [isolated node] keeps running the route because "hasLeadership()" is
> still "true" at the moment the event fires
> - The [healthy side] elects a new leader, which also starts the same route
> - Both nodes now process the same workload simultaneously with no mutual
> exclusion — a classic [split-brain]
>
>
> ------------------------------------------------------------------------------------------------------------------
> Solution:
> ------------------------------------------------------------------------------------------------------------------
> Introduce a "volatile boolean leader" flag inside "ZooKeeperClusterView" that
> is owned and controlled by the view itself, rather than delegating to
> "leaderSelector.hasLeadership()".
> Change "CuratorLocalMember.isLeader()" to return this flag instead.
> In "takeLeadership()", set the flag to "true" before firing the
> leadership-gained event, and in the "finally" block set it to "false" before
> firing the leadership-lost event. This guarantees that when
> "ClusteredRoutePolicy" calls back into "isLeader()" during the event, it
> reads "false", which causes "stopManagedRoutes()" to be called and the route
> to stop correctly before any other node can win the election and start it.
> Additionally, "leaderSelector.autoRequeue()" should be called in "doStart()"
> so that after losing leadership due to a ZooKeeper disconnect, the node
> automatically re-enters the election when ZooKeeper reconnects and the route
> can start again on whichever node wins.
>
> - "ZooKeeperClusterView": add "volatile boolean leader" field
> - "ZooKeeperClusterView.CuratorLocalMember.isLeader()": return "leader" flag
> instead of "leaderSelector.hasLeadership()"
> - "ZooKeeperClusterView.CamelLeaderElectionListener.takeLeadership()": set
> "leader = true" on entry, wrap task in "try/finally", set "leader = false"
> before firing the lost event
> - "ZooKeeperClusterView.doStart()": add "leaderSelector.autoRequeue()" to
> re-enter election after reconnect
>
> ------------------------------------------------------------------------------------------------------------------
> How to reproduce:
> ------------------------------------------------------------------------------------------------------------------
> 1. Start two instances of a Camel application using "ZooKeeperClusterService"
> with "ClusteredRouteController"
> 2. Confirm one instance is leader and its route is running
> 3. Isolate the leader node from all ZooKeeper nodes (e.g. firewall rules or
> kill ZK nodes)
> 4. Observe on the healthy side: a new leader is elected and its route starts
> 5. Observe on the isolated node: the route never stops — both nodes are now
> running the same route simultaneously (split-brain)
> 6. Expected: the isolated node's route stops as soon as its ZooKeeper session
> is lost
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
