aho135 commented on issue #20339:
URL: https://github.com/apache/druid/issues/20339#issuecomment-5672198729
Adding a diagram to make the race easier to follow. The crux is that **two
threads touch the same `WorkerHolder`**: the single-threaded `LeaderSelector`
running `becomeLeader()`, and the Curator discovery-listener thread running
`removeWorker()`/`addWorker()`.
```mermaid
flowchart TD
A["ZooKeeper leader restarts<br/>Curator SUSPENDED →
RECONNECTED<br/>Overlord leadership moves to new node"] --> B
subgraph T1["LeaderSelector thread (single-threaded, synchronous)"]
direction TB
B["isLeader() → listener.becomeLeader()"] -->
C["DruidOverlord.becomeLeader()<br/>build 'task-master' Lifecycle<br/>start
HttpRemoteTaskRunner FIRST (before SupervisorManager)"]
C --> D["HttpRemoteTaskRunner.startWorkersHandling()<br/>discover
workers → create WorkerHolder A for MM-X<br/>start all syncers"]
D --> E["loop: for (WorkerHolder w :
workers.values())<br/>w.waitForInitialization()<br/>(holds reference to holder
A)"]
E --> F["reach MM-X → holderA.awaitInitialization()<br/>await latch up
to 3 × syncRequestTimeout"]
F --> G{"holder A latch<br/>counts down?"}
G -->|"NO — A was stopped pre-init (see race)"| H["block full 3 × PT3M
= 9 min"]
H --> I["awaitInitialization() = false<br/>throw RE 'Failed to sync
with worker[MM-X]'"]
I --> J["becomeLeader() throws"]
end
subgraph T2["Curator discovery-listener thread (concurrent, ~1s later)"]
direction TB
K["MM-X ZK session reset →<br/>ephemeral node removed"] -->
L["nodesRemoved →
removeWorker(MM-X)<br/>workers.remove(MM-X)<br/>holderA.stop() → syncer.stop()"]
L --> M["stop() does NOT count
down<br/>holderA.initializationLatch<br/>(stuck at 1 forever)"]
M --> N["MM-X re-announces node →<br/>nodesAdded → addWorker(MM-X)"]
N --> O["create holder B → holderB.start()<br/>workers.put(MM-X, B)"]
O --> P["holder B syncs fine<br/>(latch B counts down, streams tasks)"]
end
D -. "loop captured ref to A" .-> E
M == "A's latch never fires ⇒ this is why G = NO" ==> G
P -. "B is healthy but the loop is awaiting A, not B" .-> F
J --> Q["CuratorDruidLeaderSelector.isLeader()
catch:<br/>'becomeLeader() failed' → notLeader()"]
Q --> R["leadership released → another node becomes leader<br/>→ mass
supervisor/task restart → ingestion-lag spike"]
classDef bug fill:#ffdddd,stroke:#cc0000,stroke-width:2px,color:#000;
classDef ok fill:#ddffdd,stroke:#22aa22,color:#000;
class M,H,I bug;
class P ok;
```
**In one sentence:** the startup loop grabs a reference to `WorkerHolder A`
for MM-X, but a split second later the discovery thread `removeWorker`s MM-X —
calling `holderA.stop()`, which stops the syncer *without* counting down its
`initializationLatch` — and creates a fresh healthy `holder B`; the loop keeps
awaiting the now-dead **A**, whose latch can never fire, so it blocks the full
`3 × syncRequestTimeout` (9 min) and fails `becomeLeader()`.
Two conditions must coincide for the hang:
1. **Ordering** — MM-X's remove+re-add lands after `startWorkersHandling()`
captured holder A but while the loop still awaits it.
2. **Latch leak** — `WorkerHolder.stop()` / `ChangeRequestHttpSyncer.stop()`
never release `initializationLatch`, so a stopped-pre-init holder is
indistinguishable from a merely-slow one and the awaiter waits the full timeout.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
