Shard manager Leader Election Implementation

[Aditya1404Sal]

resolves GOL-447

Make the shard manager elect a single leader in distributed mode and fence its state writes on the leadership

Summary

In etcd mode any number of shard manager replicas can now run. Exactly one wins an etcd lease campaign and drives all topology decisions; the rest wait without opening their gRPC port. Every write of the shard lease state carries a second precondition, that the leader's election key still exists at the creation revision its campaign returned, so a replica that has lost the leadership cannot overwrite the new leader's work even when its cached revision is still current. Losing the lease stops the process; stopping the process releases the lease; a graceful stop hands over in milliseconds.

Local mode (Postgres, SQLite) is unchanged apart from two things that apply to both modes: the configured shard count is checked against the stored one at startup instead of being silently ignored, and a shutdown signal is honoured during startup.

Before this change the shard manager was a documented singleton enforced nowhere, and #3790's compare-and-swap detected lost updates without preventing two leaders.

Changes

Election (sharding/leader_election.rs, new)

• LeaderElection campaigns through etcd's server-side Election API on its own untimed client. The lease keepalive runs concurrently with the campaign in one biased select, together with the shutdown token and a periodic "still standing by" announcement.
• Lease loss during a campaign is retriable (a standby holds nothing); a won campaign is verified against etcd before it is accepted, because etcd's Campaign returns when the key ahead is deleted and never re-checks the campaigner's own key; every failure after a lease grant revokes it through one path.
• LeaseKeepAlive::run(self) -> LeaseLost has no success case. It reconnects the stream on transport errors; only etcd reporting the lease gone, or a full TTL with no acknowledged renewal, ends a leader. The watchdog is anchored to the last renewal etcd acknowledged, by send time, so restarting the renewal loop cannot extend the belief window.
• LeaderFence compares create_revision of the leader key, which is what LeaderKey::rev() exists for. LeadershipHandle::step_down revokes the lease; a lease etcd no longer knows counts as released.

Connections and retries (sharding/etcd_connection.rs, sharding/etcd_retry.rs, new)

• Two etcd clients. request_timeout becomes a tonic channel-level timeout, and Election.Campaign is a unary RPC etcd does not answer until you win, so the election client has no channel timeout and bounds every non-campaign RPC per call instead.
• Idempotent reads (the pre-campaign shard-count read, the state load, the post-win confirmation) retry transient errors with backoff, cancellable by shutdown. Writes and lease RPCs stay single-shot.

Persistence (sharding/persistence/etcd.rs)

• The write transaction has two compares, the fence and the revision, and an else-branch read that tells LeadershipLost apart from ConcurrentModification. The fence is a required constructor argument; forgetting it is a compile error. with_client lets run() open the KV client before campaigning; stored_number_of_shards reads the stored count without a fence.

Startup and shutdown (lib.rs, server.rs)

• run() takes a Deployment: Standalone { shutdown } may block until elected and carries the shutdown token; Embedded refuses etcd mode, because the single binary awaits run() inline and a campaign there would hang the whole process silently.
• In etcd mode everything that can fail on shared configuration runs before the campaign: TTL and timeout validation, the health checker, the KV client, the stored shard-count check. Anything failing after the campaign would exit holding the lease and crash-loop the whole deployment at TTL cadence.
• Startup after winning is raced against the shutdown token. The drain loop moved into serve_until_stopped: on a task failure, on all tasks finishing, and on shutdown it aborts the remaining tasks and then releases the lease, in that order, so no executor command can follow the deletion of the election key. A failed release is a warning, never an exit code. server.rs handles SIGTERM and Ctrl-C and exits 0; a standby told to stop leaves the election queue at once.
• The initial executor health check on a new leader has a per-RPC deadline and a fifteen-second cap; executors that have not answered by then are left to the periodic loop.
• mutate_and_persist mutates a clone and swaps it in only after the write is durable, so a caller dropped mid-persist leaves memory untouched.

Configuration, metrics, docs

• persistence.config.leader_lease_ttl (default 10s, whole seconds, at least 2s). Startup refuses request_timeout above half the TTL.
• shard_manager_is_leader, shard_manager_leader_since_epoch, shard_manager_campaign_attempt_failures_total.
• deploy.mdx describes the two modes, probes (liveness on HTTP, readiness on gRPC), failover bounds, the plaintext unauthenticated etcd requirement, the single-endpoint recommendation, and the registration caveat below.
• golem-common: serialization versions 1 and 2 return an error instead of panicking.

Deviations from the spec, and why

1. Two etcd clients, not one shared client. Verified from etcd-client 0.19 and tonic: the request timeout is applied to the whole channel. A shared client aborts every standby's campaign after request_timeout.
2. Keepalive during the campaign, not after it. etcd's election server builds a session from the caller's lease and orphans it immediately; nothing server-side renews a campaigner's lease. Campaign-then-keepalive lets a standby's lease expire mid-wait, after which it can never be elected, with no error.
3. etcd's Election API, not the epic's hand-rolled IF NOT EXISTS plus watch. The hand-rolled form makes every loser watch one key (a thundering herd on failover) and has a window where a replica can miss the delete between its failed txn and its new watch. The Election API is a FIFO by creation revision and returns the create_revision the fence needs.
4. Standbys idle and never bind gRPC; no NotLeader response, no proto change. GetRoutingTable reads a process-local snapshot loaded once, and a stale snapshot silently truncates the table rather than erroring. Serving that from a standby is worse than refusing. The closed gRPC port is the readiness signal.
5. No loop-top is_alive() check. It is a cached belief that can disagree with etcd in both directions; the worker performs a fenced write before it touches any executor, so a demoted leader fails before issuing a single RPC.
6. Fence on create_revision, not on the lease id. Strictly stronger at the same cost: the key path already encodes the lease id, and creation revisions are monotonic, so a recreated key can never collide.
7. The shard-count guard applies to local mode too. The stored value governs routing; a configuration that disagrees with it was previously ignored silently. This is a behaviour change for existing Postgres deployments whose configured count drifted, and belongs in the release notes.
8. etcd auth and TLS stay unsupported; non-http:// endpoints are rejected with a message naming the gap.

Decisions made during review

• Fail-stop stays on the write path and the lease path; transient transport errors are not retried there. With a multi-member endpoint list tonic's balancer keeps a dead member in rotation (measured: five or six of twenty single-shot reads failing), which is why the docs recommend one load-balanced address rather than reopening that decision.
• The keepalive reconnects, with the watchdog as its only bound, and does not re-verify the election key after a reconnect.
• On an initial health-check timeout the unknown executors are treated as healthy; removing them would reshard a cluster over slow probes.
• The catch-the-panic guard around the state decoder was removed: the workspace builds with panic = "abort", so it only appeared to work under cargo test.
• A retry on the step-down revoke was tried and reverted: it broke the black-hole watchdog test's timing bound and cannot help into a black hole.

Adversarial review: what was found and what was done

Two rounds. The first was a four-agent sweep of the change set; the second was a 127-agent exploration of the election and lease boundary (eight angles, three-lens refutation of every finding, a completeness critic) followed by sequential remediation with an independent reviewer per item. Fifteen findings survived refutation; twenty-three were refuted.

Finding	Resolution
Lease loss mid-campaign was wrapped as a non-retriable error, so an etcd blip exited every standby	Retriable variant; test revokes a standby's lease mid-wait
A ? after a won campaign exited holding the election key, blocking every replica for a TTL	Single revoke covering every failure after the grant
Unbounded awaits on the untimed election channel (grant, handshake, revoke, leader read)	Bounded per call by request_timeout
A campaign won on an already-dead lease was accepted with a fence over a deleted key	Post-win liveness read; re-campaign on a dead key
The standby log could only fire after a campaign error, never while healthily waiting	Announcement is a never-resolving select arm
Three exit paths (keepalive loss, task failure, post-election startup failure) exited without releasing the lease	serve_until_stopped; release on every path, abort before revoke
A broken keepalive stream counted as a lost lease, so every etcd member restart bounced the leader	Reconnect on transport errors
The watchdog deadline was re-based on every entry to the renewal loop	Anchored to the last acknowledged renewal by send time
The executor health RPC had no deadline; a silent executor pinned a new leader forever with its port closed	Per-RPC deadline and a capped initial fan-out
The campaign retry loop never observed shutdown, so a standby with etcd unreachable ignored SIGTERM until killed	Whole loop raced against the token
Single-shot etcd reads failed startup or discarded a won leadership on a blip	Retries on idempotent reads
Post-election checks ran while holding the lease, crash-looping a deployment on shared misconfiguration	Health checker, KV client and shard-count check hoisted before the campaign
step_down errors made a graceful exit non-zero; gauges were sticky; a failed revoke was discarded silently; request_timeout unordered against the TTL	Warn instead of propagate; gauges reset; warning on revoke failure; startup rule
The state decoder panicked on dropped versions and on a malformed length	Versions return an error; the malformed length is characterised (see limitations)
mutate_and_persist rolled back on error, which cannot run for a caller dropped mid-persist	Mutate a clone, swap on success
No test distinguished the create-revision compare from a key-existence check	Two pure fence tests; the weakened compare reddens exactly one of them

