atelet: node-local file cache library (filecache, M1) - #1517
atelet: node-local file cache library (filecache, M1)#1517Dmitry Berkovich (dberkov) wants to merge 6 commits into
Conversation
Introduce cmd/atelet/internal/filecache, the foundation of a node-local artifact cache: opaque entry keys (content-addressed sha256 and immutable-URI forms), the entries/tmp on-disk layout, a startup sweep for crash debris (unfinished fetches, interrupted evictions), and byte accounting for a GC budget. Golden snapshot restores download their files per actor with no reuse, and sandbox-asset fetches race concurrent downloads of the same asset; this package is the shared cache that will back both paths. Retrieval (singleflight fetch, atomic publication, hardlink-out) and eviction build on this skeleton in follow-up changes.
GetFileTo materializes a cached artifact at a destination path via hard link, fetching it on a miss. Concurrent callers for one key share a single fetch (singleflight), and the fetch runs detached from the callers' contexts bounded by the store's fetch timeout, so one canceled caller never aborts a download other callers are waiting on. There is no negative caching: a failed fetch reaches every waiting caller and the next call starts fresh. A fetch lands in tmp/, must produce a regular file, is made read-only (0444) so a consumer's in-place write fails loudly instead of corrupting the shared copy, and is published with one atomic rename. A hit links out and touches the entry's last-use clock under a shared lock that eviction will hold exclusively, closing the hit-vs-evict window. Destinations must not exist and must be absolute paths on the cache's mount; cross-filesystem destinations fail with a dedicated error rather than a silent copy.
6f7da13 to
f7d72fb
Compare
copyFile and its hole-preserving machinery lived in package main, usable only by atelet's own checkpoint staging. The filecache package is about to need the same copy (its copy-out mode hands consumers a private, hole-preserved copy of a cached artifact), so move the code where both can import it. Mechanical move, with one seam added: Copy(src, dst *os.File) exposes the engine on caller-owned handles, for callers that must open the source before its name can vanish or create the destination with O_EXCL. CopyFile keeps its os.Create semantics for the existing caller.
GetFileTo serves hits as read-only hard links, which is only safe for consumers that never write the staged file in place. GetFileCopyTo serves the same read-through cache as a private copy instead: the caller owns the resulting inode outright (mode 0600) and may mutate it freely, holes are preserved, and the destination may live on any filesystem. The copy reads a handle opened under the hit lock, so an eviction racing the copy retires only the entry's name — the bytes survive until the copy completes. Fetch dedup is unchanged: concurrent calls for one key share a single flight.
EvictUnused frees cache space least-recently-used first until a byte target is met, with a two-phase retire: inside the key's singleflight and the hit lock, a victim is re-verified (a moved last-use clock or an in-flight fetch vetoes) and renamed to a .rm-* dir, making it invisible to lookups; the slow physical deletion runs after all retires, outside the locks the hot path contends, so hits and fetches never wait on it. Entries younger than the store's min age are never touched, covering the window between publication and a consumer's first link. Entries whose data a consumer still hard-links may be retired but count as pending rather than freed bytes - the kernel returns that space when the last consumer link goes - so eviction can only ever cost a re-download, never break a consumer. FreedBytes is credited per entry only after its physical removal succeeds; a failed removal leaves the bytes in a .rm-* dir for the startup sweep and out of the freed count.
State the package's consumer-protection contracts in the package doc (link-out immunity, min-age sizing, the read-only shared-bytes rule, and key immutability), and pin them with a race-detector stress test: getters and evictors hammer the same keys concurrently, and every get must succeed with intact content - eviction may force refetches but can never fail a caller, corrupt a served file, or leave half-states in the store.
f7d72fb to
ee7dd93
Compare
There was a problem hiding this comment.
Superseded by the comment-only review. The change request has been dismissed.
There was a problem hiding this comment.
🤖 AI-generated review. I looked through them to make sure they weren't crazy and they all seem worth looking at.
-
Use allocated bytes for sparse-file accounting — evict.go:213
sizeEntryandTotalBytescount logical file lengths. In a local reproduction, an entry whose regular files occupied 8 KiB was reported as freeing over 1 GiB. This can trigger unnecessary eviction and stop reclamation before the requested disk space is freed. Please use allocated bytes consistently for budgeting and eviction, such asStat_t.Blocks * 512, and add a sparse-file accounting test. -
Synchronize the failed-fetch test deterministically — getfileto_test.go:197
started.Done()runs before callers enterGetFileTo, so the fetch can finish before everyone joins. Late callers correctly retry, violating the test’s one-fetch assertion. A focused race-enabled run failed 16 times out of 100. Please wait until the callers are actually waiting on the flight before releasing the fetch, usingtesting/synctestor an equivalent deterministic barrier. -
Report filesystem errors during eviction enumeration — evict.go:180
TheStatandsizeEntryerror branches assume an entry disappeared during enumeration, swallowing permission and I/O errors too. An unreadable entry reproduced a nil error and entirely zero eviction statistics. This makes broken cache access indistinguishable from having no eligible entries. Please ignore onlyfs.ErrNotExistand return other errors with the affected path. -
Clarify the copy helper’s destination requirements — sparsefile.go:78
Copyaccepts open handles without requiring an empty destination. Copying an all-hole source over an existing file returns success while retaining the old destination bytes; this was reproduced locally. The current caller creates an empty file and is safe. The smallest fix is to document the empty, zero-offset destination requirement and add a contract check, or explicitly support overwriting. -
Make eviction-race coverage deterministic — stress_test.go:113
The stress test discards eviction statistics and exercises only hard-link retrieval. One passing run performed just the eight initial fetches. Please add deterministic overlap tests for eviction versus both serving modes and verify that eviction actually occurred. Fetch-timeout behavior also needs a test beyond checking the configured value. -
Move test-only metadata reading out of production code — filecache.go:197
readEntryMetais used only by tests. It can move into the test file, or the test can decode the metadata directly.
Validation: scoped golangci-lint and go vet passed. Atelet tests passed, and filecache/sparsefile passed three race-enabled runs. Focused repetitions reproduced the flaky test; isolated probes reproduced the accounting, filesystem-error, and existing-destination issues. Full repository verification and infrastructure E2E were not run.
Replaced with a comment-only review at the reviewer’s request: #1517 (review)
2 participants
Implements milestone M1 of the node-local artifact cache proposed in #690 (design in the issue comment): a generic
cmd/atelet/internal/filecachepackage that will back golden-snapshot restores (today re-downloaded per actor on every start/resume) and later the sandbox-asset fetches. it reads well commit by commit:atelet: add filecache store skeleton— constructor-onlyKeys (SHA256Keycontent-addressed,URIKeyfor immutable sources; prefix-disjoint canonical forms, entry dir =sha256(key)), theentries/+tmp/+.rm-*layout,SweepDebris(startup crash-debris reaper),TotalBytes(GC budget measure), debug-onlymeta.json.atelet: add filecache singleflight retrieval (GetFileTo)— atomic get-and-link: per-key singleflight oncontext.WithoutCancel+ fetch timeout (a canceled caller never aborts the download others wait on; no negative caching); fetch intotmp/, validate, chmod0444(in-place writes fail loudly instead of poisoning shared bytes), publish by one atomic rename; hit = hard link + LRU touch underhitMu.RLock. Path-basedFileFetchersoategcs's sparse zstd download plugs in unchanged;%wwrapping end-to-end forateerrorsclassification.atelet: move the sparse file copy helpers into internal/sparsefile— mechanical move ofcopyFile/copySparse/kernelCopyRange(and their tests) out of package main so filecache can reuse them; addsCopy(src, dst *os.File)for caller-owned handles (source opened before its name can vanish, destination createdO_EXCL).filecache: add GetFileCopyTo for consumers that mutate staged files— the second serving mode: a private, hole-preserving copy (mode0600) instead of a read-only hard link, for consumers that rewrite staged files in place (ateom-microvm rewritesconfig.jsonat restore and merges deltas intomemory-rangesat suspend — a shared inode would be corrupted). The copy reads a handle opened under the hit lock, so an eviction racing the copy retires only the entry's name; a copy needs no same-mount constraint.atelet: add filecache eviction (EvictUnused)— pressure-driven only: min-age gate, unlinked-first then LRU ordering, stop at target. Two-phase retire inside the key's singleflight +hitMuexclusive (moved last-use clock or in-flight fetch vetoes; rename to.rm-*), slowRemoveAllafter all retires outside the hot-path locks. Stats distinguishRetired(namespace removal, irreversible at rename) fromFreedBytes(credited only after physical removal succeeds) andPendingBytes(retired but consumer-linked; kernel frees later). Copied-out entries carry no links, so eviction is free to take them — existing copies are private inodes and unaffected.atelet: document filecache contracts and stress the get/evict races— package-doc contracts (link-out immunity, copy-out privacy, min-age sizing rule, read-only shared bytes, key immutability) plus a race-detector stress test: concurrent getters and evictors on shared keys; every get must succeed with intact content.The core safety property throughout: eviction can only ever cost a refetch — never break a consumer. Hard-linked files are protected by the link itself (the consumer's inode survives eviction); copies are private inodes; the min age covers the publish-to-use window.
Follow-ups per the design: M2 wires a golden store into
Restore(downloadExternalCheckpoint/downloadCombinedCheckpoint) with a GC driver loop — gVisor restores get hard links, micro-VM restores get copies; M3 addsGetFile/GetDir+ the sandbox-record root set and migratesfetchAsset/fetchGVisorRelease.Tested:
go test -race -count=3 ./cmd/atelet/internal/filecache/; every commit builds and passes tests individually;golangci-lint, gofmt, and boilerplate checks clean.🤖 Generated with Claude Code