Remove flaky test_concurrent_eviction_does_not_delete_replaced_file

[jpascucci-nv]

Description

Refs #2745

(from Sol 5.6 Ultra)
The failure is a race in the test’s shutdown protocol, not clear evidence of a cache bug. A valid failing interleaving is:

1. The rewriter writes survivor.
2. The churner’s final size-cap pass legitimately evicts survivor; the 5,000-byte cap only holds two ~2,000-byte entries.
3. The churner exits.
4. The parent sets done_event.
5. The rewriter observes the event before its next write and exits.
6. The final lookup returns None.

The stat guard only prevents deletion when the file changes between the eviction snapshot and its second stat(). It does not protect survivor from ordinary LRU eviction. There is also an explicitly documented residual race between the second stat() and unlink().

Supporting evidence:

• Both subprocesses exited successfully; only the final survival assertion failed.
• Repetitions 1–3 and 5 passed; repetition 4 failed under Python 3.14t with parallel tests and --count=5.
• The same failure was previously reported on an unrelated PR and identified as racy.
• A deterministic test already forces replacement between the scan and re-stat and verifies the intended guard at test_program_cache.py (line 1697).

I recommend removing the stochastic multiprocess test at test_program_cache_multiprocess.py (line 148), since the deterministic test covers the property reliably. Simply serializing it or adding a final rewrite would only hide the race, not validate the stat guard.

Checklist

• New or existing tests cover these changes.
• The documentation is up to date with these changes.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: NVIDIA/cuda-python/.coderabbit.yaml
• Review profile: CHILL
• Plan: Enterprise
• Run ID: ba190903-54f2-4179-bddd-fe5907246a40

📥 Commits

Reviewing files that changed from the base of the PR and between 66387f1 and 2a7e0cb.

📒 Files selected for processing: 1

• cuda_core/tests/test_program_cache_multiprocess.py

💤 Files with no reviewable changes: 1

• cuda_core/tests/test_program_cache_multiprocess.py

📝 Summary

Summary by CodeRabbit

• Tests
  • Removed a multiprocess test covering concurrent cache eviction and rewrites.

The change removes a multiprocess test that raced rewrites of a survivor entry against size-cap eviction churn, along with its writer helpers.

Changes: Program cache test removal

Layer / File(s)	Summary
Remove concurrent eviction test cuda_core/tests/test_program_cache_multiprocess.py	Removes the writer and rewriter helpers and the test that checked whether survivor remained present with the rewriter’s payload after concurrent churn.

Suggested reviewers: cpcloud

Priority: ⬇️ Low

Change: Other

Merge Risk: ⚪ Minimal · up to 2a7e0

This change removes a flaky test and does not affect runtime behavior. It carries no actionable merge risk.

- [ ] Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

✨ Finishing Touches: 💡 1

🛠️ Fix failing CI checks: 💡

• Commit to this branch
• Create a new PR

[github-actions]

Doc Preview CI
Preview removed because the pull request was closed or merged.

[rwgk]

🤖 Written by Claude Fable 5.1 with Claude Code, posted at rwgk's request.

Verdict: agree with removing the test. The diagnosis in the description holds up against the code, and I would go one step further: the test could not have caught the bug it was written for, so there is nothing worth rescuing.

Findings

