Anubhav-Roy opened a new issue, #20838:
URL: https://github.com/apache/druid/issues/20838
### Affected Version
Seen on 33.0.0. The same code is on `master`: `CuratorDruidLeaderSelector`
and `DruidOverlord` have the same election behavior.
### Description
Coordinator and Overlord leader election (`CuratorDruidLeaderSelector`, a
Curator `LeaderLatch`) treats "connected to ZooKeeper" as "able to lead". A
node that is alive and connected but can't do its job (for example, because its
disk is full) stays a candidate. It can win leadership, and once it's leader,
nothing makes it give leadership up. Leadership only moves when the leader's
ZooKeeper connection is suspended or lost, so the cluster stays on a broken
leader until someone intervenes manually.
In our case this caused a multi-hour Overlord outage: every task submission
failed, while the healthy Overlord sat idle as a candidate.
**What happened**
1. The root disk of one master filled up completely (0 bytes free). The
Druid processes on that node kept running and their ZooKeeper sessions stayed
alive, so the node stayed in both elections. Its log output stopped because the
disk was full.
2. About 14 hours later, the ZooKeeper nodes were replaced. The healthy
Overlord leader's connection was suspended and its session timed out:
```
WARN [main-SendThread(...)] org.apache.zookeeper.ClientCnxn - Session
0x... for server ..., Closing socket connection. Attempting reconnect except it
is a SessionExpiredException.
org.apache.zookeeper.ClientCnxn$SessionTimeoutException: Client session
timed out, have not heard from server in 26672ms for session id 0x...
INFO [LeaderSelector[/druid/overlord/_OVERLORD]]
org.apache.druid.indexing.overlord.DruidOverlord - I am no longer the leader...
```
3. `notLeader()` closed the healthy node's latch, waited 1–5 s in
`startLeaderLatch()`, and rejoined at the back of the queue. The only other
candidate, the Overlord on the full-disk node, became leader. `becomeLeader()`
succeeded because starting the leader lifecycle doesn't need disk. Clients then
used it as the leader. For example, peons sent task status to it:
```
ServiceClientImpl - Service [overlord] request [POST
https://<full-disk-node>:8290/druid/indexer/v1/taskStatus] encountered
exception on attempt #1; ...
```
4. That Overlord couldn't do its job. Task submissions forwarded by the
Routers to the leader failed with `Error 502 Bad Gateway`
The Routers kept passing `/status/health`, so the load balancer kept
sending them traffic.
5. Leadership never moved back to the healthy Overlord. The broken leader's
ZooKeeper session stayed healthy the whole time, and nothing else can trigger a
change. The outage ended only when both masters were replaced manually.
**Where it comes from in the code**
- `CuratorDruidLeaderSelector.registerListener()` joins the latch
unconditionally. Any process with a live ZooKeeper session is a candidate.
- `isLeader()` only checks the latch state before calling
`listener.becomeLeader()`. Nothing checks whether the node can lead.
- Leadership is only given up from `notLeader()`, which Curator calls when
the connection is `SUSPENDED` or `LOST` (Druid uses Curator's default
`StandardConnectionStateErrorPolicy`). A leader that stays connected is never
asked to give leadership up, however broken it is.
- After `notLeader()`, the healthy node deliberately rejoins at the back of
the queue, which hands leadership to the next candidate even if that candidate
is broken.
**Expected behavior**
A node that can't do its job shouldn't take leadership while a healthy
candidate exists, and a leader that becomes unable to do its job should give
leadership to a healthy candidate.
### Related
These issues share the same underlying gap: a node that holds leadership, or
is elected, isn't necessarily able to do the leader's job, and election doesn't
take that into account.
- #20339: Overlord `becomeLeader()` blocks for 3 ×
`druid.indexer.runner.syncRequestTimeout` and fails, leaving the cluster
effectively without a leader in the meantime.
- #14387: unreachable workers can prevent the Overlord from becoming leader.
- #17807: after a ZooKeeper restart, the Coordinator is leader but stops
assigning segments.
- #13251 (closed, not merged): proposed a third leader-selector state,
"initialized leader", for callers to gate on.
- #17546: separated stopping and restarting the latch in `notLeader()`, so a
node that is stepping down can't regain leadership while it's still stopping.
The proposed fix keeps this behavior.
### Proposed fix
I have a patch and can open a PR. It's off by default, so behavior doesn't
change unless it's enabled.
- A pluggable `LeadershipHealthCheck` interface (`@Nullable String
getUnhealthyReason()`), used by `CuratorDruidLeaderSelector` for both the
Coordinator and the Overlord.
- A built-in `DiskSpaceLeadershipHealthCheck`. A node is unhealthy if any
configured directory is missing, has less than a minimum percentage of free
space, or can't be written to (a small probe file is written with `SYNC` and
then deleted).
- In `CuratorDruidLeaderSelector`, when a health check is configured:
- **When chosen as leader:** an unhealthy node yields, by closing its
latch, if another candidate exists.
- **While leader:** after `failureThreshold` consecutive failed checks,
the leader gives up leadership if another candidate exists.
- A node that yielded or gave up leadership rejoins the election once it
passes the check again.
- **Never leaderless:** an unhealthy node still becomes or stays leader if
it's the only candidate, and rejoins if no node holds leadership, emitting an
alert in both cases.
- A health check that throws counts as unhealthy. Leadership state changes
run on the selector's existing listener executor.
- New configuration under `druid.leaderSelector.healthCheck`: `enabled`
(default `false`), `paths` (default `java.io.tmpdir`), `minFreeDiskPercent`
(`5.0`), `writeProbe` (`true`), `period` (`PT30S`), `failureThreshold` (`3`).
Tests:
- `DiskSpaceLeadershipHealthCheckTest` covers the disk check and config.
- `CuratorDruidLeaderSelectorHealthCheckTest`, against a test ZooKeeper,
covers: an unhealthy leader gives up and later rejoins; an unhealthy
next-in-line candidate yields; a sole unhealthy candidate still becomes leader;
an unhealthy leader with no other candidate keeps leadership; a check that
throws counts as unhealthy.
- The existing `CuratorDruidLeaderSelectorTest` passes unchanged.
--
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]
