Repository navigation
restore: keep the full and incremental snapshot pair matched - #12568
Conversation
┌─ ⚡ PERF · 32f26d1 vs main@11001ff ─────────────────────────────────
│ SUITE BASELINE NEW Δ
│ replay tps, mainnet 14,704 tps 14,709 tps · +0.03%
│ bench tps, localnet 659,177 tps 660,008 tps · +0.13%
│ snapshot load, testnet 4.89 s 4.89 s · -0.02%
│ snapshot load, mainnet 75.64 s 75.22 s · -0.55%
│ epoch boundary, mainnet 144.5 ms 141.6 ms · -2.03%
│ mem total, mainnet 143.84 GiB 143.84 GiB · 0.00%
│ mem total, testnet 81.16 GiB 81.16 GiB · 0.00%
│ clean compile, firedancer 3.21 s 3.20 s · -0.23%
│ binary size, firedancer 58.69 MB 58.69 MB · 0.00%
├─────────────────────────────────────────────────────────────────────
@@ 0 REGRESSIONS · 0 WARNINGS · 0 IMPROVED · 9 NOISE @@
└─────────────────────────────────────────────────────────────────────history · 9 pushes ┌─ HISTORY · Δ vs main, per push, newest first ───────────────────────────────────────────
│ HEAD TPS BENCH SNAP·T SNAP·M EPOCH MEM·M MEM·T COMPILE BINARY
│ 32f26d1 +0.03% +0.13% -0.02% -0.55% -2.03% 0.00% 0.00% -0.23% 0.00%
+│ 2811ed1 +0.61% +1.76% +0.48% +0.64% -0.67% 0.00% 0.00% -1.38% 0.00%
!│ 7f44f35 +0.46% +0.30% -0.75% +0.08% +0.48% 0.00% 0.00% +0.01% 0.00%
│ 4538ef5 +0.09% +0.09% -0.47% +0.54% +0.51% 0.00% 0.00% -0.36% 0.00%
!│ 2451765 -0.27% +0.35% -0.16% +0.27% -0.68% 0.00% 0.00% -1.07% 0.00%
│ 4ea58a4 +0.20% -0.06% -0.05% … +0.60% 0.00% 0.00% +0.63% 0.00%
-│ 54f1521 +1.39% -0.05% +0.09% … … 0.00% 0.00% -0.49% 0.00%
│ 406667d +0.18% +0.41% +0.26% … … 0.00% 0.00% +0.41% 0.00%
!│ 2685ebc +0.39% +0.62% -0.02% … … 0.00% 0.00% -0.46% 0.00%
└───────────────────────────────────────────────────────────────────────────────────────── |
2685ebc to
406667d
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Incorrect snapshot peer state handling can cause wrong-archive requests, restore timeouts from missing fallback entries, and reuse of blacklisted peers.
3 open issues
What changed in this PR
This PR aims to keep full and incremental snapshots matched during restore, including when a server publishes a newer full snapshot while a download is underway.
Changes:
- Track incremental base slots and filter mismatched HTTP resolution results.
- Download incrementals by explicit filename, retain a fallback peer, and adjust pairing when a full snapshot redirects.
- Add regression tests for pairing, fallback selection, and blacklist handling.
| File | Description |
|---|---|
src/discof/restore/utils/test_http_resolver.c |
Tests rejection of an incremental based on an older full. |
src/discof/restore/utils/fd_http_resolver.c |
Tracks incremental base slots and filters mismatched pairs. |
src/discof/restore/test_snapct_tile.c |
Tests named downloads, fallback peers, blacklist handling, and full-slot changes. |
src/discof/restore/fd_snapct_tile.c |
Adds remembered peers, explicit incremental filenames, and full-slot pairing updates. |
Review effort: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
The resolver refresh preserves incompatible cached incremental data, allowing nonexistent snapshot requests and blacklisting healthy peers; it needs explicit cache clearing and a regression test covering successive resolution cycles.
No open issues 🎉
3 resolved since last review
Review effort: Balanced
54f1521 to
4ea58a4
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Refreshes can retain stale incremental snapshot state and request nonexistent archives, while fallback selection can reuse temporarily invalidated peers and permanently blacklist them after a failed download.
1 open issue
Review effort: Balanced
4ea58a4 to
2451765
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Resolver refreshes can retain stale incremental metadata and trigger invalid downloads, while peer recovery can ignore a usable remembered source and cause restore timeouts.
1 open issue
1 resolved since last review
Review effort: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
Resolver refreshes can retain an incremental snapshot tied to an old full snapshot, causing requests for nonexistent snapshot files and erroneous peer blacklisting.
No open issues 🎉
1 resolved since last review
Review effort: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
Resolver refreshes can retain stale incremental metadata after the full snapshot changes, causing requests for nonexistent snapshots and blacklisting healthy peers; explicit cache clearing and a two-cycle regression test are needed.
No open issues 🎉
Review effort: Balanced
snapct: the init name selection merged with #12568 (normal boots name the incremental at START as main does; the fast boot keeps the name built from the stream at INIT; the stream never runs snapshot_output_prepare), and main's full re-pairing on META is skipped behind a fast boot, where the stream fixes the incremental. executor: fd_executor_read_keys also emits programdata skipped for the loaded accounts data size limit (main 4cd5bf2), so the stream still carries every key the transaction touched. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
snapct: the init name selection merged with #12568 (normal boots name the incremental at START as main does; the fast boot keeps the name built from the stream at INIT; the stream never runs snapshot_output_prepare), and main's full re-pairing on META is skipped behind a fast boot, where the stream fixes the incremental. executor: fd_executor_read_keys also emits programdata skipped for the loaded accounts data size limit (main 4cd5bf2), so the stream still carries every key the transaction touched. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
No description provided.