Skip to content

Give analysis three workers, and say why cpus alone was not enough - #261

Closed
jeffcrouse wants to merge 1 commit into
mainfrom
raise-analysis-workers
Closed

Give analysis three workers, and say why cpus alone was not enough#261
jeffcrouse wants to merge 1 commit into
mainfrom
raise-analysis-workers

Conversation

@jeffcrouse

Copy link
Copy Markdown
Member

The re-analysis from #258 was running at 31.7 s/track, ~230 hours. Two changes take it well below that, and the first cost nothing at all.

The CPU limit was already right in this file, and inert on the machine

docker-compose.prod.yml has said cpus: '6.0' since issue #13. The running container had 2.0. The comment directly above the limit already explains why:

deploy-dev.sh does docker cp + docker restart, which preserves the container's existing HostConfig — it will NOT apply a change made here.

So the fix needed no restart and no code:

docker update --cpus=6 familiar-api

31.7 s/track → 15.7 s/track. Measured over 600 s in both cases. ~230 h → ~112 h.

That is this project's recurring shape again — a change correct in the repo and absent from the running system — except here the file had already documented its own failure mode and nobody had run the one command.

Workers need a recreate, which has a trap

MAX_ANALYSIS_WORKERS is read from the environment at process start, so docker update cannot set it. Recreating the container discards whatever deploy-dev copied in — verified rather than assumed:

chunked-mean markers EMBEDDING_VERSION
running container 2 7
ghcr.io/...:latest image 0 6

After the recreate the container was serving middle-ten-second embeddings again until make deploy-dev was re-run. The sequence is now documented beside the setting.

Sizing

One worker leaves most of a six-core allowance idle — a track is one process, and max_tasks_per_child=1 means it does not even amortise its own interpreter start.

Memory goes 6G → 10G because each worker holds a CLAP model. The old comment claimed "~4GB peak"; measured reality was 1.57 G total with one worker running, so this is headroom rather than a requirement.

Applied to the NAS

Live now: 6 CPUs, 10 G, 3 workers, chunked-mean code restored and verified. Three-worker throughput is being measured.

Note: the NAS copy of this file had drifted from the repo — still cpus: '2.0', music mounted rw rather than ro, no mixtapes volume, plus ANTHROPIC_API_KEY/SPOTIFY_* this repo dropped with ADR-0048. I patched it surgically rather than overwriting, since reconciling that drift is a separate change and this one was made mid-re-analysis. Worth doing deliberately later.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Emjrwedq7W1scadHdgiLfs

The container was pinned at 2.0 CPUs while this file has said 6.0 since
issue #13. The comment above the limit already explained why: deploy-dev
does `docker cp` + `docker restart`, which preserves HostConfig, so the
change had been correct in the repo and inert on the machine for as long
as it has existed. `docker update --cpus=6 familiar-api` applied it
without a restart and took the re-analysis from 31.7s to 15.7s per track.

Three workers is the next step and needs more than `docker update` can
do, because MAX_ANALYSIS_WORKERS is read from the environment at process
start. One worker leaves most of a six-core allowance idle: a track is
one process, and max_tasks_per_child=1 means it does not even amortise
its own interpreter start.

Memory goes to 10G because each worker holds a CLAP model. The old
comment claimed ~4GB peak; measured reality was 1.57G total with one
worker running, so this is headroom rather than a requirement.

Recreating the container to apply the environment discards whatever
deploy-dev copied in — verified: the image still carries EMBEDDING_VERSION
6 and no chunked-mean code, so the container reverted to middle-ten-second
embeddings until deploy-dev was re-run. That sequence is now written down
beside the setting.

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

Measured: three workers, and the scaling is worse than expected

configuration rate remaining
1 worker, 2 CPUs 31.7 s/track ~230 h
1 worker, 6 CPUs 15.7 s/track ~112 h
3 workers, 6 CPUs 11.3 s/track ~80 h

All three measured over 600 s against count(*) where embedding_version >= 7.

Three workers bought 1.4×, not 3× — 46% efficiency. And it is not CPU or memory that stops it:

cpu  296% – 328%   (of 600% allowed)
mem  1.8 – 2.3 GiB (of 10 GiB)

Both have plenty of headroom, so the wall is disk I/O. That matches the per-track breakdown taken earlier, where one cold decode took 19 s against 1.4–1.8 s warm: three processes doing random reads off the array contend with each other, and with music streaming.

So this stops at 3. More workers would mostly thrash and would compete with playback on the machine that serves it. If this needs to go faster later, the lever is the storage path — not more processes.

The 10 G memory limit turns out to be unnecessary. Peak was 2.3 GiB with three workers, against an old comment claiming "~4GB peak" for one. It is harmless headroom, but nobody should read it as a requirement.

Net effect of both changes: ~230 h → ~80 h, and the first half of that came from a command the file had already documented.

@jeffcrouse

Copy link
Copy Markdown
Member Author

Superseded by the PR above — this branch predates #262 and merging it would revert NVIDIA_DRIVER_CAPABILITIES=compute,utility, which is what lets libcuda.so reach the container. Losing it returns the deployment to CPU silently.

Everything else here (cpus: '6.0', memory: 10G) already landed via #262. The replacement carries only the missing MAX_ANALYSIS_WORKERS, on top of current main.

@jeffcrouse

Copy link
Copy Markdown
Member Author

Closing so it cannot be merged by accident: this branch predates #262 and merging it would revert NVIDIA_DRIVER_CAPABILITIES=compute,utility, silently returning the deployment to CPU.

Replaced by #268, which carries the one thing still missing (MAX_ANALYSIS_WORKERS) on top of current main.

A closed PR only protects against this branch, so #268 also adds a test asserting the compose settings that are load-bearing and invisible when removed.

@jeffcrouse jeffcrouse closed this Sep 3, 2026
jeffcrouse added a commit that referenced this pull request Sep 3, 2026
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 added a commit that referenced this pull request Sep 3, 2026
* Set the analysis worker count in the compose file

`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

* Assert the deployment settings that fail silently when wrong

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

* Recover the executor automatically, and point its error at a real URL

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

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

1 participant