Repository navigation
review: whole-repo audit after the volume merges, with the #403 controller docs - #405
Merged
Merged
Conversation
pool kept its own syncDir next to the identical tail of ReplaceFileSync; both now call utils.SyncDir, which keeps the open/sync stage errors the catalog ordering test pins. Drops a comment that restated volumeQuiesceMax's use.
Complete the volume series with the external detach, hibernate, wake and reattach flow. Exercise the Python example through SDK requests and keep reservation, quiescence and access gating with the controller.
The VolumeHooks docstring repeated the doc in 26 lines; each hook's contract now lives once in docs/volume-controller.md, which also points mount_and_verify at the attach-only device discovery and mount retry. The cancellation test repeated the failure cases: the functions hold no cleanup, so any exception leaves the gate closed the same way.
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.
Whole-repo audit after today's volume merges (#382, #383, #395, #396, #398, #399, #401, #402, #404), with #403 folded in.
Changes
utils: one SyncDir owns the directory fsync:poolkept a privatesyncDirthat duplicated the tail ofutils.ReplaceFileSync. Both now callutils.SyncDir, which keeps theopen dir/sync dirstage errors thatvolume_catalog_ordering_testpins. Drops one comment that restatedvolumeQuiesceMax. Go prod −2 lines, comment lines +1/−1.docs: match the SDK volume options and the mount retry: since pool: runtime volume catalog follow-ups to #382 #395,WithVolumesAttachOnlyand Pythonmount=Falseno longer validate locally (sandboxd answers 400).Volumecarriesattach_only. A mount that reportsnot a valid block deviceis retried within the setup budget (engine: retry a volume mount while the device is not yet openable #404), and attach-only callers get the same advice. The README names runtime volume registration (pool: register runtime volume catalog entries #382).e2e: cover controller-managed volume hibernation(from docs: document controller-managed volume hibernation #403, by @minatoaquaMK2, cherry-picked unchanged): the controller doc, the Python example with its tests, and the HTTP/Go SDK detach → hibernate → wake → reattach test.review: tighten the volume controller example and its doc: the 26-lineVolumeHooksdocstring repeated the doc. Each hook contract now appears once, as a list indocs/volume-controller.md. That list pointsmount_and_verifyat the attach-only device discovery and mount retry. The example gets a one-line module docstring. The cancellation tests are dropped: the functions hold no cleanup, so aCancelledErrortakes the same path as the failure cases already tested.docs: list attach_only in the sandbox index row.No daemon behavior change. Hot-path cost: zero. The rw-volume and catalog paths make the same open/fsync/close syscalls, and the warm-claim path does no new work.
Audit scope
@live-volumesflow. Every prod file changed since 9974f1b was read against /code, /code-py, /code-rs, use-modern-go, /simplify and /loc-justify.aslforwarder findings (replaceArchive,replaceReplica,skipIdle,refillGated,isZero) were already recorded as kept.time_tick_gcis skipped for the volume poll loops: they are bounded and return, soStopreleases the 10 ms ticker at return instead of at the next GC.Gates
make go-lint: 10 ×0 issues.(5 modules × linux/darwin), fmt clean. With a warm cache, staticcheck SA4023 panics ine2eafter a dependency edit; a cold cache (as in CI) is clean.asl -forwarder=false ./...: 0 findings, every module, linux and darwingo test -race -count=1 ./...: 25 packages ok;TestRuntimeVolume*pass on the final head-D warningsand tests green on macOS and in a linux/arm64rust:1container (rustc 1.99.1)examples)Hardware (bare metal, 384 cores, cocoon v0.7.4 official, CH v54.0.0, rt:24.04 sha256:03e8bed49d4c, isolated cocoon root)
Full e2e suite, interleaved PR / main / main / PR. All 32 legs passed (seconds):
bench.shran in 8 interleaved runs (PR, main, main, PR, then main, PR, PR, main). Warm hit p50 was 0.4–0.5 ms in both arms. Clone-tier p50: PR 73.4 / 161.1 / 84.5 / 78.2 ms, main 65.9 / 167.1 / 177.0 / 79.0 ms. The slow runs (~160–177 ms) depend on run position, not arm: main #2, PR #2 and main #3 were slow. Cold boot p50 was 650–667 ms in both arms.The #403 controller cycle ran on this head with a real tenant claim and a runtime-registered writable attach-only volume: attach, mount and write, then detach, hibernate, sandboxd restart while asleep, exec wake, reattach, mount and verify, then a second detach and hibernate. The claim, device, filesystem UUID, serial and payload all matched across the cycle. Timings: attach 78 ms, hibernate 605 ms, wake + reattach 212 ms. Unregister after release returned 204, and the daemon logged no errors.
Closes #403.