Skip to content

cache: don't let shared-cache pruning kill concurrent builds - #14

Merged
EyalIO merged 2 commits into
weka-1.38from
cache-prune-temp-race
Aug 2, 2026
Merged

cache: don't let shared-cache pruning kill concurrent builds#14
EyalIO merged 2 commits into
weka-1.38from
cache-prune-temp-race

Conversation

@EyalIO

@EyalIO EyalIO commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Fixes the farm-wide build failures tracked in WEKAPP-651245:
https://wekaio.atlassian.net/browse/WEKAPP-651245

Root cause

CachePruner.doPrune() unlinked every temp file in the cache directory
unconditionally:

// Delete all temporary files.
deleteFiles(cachePath, filePattern ~ ".tmp???????");

No age check, no ownership check — contrast pruneForExpiry right below it,
which does gate on timeLastAccessed. The implicit assumption is that a temp
file in the cache dir is a leftover from a crashed compiler, so it is garbage.

That holds for a private cache. It does not hold for a cache shared between
concurrent compilers. Temp files are created inside the cache directory
(storeCacheFileName + ".tmp%%%%%%%") because an atomic rename() requires
source and destination on the same filesystem; their only privacy is the random
7-char suffix, which prevents name collisions but is matched exactly by
.tmp???????. So one compiler's prune pass deletes other compilers' in-flight
temp files, their rename() fails with ENOENT, and cacheObjectFile() calls
fatal() — killing an otherwise healthy compilation.

Evidence from the affected build farm

Weka shares one LDC cache across 22 build servers on a cluster filesystem.

  • ircache_prune_timestamp had a byte-identical mtime on three separate
    builders, i.e. one prune clock for the whole farm: every
    -cache-prune-interval, whichever compiler process notices the stale
    timestamp sweeps temp files on behalf of every machine.
  • 12 build failures over 4 days, each the only error in a ~20,000-line build
    log, on an object that had already been written successfully.
  • Measured ~0.53 new cache entries/sec farm-wide with a temp file caught in
    flight; ~4% of sweeps landing on a temp implies an ~80ms temp lifetime, which
    matches a copy_file + rename of a ~34MB object.

Changes

  1. cache: don't delete in-flight temp files when pruning a shared cache
    only remove temp files older than a grace period (1 hour), so genuine
    orphans are still cleaned up but live ones survive.

  2. cache: warn instead of aborting when storing into the cache fails
    defense in depth. Storing is a pure optimization: the object file has
    already been written to its output location by the caller and nothing in the
    compilation reads it back, so a store failure means "cache miss next time",
    not "build invalid". recoverObjectFile() deliberately keeps fatal() — it
    removes the output file before copying from the cache, so a failure there
    really does lose the object.

Commit 1 fixes the known race; commit 2 makes any future cache-write failure a
slower build rather than an outage. They are independent if you only want one.

Testing

  • driver/cache_pruning.d compiles standalone (ldc2 -o- -c); the module has
    no LDC dependencies by design.
  • driver/cache.cpp compiles with the project's real build flags extracted
    from a configured ninja tree.
  • Full GitHub Actions build matrix runs on this branch.

🤖 Generated with Claude Code

EyalIO added 2 commits August 1, 2026 13:39
The prune sweep unlinked every ircache_*.tmp??????? unconditionally, on the
assumption that a temp file in the cache dir is a leftover from a crashed
compiler. That holds for a private cache but not for one shared between
concurrent compilers: it deletes temp files other processes are still
writing, so their rename() fails with ENOENT and cacheObjectFile() calls
fatal(), killing an otherwise healthy compilation.

Only remove temp files older than a grace period, so genuine orphans are
still cleaned up but live ones survive.
Storing is a pure optimization - the object file has already been written
to its output location and nothing in the compilation reads it back - yet
every failure in cacheObjectFile() called fatal(), so a cache-write hiccup
killed the whole compilation. Warn and skip caching instead; the worst
consequence is a cache miss next time.

recoverObjectFile() keeps fatal(): it removes the output file before
copying from the cache, so a failure there really does lose the object.
@EyalIO
EyalIO requested a review from ehudt-weka August 1, 2026 10:59
@EyalIO
EyalIO merged commit e2a68e5 into weka-1.38 Aug 2, 2026
10 of 15 checks passed
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.

2 participants