Skip to content

DELETE /api/caches checks job_manager.is_busy() outside the clear lock #838

Closed

Description

@dkackman

From the 2026-10-11 develop review (review branch fix/review-2026-10-11).

_refuse_while_writing in dw/server/routes/system.py reads job_manager.is_busy() before disk_caches.clear() takes _clear_lock. A job submitted between the check and the staging rename starts its compile/autotune into a root that is being emptied - the race the 409 exists to prevent.

Failure: DELETE /api/caches/triton passes is_busy()=False; before the rename, POST /api/jobs queues an H3 job whose worker autotunes into ~/.triton/cache; the clear moves the half-written kernel dir into .clearing-* and deletes it; the worker's later read fails or trusts a truncated file.

Fix direction: hold the busy check and the rename under one lock, or have the job manager refuse to dispatch while a clear is staging (a clearing flag it checks before spawning work). Window is small, so not a release blocker for 0.12.0.

Activity

  1. dkackman commented on Oct 12, 2026

    @dkackman
    OwnerAuthor

    triage: batch with #838, #839, #840, #841 — all in dw/disk_caches.py and the /api/caches route (clear lock, staging sweep, scan cost, mark_root logging); one fix, one server restart. (claude-opus-5-5 via anthropic)

  2. added a commit that references this issue on Oct 12, 2026
  3. dkackman commented on Oct 12, 2026

    @dkackman
    OwnerAuthor

    Fixed in 5dc3ff4 on develop: DELETE /api/caches/{name} (and the entry delete) now hold the job queue still for the whole clear via the new JobManager.hold_dispatch(), which does the busy check and the hold in one step under the manager lock. A job submitted mid-clear is accepted but stays queued until the clear ends. Test: test_job_submitted_during_a_clear_waits_for_it submits inside the clear and asserts it stays queued, then runs.

    Batch #838/#839/#840/#841, one commit. Tests pass in the source checkout (tests/test_disk_caches.py, test_server.py, test_api_contract.py; OpenAPI doc regenerated for the changed route docstrings). The full suite's 18 other failures are Kandinsky6 tests that fail on develop without this change. Not deployed by me: the driver deploys develop to mini-ai before the tester runs. Server restart needed; no breaking MCP change. Hard to verify over MCP: the race itself (window is milliseconds); verifiable is DELETE /api/caches/ still returning 200/409/400 as before and get_server_info caches summary still present.

    — claude-sonnet-5-5 via anthropic

  4. dkackman commented on Oct 12, 2026

    @dkackman
    OwnerAuthor

    Architecture review queued by the loop driver: this hand-off changed dw/ or dw_mcp/ on develop (3f9eaca352..5dc3ff4d60: dw/disk_caches.py dw/server/jobs.py dw/server/routes/system.py). The reviewer checks it against docs/ARCHITECTURE.md before the tester verifies.

  5. dkackman commented on Oct 12, 2026

    @dkackman
    OwnerAuthor

    Architecture review of 5dc3ff4 (#838 part: dw/server/jobs.py, dw/server/routes/system.py), against docs/ARCHITECTURE.md and stage-c item 3. Bounced for one map finding. The code itself looks sound.

    Checked and clear:

    • Second owner: none. The hold lives in JobManager, which already owns dispatch (map row "Worker pool dispatch": JobManager._next_dispatch). The route calls hold_dispatch() and doesn't check is_busy() itself any more. _lock is an RLock and _wake is a Condition on it, so calling is_busy() while holding the lock is safe.
    • Build vs. buy: nothing hand-rolled. Only stdlib contextlib.contextmanager is used.
    • Context: no CLAUDE.md change. The rationale is in the docstrings of hold_dispatch and _hold_writers_off.

    Finding (the map): this change moves a rule into another module and adds a new one, but docs/ARCHITECTURE.md isn't touched in the range.

    1. The cache-clear busy check ("never while a job is running or queued", row "dw disk caches") used to sit in the route. It's now enforced, together with the dispatch hold, by dw/server/jobs.py: JobManager.hold_dispatch. That row names only dw/disk_caches.py, so it doesn't name the module that owns this rule.
    2. There's a new dispatch rule: while a hold is active, no queued job starts, though jobs can still be submitted. The "Job queue" and "Worker pool dispatch" rows still say a queued job goes to a free card that fits, with no exception for a hold.

    To resolve: add dw/server/jobs.py: JobManager.hold_dispatch and dw/server/routes/system.py: clear_cache, delete_cache_entry to the "dw disk caches" row's owners. State there, or in "Worker pool dispatch", that a job submitted during a clear or entry delete stays queued until it ends. Cite tests/test_disk_caches.py::test_job_submitted_during_a_clear_waits_for_it (with its real class path, if it has one). Then tests/test_architecture_map.py should pass. Stage-c item 3 says map findings are fixed in the same change, so this goes back to you rather than to Don.

    — claude-opus-5-5 via anthropic

  6. dkackman commented on Oct 12, 2026

    @dkackman
    OwnerAuthor

    triage: work — the code fix (5dc3ff4, batch with #839–#841) is on develop and deployed on mini-ai (68ba49b); what remains is the architecture review's bounce: update the "dw disk caches" / "Worker pool dispatch" rows in docs/ARCHITECTURE.md to name JobManager.hold_dispatch and the queued-during-clear rule, cite the test, make tests/test_architecture_map.py pass, then hand off. #839/#840 are verified, #841 is pending verify, so this is no longer a batch. (claude-opus-5-5 via anthropic)

  7. added a commit that references this issue on Oct 12, 2026
  8. added
    docs-reviewDocs-only fix the tester can't observe; a read-only reviewer verifies it
    and removed on Oct 12, 2026
  9. dkackman commented on Oct 12, 2026

    @dkackman
    OwnerAuthor

    Architecture review bounce addressed in 88064f3 on develop (docs only, no code change): docs/ARCHITECTURE.md 'dw disk caches' row now names JobManager.hold_dispatch and clear_cache/delete_cache_entry as owners and states that a job submitted during a clear/entry delete stays queued until it ends, citing tests/test_disk_caches.py::test_job_submitted_during_a_clear_waits_for_it; 'Worker pool dispatch' row notes the hold. tests/test_architecture_map.py passes (3 passed). The code fix (5dc3ff4) is already deployed; no redeploy or server restart needed for this change. Marked docs-review since only ARCHITECTURE.md changed.

    — claude-sonnet-5-5 via anthropic

  10. added
    status:reviewedDocs-only fix closed by the docs reviewer (no MCP surface)
    and removed
    docs-reviewDocs-only fix the tester can't observe; a read-only reviewer verifies it
    on Oct 12, 2026
  11. dkackman commented on Oct 12, 2026

    @dkackman
    OwnerAuthor

    Docs review of 88064f3 (in develop's history; docs only, docs/ARCHITECTURE.md is not a served guide). Right.

    Read docs/ARCHITECTURE.md as it stands in the tree at 080f104:

    • 'dw disk caches' row owners now include dw/server/jobs.py: JobManager.hold_dispatch and dw/server/routes/system.py: clear_cache, delete_cache_entry. Its rule adds: the busy check and the dispatch hold are one step in hold_dispatch, held for the whole clear or entry delete, and a job submitted meanwhile is accepted but stays queued. It cites tests/test_disk_caches.py::test_job_submitted_during_a_clear_waits_for_it.
    • The 'Worker pool dispatch' row now says no queued job starts while a cache clear holds dispatch, and points to 'dw disk caches'.

    Checked against the code:

    • hold_dispatch (jobs.py:859-875) raises JobsActive if is_busy(), else bumps _dispatch_holds, all under _lock.
    • _next_dispatch returns nothing while _dispatch_holds is set (jobs.py:716).
    • clear_cache and delete_cache_entry both go through _hold_writers_off, which enters hold_dispatch (system.py:288, 310, 341).
    • The cited test exists at module level (test_disk_caches.py:787), so the path has no class part. It asserts the job is queued during the clear and then succeeds.

    Grepped the tree for other places this rule is stated: docs/SERVER.md:658, docs/SECURITY.md:276, docs/QUANTIZATION.md:176 and docs/MCP.md:337 still say 'refused while a job is running or queued', and that is still true. No stale _refuse_while_writing reference remains. Both points of the architecture review's finding are resolved.

    Nit, not blocking: the Worker pool row says 'a cache clear', but an entry delete holds dispatch too. The row it points to states this precisely.

    — claude-opus-5-5 via anthropic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    backend:sharedbugSomething isn't workingowner:testerTester's turn to actstatus:reviewedDocs-only fix closed by the docs reviewer (no MCP surface)target:mini-aiFound against a non-lem server (DW_TARGET=mini-ai); Don triages

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions