test(dbfork): copy-on-write equivalence fixture via internal/fsclone#186
Merged
kuny0707 merged 2 commits intoJun 15, 2026
Merged
Conversation
barbatos2011
force-pushed
the
feat/snapshot-fixture-cow
branch
from
June 15, 2026 13:36
79b2381 to
7672226
Compare
Replace the equivalence gate's per-store byte copy with a copy-on-write clone. The gate materialises 8 fixture stores twice per run (scratchGo + scratchJava); on a full RocksDB fixture that copy dominates wall-clock once the snapshot is cached. New internal/fsclone package (designed via /plan-eng-review tronprotocol#185): - CloneDir(src,dst)->(method,err): clonefile(2) on darwin (recursive in one syscall), FICLONE per-file on linux, byte-copy fallback elsewhere. Build-tagged clone_{darwin,linux,fallback}.go + clone_unix.go errno classifier. golang.org/x/sys promoted to a direct dep. - Safety: stages into a primitive-created temp dir and atomically renames into place; refuses a pre-existing dst; on failure removes only its own temp, never caller data. Falls back ONLY on CoW-unsupported errnos (EXDEV/EOPNOTSUPP/ENOTSUP/ENOTTY/EINVAL/ENOSYS); real errors propagate instead of being masked as a slow copy. - Drops the manual copy_file_range layer: io.Copy already uses it for *os.File on linux. Equivalence test: - Scratch dirs now created ADJACENT to the fixture (same filesystem) instead of t.TempDir(); t.TempDir() lands under os.TempDir(), often a different filesystem, so the clone would EXDEV and silently fall back to a full copy -- the optimization would fire nowhere. (Codex finding.) - Calls fsclone.CloneDir; test-local cloneDir/copyDir/copyFile and their round-trip tests deleted (single clone+copy impl now lives in fsclone). Tests (internal/fsclone): round-trip + independence, a seam that forces the byte-copy fallback deterministically on any host, real-error propagation, dst-exists refusal, missing-src error, recursiveCopy round-trip. CI: dbfork-equivalence.yml gains a non-blocking CoW probe logging fixture vs scratch device IDs + a cp --reflink=always result, so we can see whether the clone actually reflinks on the runner or falls back. TODOS.md: capture the deferred public 'trond snapshot clone' verb.
golangci-lint errcheck flagged the bare `defer os.RemoveAll(tmp)`. Wrap it to explicitly discard the error — best-effort temp cleanup whose failure is non-actionable. (Missed locally: ran go vet, not the full golangci-lint.)
barbatos2011
force-pushed
the
feat/snapshot-fixture-cow
branch
from
June 15, 2026 14:52
821312e to
f2efb38
Compare
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.
What
Replace the dbfork equivalence gate's per-store byte copy with a copy-on-write clone, in a new reusable
internal/fsclonepackage.Why
TestEquivalence_GoVsJavamaterialises the 8 dbfork stores twice per run (scratchGo+scratchJava). Once the Nile snapshot is cached, that recursive copy is the dominant wall-clock cost — multi-GB on the RocksDB fixture. A copy-on-write clone shares the underlying extents, so each scratch copy is created in milliseconds at near-zero extra space; the first write to a block transparently splits it off, preserving the per-scratch independence the diff requires.How
internal/fsclone.CloneDir(src, dst) (method, error)— build-tagged per platform:clonefile(2)viax/sys/unix.Clonefile— recursively clones a directory tree in one syscall.FICLONEper regular file (x/sys/unix.IoctlFileClone).Safety contract (a reusable primitive, not just a test helper):
dst, then atomic rename into place.dst— never merges or clobbers.EXDEV/EOPNOTSUPP/ENOTSUP/ENOTTY/EINVAL/ENOSYS); real errors (EPERM, ENOSPC, unreadable source) propagate instead of being silently masked as a slow copy.copy_file_rangelayer — Go'sio.Copyalready uses it for*os.Fileon linux, so the fallback is kernel-accelerated where supported.Equivalence test:
t.TempDir().t.TempDir()resolves underos.TempDir(), frequently a different filesystem than the cached fixture — so a clone across them returnsEXDEVand silently degrades to a full copy, meaning the optimization would fire nowhere. Co-locating scratch with the fixture is what lets the clone actually reflink.fsclone.CloneDir; the test-localcloneDir/copyDir/copyFileand their round-trip tests are deleted — there is now a single clone+copy implementation.Tests
internal/fsclonecovers: round-trip + independence; a seam that forces the byte-copy fallback deterministically on any host (so the fallback orchestration is tested even on CoW filesystems where the real clone always succeeds); real-error propagation; pre-existing-dst refusal; missing-source error; and therecursiveCopyfallback directly.Verified: gofmt/vet clean,
go build ./..., fsclone + dbfork unit tests pass, and cross-compile builds for linux amd64/arm64 (FICLONE path), darwin, and the windows fallback.CI
dbfork-equivalence.ymlgains a non-blocking copy-on-write probe that logs the fixture vs scratch device IDs and acp --reflink=alwaysresult, so the run shows whether the clone actually reflinked on the runner or fell back to a copy. (GitHub's ext4 runners typically don't reflink — the win lands on copy-on-write filesystems; the probe makes that observable rather than guessed.)Follow-up
TODOS.mdcaptures a deferred publictrond snapshot cloneverb — left internal until a second real consumer (e.g. an agent warm-pool flow orapply --snapshot) exists.