Repository navigation
Conversation
The production wedge: a ShardStore ring over grpc_store backends hangs EVERY CAS RPC when one peer is dead or restarting, with no error and no timeout — the operational workaround was restarting the frontend. Where the hang lived: ConnectionManager retries a dead endpoint's connect forever in the background (by design — never give up on an endpoint), but connection() requests queue unbounded in waiting_connections while it does, so no error ever reaches the retry/error machinery. rpc_timeout_s was only applied on the ByteStream.Write path; reads and unary RPCs had no deadline at all. The fix, two layers: - connection_manager: track per-endpoint connect health. When the last connect attempt to every endpoint has failed and no channel is available, fail queued and incoming connection() requests fast with Unavailable instead of queuing them forever. A background reconnect success clears the flag and service resumes. - grpc_store: apply the existing rpc_timeout_s knob to every RPC — unary calls, and read-stream establishment through the first response chunk — not just writes. A timeout maps to DeadlineExceeded, which is retryable. Tests (repro first; both failed against the old code by hanging past a generous outer timeout): - connection_manager_test::dead_endpoint_fails_fast_instead_of_queuing_forever - shard_ring_dead_peer_test: an in-process two-shard lab ring (one live fake CAS peer, one dead port) proving reads and multi-key existence checks settle with Unavailable/DeadlineExceeded while the live shard keeps serving. Co-Authored-By: Claude Fable 5 <[email protected]>
Regenerated web/apps/docs/content/docs/reference/nativelink-config/main.mdx from the nativelink-config crate (build-schema + schema-to-mdx), as AGENTS.md requires when a config field or its doc comment changes.
ShardStore routes blobs over a weighted consistent-hash ring built once at construction (cumulative weights, binary search). A dead shard keeps its ring weight forever, so its share of the keyspace (~10% in our fleet) keeps routing reads to a node that answers nothing but Unavailable/DeadlineExceeded; every CAS lookup or fetch for those keys fails (or, before the connection-manager fix, wedged) until an operator restarts the frontend. Fix: read-path failover with per-shard health tracking. - Each shard carries a ShardHealth (AtomicU32 consecutive-failure count + AtomicU64 last-failure instant, nanos since store creation). A shard becomes "suspect" after 3 consecutive connectivity failures (Unavailable/DeadlineExceeded) and is skipped as a first read target for a 5s cooldown, after which it is probed again. Any success resets the counter. - get_part and has_with_results fail over in ring order on a connectivity error from the primary (or skip a suspect primary), trying each shard at most once. get_part only fails over when no bytes have reached the client (writer reborrowed across attempts), and never lets a failover shard's NotFound mask the owning shard's connectivity error. If every shard is suspect, the primary is probed directly rather than failing without touching the network. - The write path (update) is unchanged: writes always target the key's home shard so a flaky peer cannot cause duplicate or misplaced blobs; a failed write errors exactly as before. Tests (nativelink-store/tests/shard_ring_dead_peer_test.rs): reads and existence checks for keys owned by a dead shard now succeed via a live replica, multi-key has() no longer fails wholesale when one bucket's shard is down, and a revived shard serves its own keys again after the suspect cooldown. Co-Authored-By: Claude Opus 4.8 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
fix(store): dead shard-ring peer fails fast instead of wedging every RPC
The production wedge: a ShardStore ring over grpc_store backends hangs
EVERY CAS RPC when one peer is dead or restarting, with no error and no
timeout — the operational workaround was restarting the frontend.
Where the hang lived: ConnectionManager retries a dead endpoint's
connect forever in the background (by design — never give up on an
endpoint), but connection() requests queue unbounded in
waiting_connections while it does, so no error ever reaches the
retry/error machinery. rpc_timeout_s was only applied on the
ByteStream.Write path; reads and unary RPCs had no deadline at all.
The fix, two layers:
connect attempt to every endpoint has failed and no channel is
available, fail queued and incoming connection() requests fast with
Unavailable instead of queuing them forever. A background reconnect
success clears the flag and service resumes.
unary calls, and read-stream establishment through the first response
chunk — not just writes. A timeout maps to DeadlineExceeded, which is
retryable.
fix(store): bounded read failover around dead shards in ShardStore
ShardStore routes blobs over a weighted consistent-hash ring built once
at construction (cumulative weights, binary search). A dead shard keeps
its ring weight forever, so its share of the keyspace (~10% in our
fleet) keeps routing reads to a node that answers nothing but
Unavailable/DeadlineExceeded; every CAS lookup or fetch for those keys
fails (or, before the connection-manager fix, wedged) until an operator
restarts the frontend.
Fix: read-path failover with per-shard health tracking.
count + AtomicU64 last-failure instant, nanos since store creation).
A shard becomes "suspect" after 3 consecutive connectivity failures
(Unavailable/DeadlineExceeded) and is skipped as a first read target
for a 5s cooldown, after which it is probed again. Any success
resets the counter.
connectivity error from the primary (or skip a suspect primary),
trying each shard at most once. get_part only fails over when no
bytes have reached the client (writer reborrowed across attempts),
and never lets a failover shard's NotFound mask the owning shard's
connectivity error. If every shard is suspect, the primary is probed
directly rather than failing without touching the network.
home shard so a flaky peer cannot cause duplicate or misplaced
blobs; a failed write errors exactly as before.
How was this verified?
Tests (repro first; both failed against the old code by hanging past a
generous outer timeout):
fake CAS peer, one dead port) proving reads and multi-key existence
checks settle with Unavailable/DeadlineExceeded while the live shard
keeps serving.
Tests (nativelink-store/tests/shard_ring_dead_peer_test.rs): reads and
existence checks for keys owned by a dead shard now succeed via a live
replica, multi-key has() no longer fails wholesale when one bucket's
shard is down, and a revived shard serves its own keys again after the
suspect cooldown.
Also part of the soak branch (#73): upstream HEAD plus every pending fix, run through the railgun fleet for an hour of continuous remote builds with no errors.
docs(config-reference): regenerate main forrpc_timeout_s``: regenerated withgen:config-reference, never hand-edited.Risk
Stacked on #59 (
pr/shard-ring-dead-peer), whose two commits are included here so CI runs; only the last commit is new. Failover is bounded and only engages for reads that fail with Unavailable/DeadlineExceeded from a shard; healthy rings route as before.AI assistance
Written with Claude Code; b7r6 reviewed the change. Draft until the author has read the CI results.
🤖 Generated with Claude Code