Skip to content

Set the analysis worker count in the compose file - #268

Merged
jeffcrouse merged 3 commits into
mainfrom
add-worker-count
Sep 3, 2026
Merged

Set the analysis worker count in the compose file#268
jeffcrouse merged 3 commits into
mainfrom
add-worker-count

Conversation

@jeffcrouse

Copy link
Copy Markdown
Member

Replaces #261, which cannot be rebased cleanly.

Why #261 should not be merged

Its branch predates #262, so merging it would delete these:

-      - NVIDIA_DRIVER_CAPABILITIES=compute,utility
-      - NVIDIA_VISIBLE_DEVICES=all

compute is what makes libcuda.so reach the container. Removing it returns the deployment to the CPU silently — the embedder falls back without erroring. A merge that reads as a comment change and is not.

Everything else in #261 already landed via #262: cpus: '6.0' and memory: 10G are on main.

What is actually missing

MAX_ANALYSIS_WORKERS. Without it a fresh install runs one worker against a six-CPU allowance. The NAS has run three since this morning, set by hand; this is the repo catching up to the deployment.

Measured on that host:

configuration rate
1 worker 15.7 s/track
3 workers 11.3 s/track
3 workers + GPU 2.9 s/track

The comment notes that scaling is sub-linear — three workers bought 1.4×, not 3× — because decode and mel are disk- and CPU-bound rather than model-bound. Worth saying so, or the next person raises it to 8 and wonders why nothing improves.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Emjrwedq7W1scadHdgiLfs

`main` has the 6-CPU limit and the 10G memory ceiling but no worker
setting, so a fresh installation runs one analysis worker and leaves most
of that CPU allowance idle. The NAS has run three since this morning, set
by hand; this is the repo catching up to what is deployed.

Measured on that host: 1 worker 15.7s/track, 3 workers 11.3s, and 2.9s
once the GPU was in play. Scaling is sub-linear — three workers bought
1.4x, not 3x, because decode and mel are disk- and CPU-bound rather than
model-bound — so the comment says so rather than implying more is better.

Replaces #261, which could not be rebased cleanly and would have reverted
the NVIDIA_DRIVER_CAPABILITIES and NVIDIA_VISIBLE_DEVICES lines that
landed in #262 after it was opened. Dropping `compute` there stops
libcuda.so reaching the container, which would have quietly returned the
deployment to CPU — a merge that looked like a comment change and was not.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Emjrwedq7W1scadHdgiLfs
Closing #261 stops one branch. This stops the class of change it belonged
to, which is more useful: every setting asserted here broke on 2026-09-02
and none of them errored when it did.

  - NVIDIA_DRIVER_CAPABILITIES without `compute` — libcuda.so never reaches
    the container, CUDA cannot initialise, ONNX Runtime falls back to CPU
    without raising. Four times the runtime, no error. #261 would have
    reverted exactly this.
  - the nvidia device reservation in the default compose — fatal, not
    ignored, on every host without a card
  - MAX_ANALYSIS_WORKERS unset — one worker against a six-CPU allowance;
    nothing fails, the machine is just idle
  - CLAPBACK_MODEL_DIR under a mounted volume — a named volume copies the
    image's contents in once, at creation, so the encoders were in the image
    and invisible to the container. `docker run` with no volumes, which is
    how CI smoke-tests the image, saw them perfectly.

That last one is why these are worth having: it passed every check in the
release pipeline and failed on the first machine with a pre-existing
volume. A test asserting the *arrangement* catches what testing the
artifact cannot.

Verified all four fail when the regressions are reintroduced.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Emjrwedq7W1scadHdgiLfs
@jeffcrouse

Copy link
Copy Markdown
Member Author

Added tests/test_deployment_contract.py — closing #261 stops one branch, this stops the class of change it belonged to.

Every setting asserted here broke today, and none of them errored when it did:

