Skip to content

perf(snapshot): drop fsyncs on transient extract/copy paths; fold NoSync variants into SyncMode#119

Closed
CMGS wants to merge 1 commit into
masterfrom
fix/snapshot-create-dirsync
Closed

perf(snapshot): drop fsyncs on transient extract/copy paths; fold NoSync variants into SyncMode#119
CMGS wants to merge 1 commit into
masterfrom
fix/snapshot-create-dirsync

Conversation

@CMGS

@CMGS CMGS commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Stacked on #120 (review/merge that first; after it lands, rebase this branch onto the updated master and confirm the diff shows only this one commit before merging).

What

The counterpart to #120: paths whose product is rebuilt from a durable source on retry stop paying per-file fsyncs.

  • restore staging (PrepareStagingDir) and stream-clone extraction (CloneFromStream): ExtractTar gains the SyncMode parameter from fix(snapshot): complete ingestion durability; move capture fsyncs off the pause window #120; these pass NoSync.
  • direct clone/restore file clones (CloneSnapshotFilescopyPairs): flip to NoSync.
  • store ingestion stays durable, now in a single sync pass: Import keeps Sync; Create extracts with NoSync and relies on its existing SyncTree (files + dir entries) — removing the double fsync fix(snapshot): complete ingestion durability; move capture fsyncs off the pause window #120 left (each file was synced by ExtractTar, then re-synced by SyncTree), and letting extraction overlap background writeback instead of blocking per file.
  • AtomicWriteFileNoSync / AtomicWriteJSONNoSync fold into the same parameter — one sync convention across utils, five exported write helpers instead of ten.

Crash-semantics change (the part to review)

Interrupted operations converge exactly as before (restore tombstone, create placeholder + GC). What changes: after host power loss, an instance cloned/restored within the writeback window (~30s) may have holes in its COW base. That surfaces as loud guest fs errors on the next cold boot; recovery is recreate (vm rm + re-clone/re-restore) — sources stay durable in the snapshot store. Ops note: after a host power-loss reboot, recreate instances cloned/restored in the last minute instead of debugging them.

Why

Same measurement as #120: fsync tax ≈ 0.155 s/GiB on the ext4/NVMe floor (host ai), seconds/GiB on PD-class disks — paid on the clone/restore latency path for zero durability benefit (the extracted/copied working set is consumed immediately from page cache and is replayable).

Verification

  • go build ./..., go vet ./...
  • make test (full suite, -race): 28 packages ok
  • make lint (GOOS=linux + darwin): 0 issues

@CMGS CMGS changed the title fix(snapshot): sync dir entries on the tar-fallback create path fix(snapshot): scope fsyncs to the durability contract per path Jul 10, 2026
@CMGS CMGS force-pushed the fix/snapshot-create-dirsync branch from e21d748 to 7979759 Compare July 10, 2026 18:27
@CMGS CMGS changed the title fix(snapshot): scope fsyncs to the durability contract per path perf(snapshot): drop fsyncs on transient extract/copy paths; fold NoSync variants into SyncMode Jul 10, 2026
@CMGS CMGS changed the base branch from master to perf/capture-sync-placement July 10, 2026 18:28
@CMGS CMGS force-pushed the fix/snapshot-create-dirsync branch from 7979759 to dd17eed Compare July 11, 2026 03:10
Base automatically changed from perf/capture-sync-placement to master July 11, 2026 03:17
Restore staging, stream-clone extraction, and direct clone/restore
file clones are rebuilt from a durable source on retry, so their
per-file fsyncs bought no durability while adding GiB-scale writeback
to clone and restore latency. ExtractTar gains the SyncMode parameter
and the clone/restore copyPairs flip to NoSync; the AtomicWrite NoSync
variants fold into the same parameter, leaving one sync convention
across utils. Store ingestion stays durable: Import keeps Sync, and
Create extracts NoSync then syncs once via SyncTree instead of twice.
@CMGS CMGS force-pushed the fix/snapshot-create-dirsync branch from dd17eed to 5507725 Compare July 11, 2026 03:21
@CMGS

CMGS commented Jul 11, 2026

Copy link
Copy Markdown
Contributor Author

Parked rather than rejected. Rebased onto master (post-#120) so the branch carries exactly one clean commit for revival.

Why close now:

  • This is the only piece of the fsync work that changes crash semantics: an instance cloned/restored within the writeback window (~30s) of a host power loss may have holes in its COW base — recovery is recreate, not repair.
  • Today's benefit doesn't pay for that: the default local clone/restore path hardlinks memory files (no copy at all), sandbox COWs are small, and the stream paths (cross-node clone / stream restore) aren't hot yet. Measured tax being saved: ≈0.155 s/GiB on the ext4/NVMe floor (host ai), seconds/GiB on PD-class disks.
  • Reopen when clone/restore latency enters the product budget or GiB-scale COW / cross-node stream paths become hot. fix(snapshot): complete ingestion durability; move capture fsyncs off the pause window #120 already landed the SyncMode plumbing, so this stays a small self-contained diff; the branch (fix/snapshot-create-dirsync) is kept.

@CMGS CMGS closed this Jul 11, 2026
@CMGS

CMGS commented Jul 11, 2026

Copy link
Copy Markdown
Contributor Author

Two more findings from a whole-repo over-durability audit (master @ f8180ed), recorded here since they are the same class as this PR — optional fsync/durability scoping, not worth reopening on their own:

  1. ExportToDir (snapshot/localfile/export.go) copies with utils.Sync. The export target is a transfer staging dir with a replayable source (re-export), and snapshot.json is written last as the all-data-ready marker — after power loss vm clone --from-dir fails loudly at integrity preflight and a re-export converges. Flipping to utils.NoSync saves seconds/GiB on slow disks for this cold path.

  2. OrphanDirs cleanup-intent mechanism (VMIndex.OrphanDirs in hypervisor/db.go, migratedDirs/clearOrphanDirs in stop.go, sweepOrphanDirs in gc.go; ~60 lines). It only fires on a double edge: the VM's dirs live under an old --run-dir root and delete removes the record but fails the dir removal. The ops recourse is manual cleanup of the old root after a run-dir migration. Zero hot-path cost (rides the existing delete transaction), so it stays for now; recorded as removable if migration cleanup is ever declared an operator responsibility.

If clone/restore latency enters the product budget, both can ride the same change as this branch's ExtractTar/CloneSnapshotFiles scoping.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant