Repository navigation
[Router] Vet a peer's snapshot before it may touch the tree (6/13) - #40692
Merged
Merged
Conversation
This was referenced Sep 22, 2026
Kangyan-Zhou
added this pull request to stack #40701
September 22, 2026 06:31
Kangyan-Zhou
force-pushed
the
router-peer-bootstrap-6-vet-snapshot
branch
from
September 27, 2026 02:28
ad8fc9f to
f1245e2
Compare
Kangyan-Zhou
force-pushed
the
router-peer-bootstrap-6-vet-snapshot
branch
from
September 27, 2026 06:15
f1245e2 to
143a0c3
Compare
Kangyan-Zhou
force-pushed
the
router-peer-bootstrap-6-vet-snapshot
branch
from
September 27, 2026 07:04
143a0c3 to
2ebf73e
Compare
Kangyan-Zhou
force-pushed
the
router-peer-bootstrap-6-vet-snapshot
branch
from
September 27, 2026 09:06
2ebf73e to
c03eb68
Compare
ShangmingCai
force-pushed
the
router-peer-bootstrap-6-vet-snapshot
branch
4 times, most recently
from
September 29, 2026 10:56
426e125 to
05d3a4c
Compare
Base automatically changed from
router-peer-bootstrap-5-tracker-fetch
to
main
September 29, 2026 11:06
The last piece of the consumer's half of the wire contract, still with no consumer: the type that turns a peer's `PeerSnapshot` into something graftable. The pump that uses it lands next. The trust boundary is enforced by types rather than by discipline. A snapshot arrives over the network as `PeerSnapshot`, carrying `WireWorker` rather than `KvWorkerId`; `VettedSnapshot::from_wire` is the only bridge, and it resolves every wire identity against the locally discovered worker set, so a peer cannot introduce a `KvWorkerId` the local registry does not already know — which would otherwise let routing resolve to a non-registered endpoint. `VettedSnapshot`'s fields are private and `graft_into` is the only route to the tree's restore path from outside the module, so format, block-size and parent-bound checks cannot be skipped by a caller that assembles nodes itself. That is also what retires `restore_snapshot`'s `#[allow(dead_code)]`. Vetting rejects rather than repairs: an unknown format, a producer that is itself cold or not ready, an out-of-range or forward parent reference, tiers that do not pair with workers, a block-size mismatch, or a snapshot that prunes to nothing. A rank whose snapshot is rejected runs cold, which is slow and visible; a snapshot grafted on a hope can hide a missed `BlockRemoved` as a permanent false cache hit, which is silent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JBP3reyKKk4TPeZuppMPmW
ShangmingCai
force-pushed
the
router-peer-bootstrap-6-vet-snapshot
branch
from
September 29, 2026 11:06
05d3a4c to
4f0afb4
Compare
ShangmingCai
marked this pull request as ready for review
September 29, 2026 11:06
ShangmingCai
approved these changes
Sep 29, 2026
ShangmingCai
left a comment
Collaborator
There was a problem hiding this comment.
This one is also very straightforward and it looks correct to me.
Collaborator
|
/tag-and-rerun-ci |
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.
Stack 6 of 13. Base:
router-peer-bootstrap-5-tracker-fetch. 5 files changed, 915 insertions(+), 18 deletions(-)The stack
export_snapshot/restore_snapshot; nothing calls them yetGET /internal/kv_snapshot— the producer half; nothing consumes it yet--kv-peer-selector, the peer registry, the_peersgauges; no behaviour changeBootstrapTracker+ the fetch client; the consumer's state and transportVettedSnapshot::from_wire— the only bridge from wire bytes to the tree ← this PRPendingrank's batches and grafts a snapshot handed to itmatch_prefixanswers as the source/readyzholds until bootstrap settles;--kv-bootstrap-seed-required; the metricsEach PR's base is the branch below it, so every diff shown here is that
PR's own change. Review bottom-up; GitHub retargets each child to
mainasits parent merges.
What the series does
A router replica subscribes to each worker's KV topic mid-stream, so every
block already resident in the engine's radix cache is invisible to it — and
engines publish
BlockStoredonly as they insert, so a prefix cached hoursago is never re-announced. A cache-blind replica then scatters the prefixes the
warm replicas were keeping consolidated, degrading the engines' locality for
the whole fleet; a rolling update does that to every replica in turn. This
series makes a booting replica pull a tree snapshot from a warm sibling over
HTTP and graft it beneath its live delta stream. Off unless
--kv-peer-selectoris set.
Supersedes #39750, which carried the same work as one branch on a stale base.
What this change does
The last piece of the consumer's half of the wire contract, still with no
consumer: the type that turns a peer's
PeerSnapshotinto something graftable.The pump that uses it lands next.
The trust boundary is enforced by types rather than by discipline. A snapshot
arrives over the network as
PeerSnapshot, carryingWireWorkerrather thanKvWorkerId;VettedSnapshot::from_wireis the only bridge, and it resolvesevery wire identity against the locally discovered worker set, so a peer cannot
introduce a
KvWorkerIdthe local registry does not already know — which wouldotherwise let routing resolve to a non-registered endpoint.
VettedSnapshot'sfields are private and
graft_intois the only route to the tree's restore pathfrom outside
state::kv_events, so format, block-size and parent-bound checks cannot beskipped by a caller that assembles nodes itself. That is also what retires
restore_snapshot's#[allow(dead_code)].Vetting rejects rather than repairs: an unknown format, a producer that is
itself cold or not ready, an out-of-range or forward parent reference, tiers
that do not pair with workers, a block-size mismatch, two cursors for one
worker or two worker-table entries naming the same live worker, or a snapshot
that prunes to nothing. Pruning happens at vetting, not at restore: carriers of
workers this replica does not know and carriers held only on tiers this build
does not rank are dropped, and every node left carrying nothing with no
carrying descendant is pruned, so a carrier-less leaf never turns a real hit
into a match with no workers. A rank whose snapshot is rejected runs cold, which is slow and
visible; a snapshot grafted on a hope can hide a missed
BlockRemovedas apermanent false cache hit, which is silent.
Tests
cargo fmt --check,cargo clippy --all-targets -- -D warnings, and the lib +component + proxy suites all pass on this branch on its own, not only on the
tip of the stack.
Review pass
Reviewed with
/code-review --fixand/simplify; fixes were folded into this PR's own commit and the stack was re-verified tier by tier (cargo fmt --check,cargo clippy --all-targets -D warnings, lib + component + proxy tests on every branch).state/kv_events/bootstrap/vet.rs(new), plustree/snapshot.rs; no file the stack creates exceeds ~1,300 lines.🤖 Generated with Claude Code
https://claude.ai/code/session_016HmJvHV7QDPk3qAjQYzthd
CI States
Latest PR Test (Base): ✅ Run #36559683560
Latest PR Test (Extra): ❌ Run #36559682954
Latest PR Test (AMD ROCm 10): ➖ No AMD PR run found for this commit.