Skip to content

perf(fork): diff-snapshot parent + fail-loud reflink - #7

Closed
drewstone wants to merge 1 commit into
mv37-org:mainfrom
drewstone:perf/fork-diff-snapshot-reflink-failloud
Closed

perf(fork): diff-snapshot parent + fail-loud reflink#7
drewstone wants to merge 1 commit into
mv37-org:mainfrom
drewstone:perf/fork-diff-snapshot-reflink-failloud

Conversation

@drewstone

Copy link
Copy Markdown

Forking a sandbox from a live parent was paying two stacked, avoidable costs. This PR removes both and adds a fail-loud safety guard so a slow filesystem can never silently make forks 50+ seconds slower.

What was slow

1. Parent re-snapshotted from scratch every fork (~23s).
Each fork forced the parent to take a Full memory snapshot (~23s for a 2 GB guest), even when a usable base snapshot already existed. Now fork reuses the same Diff-vs-Full decision the standby path already uses: if the parent already has a base memory file and live dirty-page tracking, it takes a Diff — only the pages changed since the last snapshot are rewritten in place (~0–7s) — leaving a complete, current image. A brand-new parent, or one restored from a golden snapshot (whose memory file is a shared hardlinked inode that must never be diffed in place), correctly takes a Full first to create the base.

2. Per-VM disk copies silently fell back to full copies (~58s).
Every per-VM artifact copy (the child's memory file, root disk, overlay; golden staging; private-disk restore) shelled out to cp --reflink=auto and ignored the result. On a copy-on-write filesystem (xfs, btrfs, ext4 with reflink) that is instant and free. On a plain-ext4 / tmpfs box it silently degrades to a full byte-for-byte copy — the measured ~58s per fork on the ext4 node — and a copy that failed partway could be mistaken for a complete image. All of these now route through one reflink_copy helper that fails loudly on a non-success copy, so a broken copy can never masquerade as a finished artifact.

The new safety knob

A new require_reflink setting (off by default, so existing dev/ext4 boxes are unchanged) probes the data filesystem once at startup. When on, fork / private-disk staging fail closed instead of silently emitting a multi-GB full copy when the operator asserted the filesystem supports copy-on-write. The daemon logs the reflink capability at boot, and provision-node.sh now arms this guard automatically on a reflink-capable node — so if the filesystem ever regresses, forks fail loudly instead of quietly getting 50+ seconds slower.

Also hardens the fork hot path: child bringup now runs inside a guard that reclaims the live child microVM, its tap, and its jail directory on any error (a partial fork previously leaked all three), mirroring the existing cleanup in create().

Expected win

  • Cold parent snapshot per fork: ~23s -> ~0–7s once a base snapshot exists (Diff instead of Full).
  • Per-VM disk copy: ~58s -> instant copy-on-write on a reflink filesystem.

What is proven here vs pending a node

Proven on this machine: cargo build clean, 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 figures above are the previously measured numbers and remain pending verification on a real node.

Follow-up (not in this PR)

Proactively pre-snapshot idle fork parents in the background so the first fork of a freshly idle parent also lands on the fast Diff path instead of paying the one-time Full.

🤖 Generated with Claude Code

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.

@arslnb arslnb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the detailed PR, @drewstone. The direction looks right to me, but I found two runtime issues I’d want fixed before this merges.

}
let _ = std::fs::remove_file(to);
let st = tokio::process::Command::new("cp")
.args(["--reflink=auto"])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@drewstone I think this misses the actual fail-closed guarantee. Even when require_reflink is true, this still uses cp --reflink=auto, so a pair-specific clone failure can silently fall back to a full byte copy and still return success. The startup probe proves the data dir can reflink one tiny sibling file; it does not prove every later source/destination clone succeeds. I’d switch this to --reflink=always when the guard is enabled, or otherwise verify the actual copy did not fall back.

poll_until(Duration::from_secs(4), || child_sock.exists()).await;
(child_sock, child_jail.join("vsock.sock"), pid)
};
child_pid = pid;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@drewstone This assignment happens after the whole jailer staging branch returns, but that branch spawns the child process before several new fallible reflink_copy calls. If one of those copies fails, the error handler still has child_pid = None and there is no VM record yet, so kill_and_reclaim cannot kill the spawned jailer/Firecracker process. I’d capture pid immediately after spawn, before any fallible staging work.

@arslnb

arslnb commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

@drewstone labeling this as interesting slop rather than plain slop. The fork/reflink direction is worth keeping, but the previous inline blockers still make it not merge-ready: crates/sandboxd/src/runtime/firecracker.rs:1293 still uses cp --reflink=auto under the fail-loud path, and crates/sandboxd/src/runtime/firecracker.rs:2651 assigns child_pid after staging work that can now fail, so cleanup can still miss the spawned child.

@arslnb arslnb added the interesting slop Interesting direction but needs careful review or cleanup label Jul 1, 2026
@arslnb

arslnb commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Follow-up is ready in #18. This branch is conflict-marked, so I pushed a base-repo replacement that keeps the same fork/reflink direction and fixes the earlier blockers: crates/sandboxd/src/runtime/firecracker.rs:1350-1354 uses --reflink=always when require_reflink is set, and crates/sandboxd/src/runtime/firecracker.rs:2690-2703 captures the child pid immediately after spawn before reflink staging can fail. #18 is green and mergeable.

@arslnb

arslnb commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Closing as superseded: the reviewed/fixed replacement #18 has now been merged to main.

@arslnb arslnb closed this Jul 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

interesting slop Interesting direction but needs careful review or cleanup

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants