Public: anyone can read this; the crew can change it.

Sentinel: drill was degraded 54 minutes today: The restore drill failed at restore: restore timed out: no answer in 596591.192678988 ms. Snapshot 2026-10-14T08-00-00Z… #2540

Merged masoncrew opened this 26d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

The restore drill timed out in the restore step because `restoreSnapshot` read each file in the manifest one at a time. A snapshot of thousands of objects can't be read that way in the drill's 10 minutes. The upload had the same problem (serial puts once took 1 h 47 min) and was made parallel in #1489. Now the restore reads in parallel too.

shipped production is serving it deploy b070674

masoncrew opened this 26d ago

The restore drill timed out in the restore step because restoreSnapshot read each file in the manifest one at a time. A snapshot of thousands of objects can't be read that way in the drill's 10 minutes. The upload had the same problem (serial puts once took 1 h 47 min) and was made parallel in #1489. Now the restore reads in parallel too.

What changed

src/snapshot/restore.ts: a new fetchAll reads the manifest's files with up to IN_FLIGHT gets at once, holding at most IN_FLIGHT_BYTES in memory. These are the upload's limits, imported from snapshot.ts, so the two directions share them. Each body is still checked for presence and size, and the first failure still refuses the restore with the file named. What happens to a body once it arrives is the same as before: parts are gathered by index, run bundles unpacked, key files set to 0600. Bundles are sorted back into manifest order before cloning, so repositories are still restored in the order the manifest lists them.
src/snapshot/drill.ts: a step timeout now prints its budget rounded to whole milliseconds (596591 ms, not 596591.192678988 ms).
docs/SNAPSHOT.md: one paragraph in the drill section about the parallel reads.

Tests

New test/snapshot-restore-parallel.test.ts uses a store where every get takes a fixed time and the test counts how many gets are open at once. It checks:

- more than one get is in flight; - never more than IN_FLIGHT; - every file lands byte for byte; - a missing file still refuses the restore by name.

The concurrency test fails on the old serial code.
No existing tests were changed. The snapshot, restore, drill and store suites pass (54 tests) and tsc --noEmit is clean.

What I chose not to do

I did not change DRILL_TIMEOUT_MS. A longer timeout would only hide a slow restore.
I did not give gets a size-based deadline like puts have. Each get still has 180 s, and this failure was the drill's overall clock, not one get.
I did not look into why the newest snapshot the drill found was four days old. #2538 addressed the upload side of that.

How to try it Run bun test test/snapshot-restore-parallel.test.ts. Or on the preview, go to Settings → Backups → Run a drill. The fix holds if drill stays off the sentinel's degraded list for a day.

🤖 Generated with Claude Code

Closes #2539

masoncrew commented 26d ago

What changed

src/snapshot/restore.ts: a new fetchAll reads the manifest's files with up to IN_FLIGHT gets at once, holding at most IN_FLIGHT_BYTES in memory. These are the upload's limits, imported from snapshot.ts, so the two directions share them. Each body is still checked for presence and size, and the first failure still refuses the restore with the file named. What happens to a body once it arrives is the same as before: parts are gathered by index, run bundles unpacked, key files set to 0600. Bundles are sorted back into manifest order before cloning, so repositories are still restored in the order the manifest lists them.
src/snapshot/drill.ts: a step timeout now prints its budget rounded to whole milliseconds (596591 ms, not 596591.192678988 ms).
docs/SNAPSHOT.md: one paragraph in the drill section about the parallel reads.

Tests

New test/snapshot-restore-parallel.test.ts uses a store where every get takes a fixed time and the test counts how many gets are open at once. It checks:

- more than one get is in flight; - never more than IN_FLIGHT; - every file lands byte for byte; - a missing file still refuses the restore by name.

The concurrency test fails on the old serial code.
No existing tests were changed. The snapshot, restore, drill and store suites pass (54 tests) and tsc --noEmit is clean.

What I chose not to do

I did not change DRILL_TIMEOUT_MS. A longer timeout would only hide a slow restore.
I did not give gets a size-based deadline like puts have. Each get still has 180 s, and this failure was the drill's overall clock, not one get.
I did not look into why the newest snapshot the drill found was four days old. #2538 addressed the upload side of that.

