fix: one writer per cache_dir, and chunk files that are replaced, not truncated

[emfdavid]

Summary

Part 1 of #42 — the arbitration half. It does not touch the manifest, so it fixes the
silent corruption without invalidating anybody's cache. Part 2 (per-variable cache identity)
still needs the format bump and is unchanged; the issue stays open for it.

Two processes pointed at one cache_dir — the obvious thing to do, and what the
supply-chain audit on NVIDIA/earth2studio#962
found InSituForecastFeed inviting — could corrupt each other with nothing to show for it.
The window is re-admission after eviction: B revives a cached chunk and holds its mapping,
A misses the same chunk and re-admits it, and open_memmap(mode="w+") truncates the file
under B, which then reads zero pages as data. Right shape, right dtype, wrong numbers.

The fix, in two independent layers.

1. _alloc replaces instead of truncating. A slot file is written under a temp name and
   renamed into place; POSIX keeps the old inode alive for anyone holding it, so a reader's
   mapping stays intact. This closes the corruption window on its own, with no locking
   involved. A writer killed between the two leaves a temp file, swept by the next writer to
   hold the exclusive lock (the only moment it is safe).
2. An advisory flock on the cache dir, held for the pool's lifetime. Exclusive to
   write, so a second writer fails fast with a message naming the holder, what to do in
   either case, and the two commands that answer "who?". Non-blocking on purpose: contention
   is a configuration fact to resolve, not a queue to join.

The lock keys on cache_dir being set, not on persist — a correction to the issue as
filed. _alloc writes {array}__{cid}.npy whenever a backing dir is set, so two processes
sharing a spill dir with persist=False collide on identical filenames just as surely;
scoping the lock to persist mode would have left that hole open. Test:
test_the_lock_keys_on_cache_dir_not_on_persist.

New readonly_cache=True (InSituDataset and ChunkPool) is the workload this is
shaped around — one job warms a cache, several score against it. Shared lock, so many
coexist and none coexists with a writer; writes nothing; and a miss raises, naming the
array and chunk. That is what makes it a contract (this cache is complete for what I am
about to read) rather than a slow path that silently re-fetches whatever the warming run's
split or transforms left out. reset_stale_cache is rejected in this mode.

Also: manifest entries now go out as a single os.write on an O_APPEND fd (sub-PIPE_BUF
appends are atomic under POSIX), so the format is robust independently of the lock —
which is what keeps a log written on an unarbitrated platform readable.

What is still unarbitrated, said plainly where the user meets it. A network cache_dir
(flock may be emulated per client, so two hosts can each believe they hold it) and a
platform with no POSIX locking. Both warn at construction saying that this is the one
configuration where two writers can still corrupt each other. Detection is not a fix. That
forced the project's first platform-support statement (README, contributing,
classifiers): Linux supported and the only thing CI proves; macOS expected to work but
untested; Windows untested and unarbitrated — untested, not unsupported.

Two things the docs now have to say, because they are the first things anyone will get
wrong: there is no such thing as a stale flock (the kernel releases it on process
death, SIGKILL and spot preemption included), so there is no cleanup procedure; and
deleting the lockfile is actively harmful — it releases nothing and makes the next two
processes lock different inodes, reintroducing exactly the corruption the check prevents.

Two deviations from the agreed plan, both deliberate

• bench/run.py did not get --readonly-cache. Its --cache-dir is a per-config
  scratch root, not a shared cache, and nothing in the bench sets persist=True — so the
  flag could only ever error. Making it usable means adding persist to Cfg, which lands a
  new column in the JSONL results schema for a knob no benchmark measures. Benchmarking the
  cross-run cache is its own piece of work. The other three CLIs got --persist and
  --readonly-cache: --readonly-cache alone would have been equally dead there, since no
  example could warm a cache for it to read.
• examples/advection/data.py was left alone. It has a --cache-dir too, but it was not
  in the agreed list, and the arbitration protects it regardless — the lock is on the
  directory, not on a flag.

For reviewers

Worth a second look:

• _alloc's filename fix-up. After tmp.replace(path) the memmap still records the
  name it was opened under, so _alloc corrects backing.filename. _record_completed
  names the cache entry through it and _free unlinks through it; both would otherwise chase
  a temp path that no longer exists. test_pool_spill_unlinks_without_persist covers the
  second.
• Releasing the lock when the constructor raises. A stale cache raises inside
  __init__, after the lock is taken, and __del__ cannot clean that up — close() reads
  attributes a half-built pool does not have, and its own suppression would swallow the
  error, leaving the directory locked until the process exits. Handled explicitly.
• The two in-process "crash" simulations in test_pool.py now call a _die(pool) helper
  that closes the lock fd without running close(). That is what process death actually
  does (the kernel drops the fds; nothing in close() runs), and the tests' contract — only
  what was appended at completion survives — is unchanged.
• readonly_cache deliberately does not require persist=True: it implies the same
  cross-run identity internally, so persist=True, readonly_cache=True and readonly_cache=True
  alone behave identically, and forgetting the pair cannot produce a confusing all-misses run.

Confident in: the failure mode itself (reproduced by a subprocess test that fails on main),
and that nothing changes for a single-process run.

Active readers while a writer invalidates the cache

Asked in review, and worth stating as a contract rather than leaving to inference.

