[
https://issues.apache.org/jira/browse/CAMEL-26571?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Work on CAMEL-26571 started by shashank.
----------------------------------------
> 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
> Priority: Major
>
> 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)
