shashank created CAMEL-26571:
--------------------------------
Summary: camel-consul - ConsulClusterView can be leader after it
was stopped, and a view stopped while Consul cannot be reached never takes part
in the leader election again
Key: CAMEL-26571
URL: https://issues.apache.org/jira/browse/CAMEL-26571
Project: Camel
Issue Type: Bug
Components: camel-consul
Reporter: shashank
Assignee: shashank
Found with a TLA+ model of {{ConsulClusterView}} (two nodes, each with its
lifecycle thread and the Consul client thread that handles the answers of the
blocking query; Consul answers, session expiry and failing calls chosen
nondeterministically), then reproduced on the real classes with mocked Consul
clients (no Consul).
A stopped view must not be leader, and a view that is started again must take
part in the leader election with a new session. Three ways break this:
- *A stop or a failed start while Consul cannot be reached.* {{doStop}} calls
{{releaseLock}} and then {{destroySession}}; if one of them throws,
{{setMaster(false)}} is skipped and the stop fails. If {{releaseLock}} throws,
the session id is also kept, so a later start does nothing ({{doStart}} only
runs when there is no session id): no new session, no watch, and the local
member keeps the leadership it had. Its old session is not renewed anymore,
Consul invalidates it after its TTL and another node takes the lock, so two
nodes are leader for good. A start that fails after it created the session is
stopped by {{BaseService}}, with the same result. For example a {{master:}}
route stopped and started again while the Consul agent restarts: the restarted
view still says leader, {{MasterConsumer}} starts its consumer, and the
consumer of the new leader runs as well. A view that was not leader never
becomes leader again.
- *An answer handled while the view stops.* {{Watcher.onComplete}} checks
{{isStarting() || isStarted()}}; when the key is free it then acquires the lock
(under {{sessionIdLock}}) and calls {{setMaster}} after releasing
{{sessionIdLock}}. A {{doStop}} that waited for {{sessionIdLock}} destroys the
session and calls {{setMaster(false)}} in between, and the callback then sets
the leadership: the stopped view is leader and fires a leadership event. After
a forced stop of the view (JMX {{stopView}}) the clustered routes of a
{{ClusteredRoutePolicy}} start on this node, while another node takes the lock
that was released.
- *An answer to a query of an earlier start.* The check only looks at the
status, so an answer that is handled after the view was stopped and started
again is applied to the new session (a stale "held by the old session" turns
the leader of the new session into a follower until the next answer; a late
failure of the old query does the same), and the callback queries again: one
more watch, and one more session renewal every interval, for each quick stop
and start.
Short traces of the model:
{noformat}
stop while Consul cannot be reached answer handled while the view stops
1 n1 start: leader with session s1 1 n2 held the lock and released
it; n1 Consul thread:
2 n1 doStop: releaseLock throws (L131) answer "key free", running
(L305, L309)
-> stop fails, sessionId = s1, leader 2 n1 doStop: releaseLock (nothing
held, L131), waits
3 n1 start: sessionId != null -> no-op (L98) for sessionIdLock (L134)
4 s1 not renewed, expires; n2 takes the lock 3 n1 Consul thread: acquireLock ->
true (L312)
-> n1 and n2 leader 4 n1 doStop: destroySession(s1),
setMaster(false)
(L136-137)
5 n1 Consul thread:
setMaster(true) (L315)
-> stopped view is leader
{noformat}
The real-class tests (mocked {{KeyValueClient}} and {{SessionClient}}) stop the
view while the clients throw, fail a start right after the session was created,
let only the lock release fail, apply the answer and the failure of the query
of the first start after a restart, and stop the view from a hook right after
the watch answer acquired the lock. A probe on the old code shows the stop case
end to end: the restarted view keeps the old session, sends no query and stays
leader, and another node then takes the free lock.
*Proposal:* {{doStop}} releases the lock and destroys the session, each best
effort (a failure is logged; Consul invalidates the session after its TTL) and
always clears the session id and the leadership. A watch answer decides and
sets the leadership under {{sessionIdLock}}, which {{doStop}} now holds for the
release, the destroy and {{setMaster(false)}}, and only if its watch is the one
of the running view: every start creates a new watch, and an answer of an older
one is ignored and does not query again.
Affected: main and 4.23.x (the same code); 4.22.x, 4.18.x and 4.14.x have the
same {{doStop}} and the same check in {{onComplete}}. The fix builds on
CAMEL-25286 (main and 4.23.x only), so the older branches need their own patch.
_Filed with Claude Code on behalf of allthingssecurity._
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