It cannot happen on the arbitrated path. Invalidation — reset_stale_cache deleting
every entry, or a run re-admitting chunks it evicted — is work a writer does, and LOCK_SH
excludes LOCK_EX, so a writer cannot open the directory while any reader holds it. The
exclusion lands at construction, before the writer can allocate, evict or delete anything.
Now tested cross-process in both directions
(test_a_writer_cannot_start_while_another_process_is_reading), not just in-process.

And the layer underneath does not depend on that — which is what matters on a network
cache_dir or a platform without advisory locking. A reader holding a chunk's mapping keeps
reading real data whether the file is replaced (new content goes to a new inode) or
deleted (POSIX keeps the inode alive until the last reference goes). So the worst a
bypassed lock can do to an active reader is cost it a future open — a miss, which
readonly_cache turns into a loud error — never wrong numbers in a batch that looks fine.
Pinned by test_deleting_a_cached_file_does_not_disturb_a_held_mapping.

Checking that turned up a real defect in this PR, now fixed (732acc9): close()
released the lock before freeing the slots, leaving a window where another process could
take the write lock while this one still had cache files mapped. Atomic replace meant the
window could not corrupt data, but the invariant the arbitration rests on is "hold the lock
for as long as you hold a mapping" — and a fix that leans on the other layer to cover it is
one that stops working the day the other layer changes. The lock is now released last, and
test_the_lock_outlives_every_mapping_the_pool_holds fails on the previous ordering.

Verified by running the CLIs, not just --help

• examples/wb2_dataloader.py — --persist warmed 6 chunks; --readonly-cache over it
  reported chunks 6/6 hit (100%) with inflight peak 0/32, i.e. it fetched nothing. Against
  a deliberately incomplete cache (entries removed from the log and disk) it raised through
  the prefetch producer onto the main thread with the intended message: "chunk 5 of 't2m' is
  not in the cache … The usual cause is that the run that warmed it used a different split,
  sample_range or transform set."
• Real two-process contention, through the CLI. Writer A held the cache for 12 s
  (--train-step-ms 4000); writer B on the same --cache-dir failed at construction with the
  contention error naming A's live PID and host. The two commands that error prints were then
  run against that lockfile and both named the same PID — fuser -v → dstuebe 54294 F.... python3, lsof → 10uW (write lock held). After A exited, a --readonly-cache run over
  the same directory succeeded immediately: no stale lock, no cleanup step.
• examples/fit_scaler.py — --persist then --readonly-cache; both complete, the
  read-only pass serving every chunk.
• bench/run.py (unchanged, but cache=resident puts a cache_dir under the new lock) —
  a local sweep runs clean, including --repeats 2, since each repeat already gets its own
  scratch subdirectory.
• examples/advection/train_torch.py (unchanged; also takes --cache-dir) — full run
  green, and the model-free check passes: persistence RMSE 0.884, which is the one signal
  that would catch a cache returning different bytes.
• examples/wb2_arraylake.py could not be run here — arraylake is not installed in this
  environment and the local config carries no token. Argparse accepts both new flags (it gets
  as far as the arraylake import before failing), and main()'s run(...) call was bound
  against the real signature to catch a kwarg-name typo that mypy cannot see through an
  untyped argparse Namespace. It deserves a run by someone with credentials.

One incidental, unrelated finding: train_torch --n-steps 24 fails with ValueError: need at least one array to concatenate (too few chunks leaves a split empty). It reproduces without
--cache-dir, so it is pre-existing and untouched here.

One small behavior change worth knowing: a spill (persist=False) cache_dir used to be
left empty on close. It now retains the .insitu.lock file — deliberately, since that inode
is the lock, and removing it is what the docs tell users never to do.

Author attestation

• I have reviewed every change in this PR, I can explain why each one is correct, and I
  have verified the claims made in this description.

(Left unchecked deliberately — that box is the human author's to tick.)

Checklist

• Tests added or updated — tests/test_cache_concurrency.py (18 tests). The bug test came
  first and is a real subprocess: a child maps a chunk, the parent re-admits it, and the
  child must still read real data. It fails on main
  (assert '38723a2e…' == '64cddc0c…'). Also covered: second writer raises with the
  guidance, reader+writer excluded, many readers coexist, a readonly miss raises naming the
  chunk, readonly writes nothing, readonly rejects reset_stale_cache, .tmp litter swept
  (and not swept by a reader), O_APPEND entries survive a seek to 0, a SIGKILLed
  holder's lock is immediately re-acquirable, and the end-to-end warm-then-read flow through
  InSituDataset.
• uv run ruff check src tests bench examples, uv run mypy src bench examples and
  uv run pytest -q are green locally (416 passed, 6 skipped)
• Docstrings and API docs for any new or changed public surface (cache_dir /
  readonly_cache on both InSituDataset and ChunkPool)
• User-facing behavior documented in docs/*.md — docs/tuning.md, "Sharing a
  cache_dir between processes"; platform support in docs/contributing.md and the README
• A bullet added under ## Unreleased in CHANGELOG.md
• No load-bearing invariant is broken — no framework, no per-sample Python, no dask, no
  reshard; the hot path is unchanged (the lock is taken once, at construction)
• Touches ChunkPool → free-threaded (3.13t) run passes: 364 passed, 29 skipped
  with PYTHON_GIL=0 and the GIL confirmed off
• No performance claim is made. _alloc gains one rename per chunk file creation (not per
  tile, not per batch), and construction gains one open+flock.

🤖 Generated with Claude Code

https://claude.ai/code/session_011iQ36j3rLYeh4R5uzmB52j

[emfdavid]