feat(raw-reads-processing): use the deacon Python bindings instead of the CLI server

[corneliusroemer-agent]

The service ran a deacon server subprocess and shelled out to deacon --use-server filter once per request. The same deacon release (0.17.0) is published on PyPI with Python bindings, which load the index into our own process and filter against it directly. Same index, same thresholds, same verdicts — no daemon and no socket in between.

Why

Server mode is the only way the CLI can share one loaded index, and it is fragile in exactly the way that matters for a service. A client that disconnects before finishing a request puts the daemon into a busy loop it never recovers from: the process stays alive so the liveness probe keeps passing, but every later filter hangs. When the daemon does die it leaves its socket file behind, so subsequent calls fail with a misleading "connection refused" instead of anything about deacon. And any error inside the daemon reaches us as an uninformative panic, because the protocol between client and server has no way to express an error at all. On top of that the daemon handles one request at a time, so our concurrent requests queued up behind each other.

All of that is gone. There is no second process to die, wedge, or lose its socket, and errors arrive as ordinary Python exceptions with real messages.

It is also faster, because requests now filter in parallel instead of queueing. The bindings hold the index immutably and release the interpreter lock while filtering, so one loaded index serves many threads at once without being copied.

Measured against the real 3.28 GB panhuman index, driving the actual service over HTTP with 602,800 read pairs per request:

concurrent requests	wall clock	filtering work done	peak memory
1	0.25 s	0.25 s	4.61 GB
4	0.51 s	1.60 s	4.64 GB
8	0.99 s	5.26 s	4.65 GB

Memory stays flat as concurrency rises, which is the point: there is one index in RAM, not one per request. Index load at startup is about 5 seconds warm and 30 seconds cold, well inside the startup probe's budget.

What a reviewer should look at

One index now means one process. The index is roughly 4.5 GB deserialised onto the heap, not memory-mapped, so each process that loads it pays for its own copy. Sharing between requests is now between threads rather than between processes — which is what the socket bought us before. That costs nothing today because we already run a single replica with a single uvicorn worker, but it makes workers=1 load-bearing rather than incidental: a second worker would quietly load a second copy and blow the pod's 8Gi limit. There is a comment at the uvicorn config saying so. If we ever want several workers or several pods sharing one index, this is the wrong approach and we should talk about it before scaling.

The filter call can no longer be timed out, so deacon_filter_timeout_seconds is gone. Killing a subprocess on timeout was straightforward; a call that has handed control to Rust and released the interpreter lock cannot be interrupted from Python at all. Rather than leave a config key that silently does nothing, the bound is now on input size: max_input_file_bytes (20 GiB, per file) is checked while the file downloads, so an oversized submission is rejected before it reaches disk or deacon. Worth being explicit that this bounds a large input, not a hang — if a filter call ever wedges, that request waits forever and holds one of the concurrency slots, while /health still answers 200 because the index is loaded. That is the same "probe passes while filtering is broken" shape we just removed from the server, relocated rather than eliminated. A watchdog that fails the pod is the alternative; happy to add one if you'd rather not accept this.

Thread budget is now configurable and needs tuning. deacon_threads: 4 and deacon_max_concurrent_filters: 2 are starting values, not measured optima. Without a cap this would be dangerous: the endpoint is a plain def, so FastAPI runs it in a threadpool of about 40 slots, and eight worker threads in each of those is several hundred threads. A semaphore bounds how many filters run at once. Note the pod's 50m CPU request with no limit is unchanged by this PR — the old server also defaulted to eight threads.

What happens on input deacon rejects

Worth its own section, because this is where the old setup was at its worst and the change is easiest to under-sell.

Hand deacon two supposedly-paired files with different numbers of reads and it stops with "Incompatible record set sizes". Under the server that killed the daemon, and because only a clean shutdown removes the socket file, every subsequent submission then failed with a confusing "connection refused" until someone restarted the pod. One bad submission broke host filtering for everybody.

In-process, submitting a mismatched pair three times in a row against the real index returns HTTP 500 each time with deacon's own message in it, and the very next good submission returns 200. Health stays green, and four concurrent good requests afterwards still complete in 0.46 s, so no concurrency slot was leaked. Same for a truncated record, a corrupt gzip file, a file that is not FASTQ at all, and a missing file: each is an ordinary Python exception carrying a specific message, and the loaded index is unaffected. Even the case that makes the command-line tool abort outright — pointing it at a compressed index — merely raises here, because the bindings convert errors where the command-line tool unwraps them.

One case deacon itself does not object to is an empty file — it filters to zero reads and reports success. That is caught before deacon twice over, so it is a caution for anyone else calling the bindings rather than a hole here: readtools rejects an empty FASTQ (plain or gzipped) with "Cannot determine candidate qualities: no qualities found", and if it were ever skipped, sampling the read length to choose -a rejects it too.

Testing this turned up two gaps that are fixed in the third commit. Nothing caught the exceptions, so any of the above would have surfaced as a generic 500 with no logged reason — the old code turned a non-zero deacon exit into a ProcessingFailure, and that is restored along with the note about alerting when deacon starts failing. Separately, malformed FASTQ never reaches deacon at all: we sample the first hundred reads to decide whether the library is short-read, and Biopython's parser throws first. That was an uncaught ValueError; it now produces the same "file may be empty or corrupted" message the submitter already gets for an empty file.

Two things that would have been bugs in a literal port

Neither is visible from reading the old code, so flagging both.

The old call passed no output path. In the CLI that means "write to stdout", and it was harmless only because the server owned stdout and we had pointed it at /dev/null. In-process, stdout is ours, and we run deacon in search mode — meaning the reads it writes out are the ones that matched the human index. A faithful port would have printed human reads into the pod logs, which is the exact thing this service exists to prevent. Output now goes to /dev/null explicitly, with a comment. As a side effect this also stops us buffering those reads into memory, which the old capture_output=True did.

The bindings offer a check_pairs option that verifies R1 and R2 names line up, which looks like a natural safety net given we pass two files positionally. It is too strict to use: it rejected ordinary paired FASTQ from SRA whose names do end in /1 and /2, because a description follows the read name. Left off. Instead R1 and R2 are named explicitly at the call site rather than relying on dictionary ordering to put them in the right order.

Deliberately not changed

The conda deacon CLI stays in environment.yml even though the service no longer uses it: build-index.sh calls it, and the index-build workflow creates its environment from that same file. Removing it would have broken the nightly index build. There is a one-line comment recording why it is still there.

local_files is keyed by file name, so a submission with two files of the same name collapses to one entry and a paired submission would be filtered as if single-ended. That is pre-existing and the readtools step shares the assumption, so it belongs in its own change.

Notes on testing

The two deacon threshold tests used to be skipped whenever the deacon binary was missing from PATH, which means they may not have been running everywhere. The bindings are an ordinary Python dependency, so those tests now always run; they produce the same 75% and 5% host proportions as before, so behaviour is unchanged. The index is loaded once per test session, which is also how the service uses it.

New tests cover the new behaviour: that an oversized download is rejected part-way through rather than after being written in full, that the semaphore really does cap concurrent filters and does not leak a slot when a filter fails, and that a mismatched pair fails the submission while leaving the index usable. One more covers get_config against defaults.yaml, because nothing did — every test built Config directly, so a key missing from the defaults file would only have shown up as a validation error at pod startup with the whole suite green.

The image build is green on this PR, and uv resolved a prebuilt wheel rather than falling back to building from source, so the switch to a PyPI dependency needs no Rust toolchain in the image. The package publishes no deacon command-line script, so it cannot shadow the conda binary that build-index.sh needs.

🚀 Preview: https://raw-reads-deacon-python-b.loculus.org

[claude]

This PR may be related to: #7110 (raw-reads-processing: send slack notifications if deacon server panics/crashes and the pod needs to be restarted) — this PR removes the deacon server/daemon architecture entirely (in favor of in-process Python bindings) and turns deacon errors into handled ProcessingFailures, which addresses the root cause of the panic/crash scenario #7110 was raised for.