1. The test asserted a property the cache never promised. _enforce_size_cap evicts oldest-atime entries until the total is under the cap (_file_stream.py#L716). The stat guard (L728) only skips an unlink when the file changed between the scan's stat (L696) and the re-stat (L724). A survivor written before the scan is an ordinary eviction candidate. No race is needed:

   with FileStreamProgramCache(root, max_size_bytes=5000) as cache:
       cache[b"survivor"] = b"X" * 2000
       time.sleep(0.02)
       cache[b"churn-0"] = b"X" * 2000
       time.sleep(0.02)
       cache[b"churn-1"] = b"X" * 2000  # third entry: over cap, oldest atime goes
       assert cache.get(b"survivor") is None  # passes

   The removed docstring's claim that survival "has to come from the in-race rewrites being preserved by the stat-guarded eviction path" misread the guarantee.

2. The failure reproduces on correct code. I ran the removed test's logic with the rewriter slowed between writes, to mimic a descheduled process. Both subprocesses exited 0 in every trial, matching the CI signature (run 39178893396, linux-64 / 3.14t, [4-5] PARALLEL FAILED) and the earlier report by @seberg on #3104.

   rewriter stall between writes	survivor missing	nonzero exit
   0, 1, 2, 5 ms	0/25 at each setting	0
   10 ms	1/25	0
   20 ms	12/25	0
   stat guard disabled (blind unlink), 0 and 2 ms	0/25 at each setting	0

3. The test had no power against the bug it targeted. With the stat guard disabled so eviction unlinks blindly (last table row), the test still passed 50/50. A wrongly deleted survivor is masked by the rewriter's next write, so the final state only records whether the last rewrite preceded the churner's last eviction. The outcome is a function of scheduling, not of the guard. That is why "make it not flaky" has no good target: the property is unobservable from the final state. A final uncontested rewrite would make it pass trivially while the docstring still claimed to validate the guard, which is worse than removal.

4. Coverage loss is marginal. This was the only test running eviction under real cross-process contention (the remaining three multiprocess tests never set max_size_bytes). The paths it exercised are covered deterministically in test_program_cache.py: the stat guard (test_filestream_cache_size_cap_does_not_unlink_replaced_file), eviction's re-stat racing a vanished file (test_filestream_write_over_cap_skips_vanished_files), and the startup scan racing a vanished file (test_filestream_open_with_size_cap_skips_vanished_entries). A slimmed cross-process smoke test asserting only clean exit codes would be sound if anyone wants it; I would not block on it.

5. Nit, commit message. The links point at /data/src/cuda-python/... absolute local paths, which resolve for nobody else, and the message is permanent once merged. Repo-relative paths or test names would be better, and "I recommend removing..." could become a declarative statement. Cosmetic.

6. Mechanics are clean. Pure deletion, both worker helpers removed with the test, module docstring still accurate, pre-commit (including ruff check/format) passes on the file.

Reproduction script for the table (runs against an installed cuda.core)

# Reproduces the removed test's logic with (a) a configurable stall between the
# rewriter's writes, to mimic a descheduled process, and (b) an optional
# "blind unlink" mode that disables the stat guard (the bug the test targeted).
# Usage: python repro.py [trials]
import multiprocessing as _mp
import os
import sys
import tempfile
import time


def _disable_guard():
    import cuda.core.utils._program_cache._file_stream as fs

    fs._stat_key = lambda st: (0, 0, 0)  # re-stat always "matches": eviction unlinks blindly


def churner(root, payload, cap, no_guard):
    from cuda.core.utils import FileStreamProgramCache

    if no_guard:
        _disable_guard()
    with FileStreamProgramCache(root, max_size_bytes=cap) as cache:
        for i in range(80):
            cache[b"churn-" + str(i).encode()] = payload


def rewriter(root, payload, cap, done, stall, n_writes, no_guard):
    from cuda.core.utils import FileStreamProgramCache

    if no_guard:
        _disable_guard()
    with FileStreamProgramCache(root, max_size_bytes=cap) as cache:
        i = 0
        while not done.is_set():
            cache[b"survivor"] = payload + str(i).encode()
            i += 1
            if stall:
                time.sleep(stall)
        n_writes.value = i


def run_once(root, stall, no_guard):
    from cuda.core.utils import FileStreamProgramCache

    payload, cap = b"X" * 2000, 5000
    ctx = _mp.get_context("spawn")
    done, n_writes = ctx.Event(), ctx.Value("i", 0)
    rw = ctx.Process(target=rewriter, args=(root, payload, cap, done, stall, n_writes, no_guard))
    ch = ctx.Process(target=churner, args=(root, payload, cap, no_guard))
    rw.start()
    ch.start()
    ch.join(timeout=60)
    done.set()
    rw.join(timeout=60)
    with FileStreamProgramCache(root, max_size_bytes=cap) as cache:
        survived = cache.get(b"survivor") is not None
    return survived, rw.exitcode == 0 and ch.exitcode == 0, n_writes.value


if __name__ == "__main__":
    trials = int(sys.argv[1]) if len(sys.argv) > 1 else 25
    settings = [(s, False) for s in (0, 0.001, 0.002, 0.005, 0.01, 0.02)] + [(0, True), (0.002, True)]
    with tempfile.TemporaryDirectory() as base:
        for stall, no_guard in settings:
            missing = crashes = 0
            writes = []
            for t in range(trials):
                survived, clean, n = run_once(os.path.join(base, f"{stall}-{no_guard}-{t}"), stall, no_guard)
                missing += not survived
                crashes += not clean
                writes.append(n)
            print(
                f"stall={stall * 1000:>4.0f}ms guard={'OFF' if no_guard else 'on '}  "
                f"survivor-missing={missing:>2}/{trials}  nonzero-exit={crashes}  "
                f"rewrites/trial avg={sum(writes) / len(writes):.0f}"
            )