fix[next]: make OTF build caches crash-consistent#2691
Conversation
Atomic publish + validate-on-read + self-heal for the translation cache (FileCache), the gtfn build folder (gt4py.json / COMPILED marker), and the dace build folder (truncated compiled SDFG library).
Reworks the dace F4 fix for the new compile-only step: self-heal on load failure is no longer possible (load() has no build lock or dace config context), so the compile step now writes a .gt4py_compile_complete marker last and pre-cleans unmarked libraries. Also applies review fixes: umask-derived permissions in atomic_write_bytes, TypeGuard for is_usable, crash-safe test cleanup.
The shared compile_commands.json template was found by existence only and written non-atomically; a truncated template poisoned every program built with the same configuration. Publish atomically and validate on read.
|
claude: @egparedes regarding #2679: they are disjoint and complementary. No file overlap (verified with |
There was a problem hiding this comment.
Pull request overview
This PR improves crash-consistency of gt4py.next on-the-fly (OTF) build caches so that interrupted writes (e.g., SIGKILL/OOM, Ctrl-C, full disk) no longer leave “cache hit but unusable payload” states that break subsequent runs. It does so by making key cache writes atomic (write temp sibling + os.replace) and by treating corrupted/partial cache entries as misses that are evicted and rebuilt, aligning behavior with the durability expectations introduced by fingerprinting in #2648.
Changes:
- Add
gt4py._core.file_utilswithatomic_write_bytes/atomic_write_text, and use it to publish cache metadata/templates atomically. - Make cache reads more defensive and self-healing: corrupted pickles (
FileCache) and corrupted build metadata (gt4py.json) are treated as misses (evicted / rebuilt) rather than propagating decode/unpickle errors. - Add targeted unit tests covering previously-failing interruption windows for FileCache, gtfn build folders, compiledb template cache, and DaCe build-folder cache.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/gt4py/_core/file_utils.py |
New atomic write helpers used to publish cache artifacts without torn writes. |
src/gt4py/_core/filecache.py |
Atomic publish for entries; corrupt/truncated entries are evicted and surfaced as KeyError (cache miss). |
src/gt4py/next/otf/compilation/build_data.py |
Atomic publish of gt4py.json; treat unreadable/corrupt metadata as “no build data”. |
src/gt4py/next/otf/compilation/compiler.py |
Consolidate “cache hit is usable” logic into is_usable (compiled + module exists). |
src/gt4py/next/otf/compilation/build_systems/compiledb.py |
Atomically publish compiledb template and validate/evict corrupt templates on lookup. |
src/gt4py/next/program_processors/runners/dace/workflow/compilation.py |
Add completion marker protocol to force rebuild when a cached DaCe library exists but was left incomplete. |
tests/core_tests/unit_tests/test_filecache.py |
New tests for truncated/corrupt FileCache entries being treated as misses and self-healing. |
tests/next_tests/unit_tests/otf_tests/test_workflow.py |
New test ensuring persistent CachedStep recomputes instead of surfacing unpickling errors. |
tests/next_tests/unit_tests/otf_tests/compilation_tests/build_systems_tests/test_cache_consistency.py |
New tests for rebuilding when gt4py.json/module/template are corrupt/missing. |
tests/next_tests/unit_tests/program_processor_tests/runners_tests/dace_tests/test_dace_cache_consistency.py |
New test ensuring DaCe rebuilds when a truncated library exists without the completion marker. |
docs/development/ADRs/next/0024-Crash_Consistent_Build_Caches.md |
ADR documenting rationale, design choices, and known gaps. |
There was a problem hiding this comment.
This file is so generic that could belong to eve
| import uuid | ||
|
|
||
|
|
||
| def atomic_write_bytes(target: str | os.PathLike, data: bytes) -> None: |
There was a problem hiding this comment.
Can we unit test this?
Description
An interrupted pipeline (Ctrl-C, SIGKILL/OOM, full disk) can leave a cache entry that a lookup reports as a hit but whose payload is truncated or missing; the next run then fails on load instead of rebuilding. Fingerprinting (#2648) made cache keys correct but says nothing about payload completeness.
This PR makes cache writes atomic (temp sibling +
os.replace, new helpergt4py._core.file_utils) and cache reads validating + self-healing (any load failure is treated as a miss and rebuilt). No fsync: process kill is covered by the atomic rename, a power-loss torn write by validate-on-read. Rationale and alternatives in ADR 0024 (part of this PR).Fixed, each with a test that reproduced the failure first:
The existing partial-build resume behavior of the gtfn status machine is preserved.
Known gap (documented in the ADR): a gtfn module truncated after a completed build (power-loss torn write) is not self-healed.
Requirements