[codex] Fix fork reflink follow-up - #18
Merged
Merged
Conversation
Fork latency was paying two stacked, avoidable costs: 1. Parent always took a Full snapshot (~23s for a 2 GB guest) on every fork, even when a base mem.file from a prior snapshot already existed. Now fork reuses standby's Diff-vs-Full decision: when the parent already has a base mem.file and dirty tracking is live (snapshotted=true), it takes a Diff — only pages dirtied since the last snapshot are rewritten in place (~0-7s), leaving a complete current image. A fresh or golden-restored parent (whose mem.file is a shared hardlinked inode that must never be diffed in place) correctly falls to a Full first to create the base. 2. Every per-VM artifact copy (child mem.file / rootfs / overlay, golden staging, private-disk restore) shelled out to `cp --reflink=auto` and ignored the result. On a reflink-capable FS that is instant CoW; on a non-reflink FS (plain ext4, tmpfs) it silently degrades to a full byte-for-byte copy — the measured ~58s-per-fork cost on the ext4 node — and a failed copy could masquerade as a complete artifact. All sites now route through one `reflink_copy` helper that bails on a non-success cp status, and a new `require_reflink` config gate (off by default) probes the data FS once at startup and FAILS CLOSED rather than emit a multi-GB full copy when the operator asserted reflink support. The daemon logs the reflink capability at boot; provision-node.sh arms the guard automatically on a reflink-capable node so a later FS regression surfaces loudly instead of regressing latency. Also hardens the fork hot path: child bringup now runs inside a guard that reclaims the live child microVM, its tap, and jail dir on any error, mirroring create()'s booted-match cleanup (a partial fork previously leaked these). Adds a `fork` benchmark path (cold-parent + child clone + paused load + resume) and docs (DEPLOY config table, RUNBOOK §10 Fork & reflink with verify/remediation). Expected win: cold fork parent snapshot ~23s -> ~0-7s via Diff once a base exists; per-VM copy ~58s -> instant CoW on a reflink FS. Proven here: cargo build, 54 mock-runtime tests pass, clippy clean, fmt clean. Not measured here: real boot/fork wall-clock — this box has no KVM/Firecracker, so the latency numbers are the documented prior measurements, pending a node.
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
2 participants
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.
Replacement for #7 with the original fork-performance change rebased onto current
mainand the review blockers addressed.What changed:
require_reflink=trueusecp --reflink=always, so operator opt-in cannot silently fall back to full multi-GB copies.mainegress-policy handling and carries the hostname-safe preview URL acceptance assertion.Validation:
cargo fmt --all -- --checkcargo clippy --workspace --all-targets -- -D warningscargo test -p sandboxd