shashank created CAMEL-26540:
--------------------------------
Summary: camel-zookeeper-master - MasterRoutePolicy never retries
a failed consumer start, and starts the consumer after the leadership was lost
Key: CAMEL-26540
URL: https://issues.apache.org/jira/browse/CAMEL-26540
Project: Camel
Issue Type: Bug
Components: camel-zookeeper-master
Reporter: shashank
Assignee: shashank
Found with a TLA+ model of {{MasterRoutePolicy}} (two nodes, each with the
ZooKeeper group thread and the Curator connection thread, ZooKeeper sessions
that are suspended, expire and reconnect), then reproduced on the real classes
with a fake group (no ZooKeeper).
{{MasterRoutePolicy}} starts the route consumer from the {{CHANGED}} group
event, on the group thread, and stops it from {{DISCONNECTED}}, which
{{ZooKeeperGroup}} runs on the Curator connection thread (and on the closing
thread when the group is closed). Nothing orders 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); the consumer is then started. If the session expires, another node
becomes the master and starts its consumer, while this one keeps consuming,
also after it reconnects as a standby, as standby events stop nothing. Two
active consumers until this node is disconnected again.
- A failed start ({{startConsumer}} throws, for example the broker is not
reachable when the node is elected) is only logged, and {{masterConsumer}}
stays 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}} (the
{{zookeeper-master:}} endpoint); the route policy has the same code shape and
was not changed.
Short trace of the model:
{noformat}
1 n1 group thread : groupEvent(CHANGED): connected, isMaster -> true
(ZookeeperGroupListenerSupport:76-83)
2 n1 connection : SUSPENDED -> connected = false, DISCONNECTED
(ZooKeeperGroup:514-523)
3 n1 connection : masterConsumer = false; stopConsumer (not started)
(MasterRoutePolicy:262-270)
4 n1 group thread : masterConsumer.compareAndSet(false, true); startConsumer
(:243-247)
5 n1 session expires; n2 group thread: master -> starts its consumer
-> n1 and n2 both consume
{noformat}
The real-class tests hold the group thread right after its {{isMaster()}} check
and deliver {{DISCONNECTED}} meanwhile: the consumer of the node that lost the
leadership is running afterwards. A consumer whose first start fails is never
started again.
*Proposal:* start and stop the consumer under a lock of the policy; a start
checks again under it that the group is connected and this node is the master
(and that the policy is running); a failed start leaves {{masterConsumer}}
false, goes to the exception handler and is retried after 5 seconds while this
node is the master; a disconnect or a stop cancels the retry. Unlike
{{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. A 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.
Affected: main, 4.23.x, 4.22.x, 4.18.x, 4.14.x (the same code).
_Filed with Claude Code on behalf of allthingssecurity._
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