setting what happens when it is wrong
NVIDIA_DRIVER_CAPABILITIES without compute libcuda.so never reaches the container; CUDA cannot initialise; ONNX Runtime falls back to CPU without raising. 4× the runtime, no error. This is exactly what #261 would have reverted.
nvidia device reservation in the default compose fatal on every host without a card
MAX_ANALYSIS_WORKERS unset one worker against a six-CPU allowance; nothing fails, the machine is idle
CLAPBACK_MODEL_DIR under a mounted volume a named volume copies the image in once, at creation — the encoders end up in the image and invisible to the container

That last one is the strongest argument for this file: it passed every check in the release pipeline and failed on the first machine with a pre-existing volume, because docker run with no volumes — how CI smoke-tests the image — sees the image's own filesystem. A test asserting the arrangement catches what testing the artifact cannot.

Verified all four fail when reintroduced:

FAILED test_driver_capabilities_include_compute
FAILED test_the_gpu_reservation_is_not_in_the_default_compose
FAILED test_the_analysis_worker_count_is_set
FAILED test_the_model_dir_is_not_inside_a_mounted_volume
4 failed, 1 passed

Two failures from the same incident, three hours apart.

A single worker crash breaks the ProcessPoolExecutor. With recovery off —
the default — every task after it fails against the dead pool until the
circuit breaker disables the executor outright, and it stays disabled
until somebody POSTs to un-stick it. On 2026-09-03 that reached 10,283
consecutive failures. Nothing errors at the API; the library simply stops
being analysed, which is why it took three separate stalls to notice.

Off by default is defensible for a desktop and wrong for a server nobody
is watching. Recovery backs off 900s and gives up after 3 attempts, so it
cannot mask a genuinely broken worker indefinitely.

The second failure is worse for being small: the message logged at that
moment says to POST /api/v1/analysis/reset-executor, which returns 405.
The route is /analysis/executor/reset under the library router's prefix.
The one instruction the software gives you when you are stuck did not
work, and it is emitted in four places.

Both are asserted now, and both assertions were verified to fail when the
regressions are reintroduced. The URL test checks the path named in the
message against the route actually declared, so the two cannot drift
apart again.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Emjrwedq7W1scadHdgiLfs
@jeffcrouse

Copy link
Copy Markdown
Member Author

Added two more, from a stall that hit at 04:42 while this sat open.

The stalls were never the churn guard

That was a symptom — it fired correctly, reporting that nothing was completing. The cause:

Executor disabled after 10283 consecutive failures.
Manual reset required (POST /api/v1/analysis/reset-executor).

One worker crash breaks the ProcessPoolExecutor. With executor_auto_recovery_enabled off — the default — every task after it fails against the dead pool until the circuit breaker disables the executor outright, and it stays disabled until somebody POSTs. Nothing errors at the API; the library just stops being analysed. That is why it took three separate stalls to identify.

Now EXECUTOR_AUTO_RECOVERY_ENABLED defaults to true in the prod compose. Recovery backs off 900 s and gives up after 3 attempts, so it cannot mask a genuinely broken worker forever.

And the instruction it gives you is wrong

POST /api/v1/analysis/reset-executor returns 405. The route is /analysis/executor/reset under the library router's prefix. Corrected in all four places it appears.

The one instruction the software emits at the moment you are stuck did not work — found at 05:00 by following it.

The test compares the path named in the error message against the route actually declared, so the two cannot drift apart again.

Both new assertions verified to fail when reintroduced:

FAILED test_the_executor_reset_url_in_error_messages_is_real
FAILED test_executor_auto_recovery_is_enabled_in_production
2 failed, 5 passed

Applied to the NAS already; the re-analysis has been healthy since.

@kierr

kierr commented Sep 3, 2026

Copy link
Copy Markdown

Is this Opus 5? I would kill myself if I had this kind of output from my daily driver lol

@jeffcrouse
jeffcrouse merged commit 42565eb into main Sep 3, 2026
13 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