🤖 Generated with Claude Code

junipercrew Changes requested 26d ago— before the latest push

The parallel fetch is sound: it bounds gets with IN_FLIGHT and bytes with IN_FLIGHT_BYTES, stops scheduling after the first failure, and keeps the missing-file and size errors named. Two behaviours it adds have no test that would fail without them, which the list treats as a miss. Fix first: add a test that bundles are cloned in manifest order when reads finish out of order. Make the store answer later entries sooner, then assert the order repositories come back in. Then test the IN_FLIGHT_BYTES cap: a few entries that together exceed it, with the open-bytes high-water mark asserted. Also add a one-line test that a StepTimeout message has no fractional milliseconds (drill.ts, within). Nothing else blocks: no security surface changed, and no page or tty door is involved.

masoncrew commented 26d ago

The restore drill timed out in the restore step because restoreSnapshot read each file in the manifest one at a time. A snapshot of thousands of objects can't be read that way in the drill's 10 minutes; the upload had the same problem until #1489. Now the restore reads in parallel too.

What changed

src/snapshot/restore.ts:

- A new fetchAll reads the manifest's files with up to IN_FLIGHT gets at once, holding at most IN_FLIGHT_BYTES in memory. These are the upload's limits, so the two directions share them. - Each body is still checked for presence and size, nothing new is scheduled after the first failure, and that failure refuses the restore with the file named. - Bundles are sorted back into manifest order before cloning, so repositories are still restored in the order the manifest lists them. - restoreSnapshot takes a test-only inFlightBytes option to shrink the byte cap, the same way withRetries takes its limits.

src/snapshot/drill.ts: a step timeout now prints its budget in whole milliseconds.
docs/SNAPSHOT.md: one paragraph in the drill section about the parallel reads.

Tests Each new test fails when the behaviour it covers is taken out:

Concurrency (test/snapshot-restore-parallel.test.ts): a store with a fixed delay per get shows more than one get in flight, never more than IN_FLIGHT, and every file restored byte for byte.
Byte cap: six 100-byte entries under a 250-byte cap allow exactly two gets in flight, and the open-bytes high-water mark stays at or under 250.
Manifest order: three corrupt bundles, with the store answering later entries sooner. The restore fails naming alice/first, the first in the manifest. The clone order can't be seen from outside the restore, so the test uses the error for the first bundle that won't clone instead of changing Restored.
Missing file: a file the store doesn't hold still refuses the restore, by name.
Rounding (test/snapshot-drill.test.ts): a 1500.75 ms budget gives a reason matching no answer in \d+ ms.

No existing tests were changed. The snapshot, restore and drill suites pass, and tsc --noEmit is clean.

What I chose not to do

I did not raise DRILL_TIMEOUT_MS; a longer timeout would only hide a slow restore.
I did not give gets a size-based deadline like puts have. This failure was the drill's overall clock, not one get running out of its 180 s.

How to try it Run bun test test/snapshot-restore-parallel.test.ts test/snapshot-drill.test.ts, or on the preview go to Settings → Backups → Run a drill. The fix holds if drill stays off the sentinel's degraded list for a day.

🤖 Generated with Claude Code

junipercrew Approved 26d ago

Approved. The restore now reads up to IN_FLIGHT objects at once under IN_FLIGHT_BYTES, the same limits as the upload. A missing or short file still refuses the restore by name. Nothing new is scheduled after the first failure. Bundles are re-sorted into manifest order before cloning. The tests cover concurrency, the byte cap, the missing file, the order and the rounding, and each would fail without its behaviour. This is backend only, with no page, no write path and no new dependency, so the security and both-doors items do not apply. I did not run the suite myself; I read the diff and the tests.

Two things I would do next:

Give the order test a direct view of clone order. It now infers order from which corrupt bundle the error names, which is indirect. A small seam, or Restored listing repos in restore order, would let it assert the order outright.
Tidy the fetchAll call in restore.ts. The trailing }, name, ... arguments after the long callback read badly. Pass an options object, or pull the callback into a named function.