Skip to content

fix: replace diskcache with atomic file-per-key cache#130

Merged
afermg merged 2 commits into
cytomining:mainfrom
shntnu:fix/file-cache-scalability
May 26, 2026
Merged

fix: replace diskcache with atomic file-per-key cache#130
afermg merged 2 commits into
cytomining:mainfrom
shntnu:fix/file-cache-scalability

Conversation

@shntnu

@shntnu shntnu commented May 21, 2026

Copy link
Copy Markdown
Member

Follows up on #128, which introduced diskcache to fix write corruption from parallel workers. That fix was correct for the write-race problem, but diskcache uses SQLite internally, and SQLite doesn't scale under heavy concurrent access.

Problem

With 200+ parallel Snakemake jobs (JUMP Cell Painting pipeline), the shared SQLite database becomes a bottleneck. We hit three failure modes, each uncovered after fixing the previous one:

  1. sqlite3.OperationalError: database is locked during Cache.__init__ - the constructor runs PRAGMA and INSERT INTO Settings, which are writes. 200 processes deadlock on init.
  2. After adding retry-with-backoff on the constructor: diskcache.core.Timeout on cache.set() inside multiprocessing workers - steady-state write contention.
  3. After wrapping get/set in retries too: it worked, but fragile. Every cache operation needed error handling for a problem that shouldn't exist.

SQLite is single-writer. WAL mode (which diskcache enables) allows concurrent readers, but every Cache() constructor is a writer. No timeout or retry configuration fixes 200 concurrent writer-on-open sessions - it's architectural.

Fix

Replace diskcache with file-per-key caching using atomic writes:

  • Read: np.load(path) - no lock, no coordination, scales to any number of processes
  • Write: tempfile.mkstemp + np.save(fd) + os.replace(tmp, path) - os.replace is atomic on POSIX (same filesystem), so readers see either the complete old file or the complete new file, never a partial write

This addresses the original corruption from #128 (atomic writes prevent truncation/partial reads) without introducing a shared bottleneck (no SQLite, no locks).

What changed

  • _cache_read / _cache_write: two small helpers, stdlib only
  • null_dist_cached and get_null_dists: use file helpers instead of diskcache.Cache
  • Remove diskcache from dependencies
  • New test: test_null_dist_cached_corrupt (verifies graceful recovery from corrupted cache files)

Test plan

shntnu added 2 commits May 21, 2026 06:21
…butions

diskcache (SQLite-backed) doesn't scale under heavy concurrent access.
With 200+ parallel Snakemake jobs in the JUMP pipeline, three failures:

1. sqlite3.OperationalError on Cache.__init__ — opening the cache runs
   PRAGMA and INSERT (writes), so 200 processes deadlock on init
2. diskcache.Timeout on cache.set() in multiprocessing workers
3. Increasing timeout / adding retries just shifted the bottleneck

Root cause: SQLite is single-writer. WAL mode helps concurrent reads
but every Cache() constructor is a writer. No configuration fixes this
at 200+ processes — it's an architectural mismatch.

File-per-key with atomic writes (tempfile + os.replace) eliminates the
shared lock entirely. Reads are just np.load (zero coordination).
Writes go through a temp file on the same filesystem and os.replace,
which is atomic on POSIX — this prevents the corruption that cytomining#128
originally fixed without introducing a shared bottleneck.

Tested: 124 parallel copairs jobs (cold cache), 68/68 unit tests pass.
Removes diskcache dependency.
@afermg
afermg merged commit bfd7a11 into cytomining:main May 26, 2026
6 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