Tests

97 unit tests and 101 integration tests; 60 are new. Every new or changed test was proven by a mutation: the guarded code was broken, the test went red with its assertion quoted, the mutation was reverted, and the test went green again. The etcd-backed modules live under tests/etcd_backed/ as one sequential suite because they share a fixed state key; tests/etcd_backed/proxy.rs is a test-local TCP proxy that can drop connections (an etcd restart) or silently swallow bytes (a black hole). Integration tests need Docker.

Not covered in-process: the signal-to-token translation in server.rs, and anything needing a spawned shard manager pointed at etcd, which golem-test-framework cannot do yet.

Known limitations, each with its home

• A Register acknowledged just before the leader steps down or loses its lease may never be persisted, and the executor does not retry. Characterised by a test; ticket 4 redesigns the Register write path.
• A demoted leader can still complete executor commands already in flight. Closing that needs a leadership term on the executor wire: ticket 5.
• A malformed length inside a stored blob aborts the process, in any mode. The unchecked addition is in desert_core::read_bytes; the fix belongs in desert-rust.
• A demoted leader serves reads for up to one renewal interval after a successor is elected; the fence blocks its writes. Documented.
• Quota operations fail in etcd mode: ticket 7.

🤖 Generated with Claude Code

[netlify]

✅ Deploy Preview for golemcloud canceled.

Name	Link
🔨 Latest commit	813903a
🔍 Latest deploy log	https://app.netlify.com/projects/golemcloud/deploys/6a9affba171ec50008b6ee87

[vigoo]

abort_all() only requests cancellation; it does not wait for the tasks to stop. A task that is already running can continue while release_leadership() revokes the lease, and could send an executor command after another replica becomes leader. Please use join_set.shutdown().await (or otherwise wait for every task to stop) before releasing leadership. The startup-error path above has the same issue.

[vigoo]

The watchdog is armed when this response is received, but etcd starts the renewed TTL when it processes the keepalive, before sending the response. Any response delay is therefore added to the local deadline. If later renewal traffic is lost, this replica can keep its gRPC port open after etcd has expired the lease and elected a successor. Please capture the time before calling keep_alive() and arm the watchdog from that, as subsequent renewals already do.

[vigoo]

Cancellation here drops campaign_loop() and campaign_once() directly. If etcd has already completed the campaign, or the confirming read is still running, the cleanup revoke in campaign_once() never runs. Etcd then keeps the winning key until the lease expires, so a graceful shutdown can block the next replica for a full TTL. Please make shutdown go through the cleanup path, or otherwise revoke a granted lease before returning.

[vigoo]

LeaseClient::keep_alive() sends an initial renewal and waits for a positive-TTL response before returning. On a successful reconnect, this replaces the stream but leaves the watchdog's old deadline unchanged. If reconnect succeeds near that deadline, drive() can immediately report RenewalDeadlineExceeded even though etcd just renewed the lease for a full TTL. Please record the reconnect attempt's send time and re-arm the watchdog from it.

[vigoo]

This check runs after ShardManagement::new(), which has already health-checked executors, spawned the worker, and called notify_one(). On the multi-thread runtime, the worker can run first and persist changes, remove executors, or contact them before this mismatch is rejected. Please validate the stored shard count immediately after reading persistence and before starting work with that state.