ci(benchmark): stop the FHIR benchmark from exhausting the shared Docker host

[mauripunzueta]

Hardens fhir-benchmark.yml so a failing benchmark can no longer take out the shared Docker host — and so the next failure is diagnosable.

Relates to #190 (the benchmark harness itself).

What happened on run 30550776427

That run did not merely fail. It exhausted the Docker host, which then failed every testcontainers-based job in the repo with disk quota exceeded for roughly three hours — Test Rust on unrelated PRs, UI Tests on fix/449-es-nightly, HFS Redis Cluster Tests on feat/cluster-capable-state. And it destroyed the evidence needed to explain itself.

The timeline is exact:

14:30:12   import starts
14:52:20   0497/1000 bundles — steady ~23/min for 22 minutes
14:52:21   0502/1000
14:52:40   0999/1000        ← 503 iterations "complete" in 19 seconds
14:58:38   search setup logs the cause

That is not a speed-up; it is every remaining request failing instantly, and 503 ≈ the 504 failed checks. The max request duration was exactly 15m0s = HFS_REQUEST_TIMEOUT=900, so the 20 in-flight requests all hit the server-side ceiling simultaneously.

The search suite's setup() had the answer all along:

sampleIds Patient: HTTP 503 … "Error occurred while creating a new object:
                               error communicating with the server"
sampleIds Encounter: HTTP 503 … "error connecting to server"

HFS was up, returning well-formed OperationOutcomes. PostgreSQL was gone. That also resolves the one number I could not explain from the summary alone: the search suite's 4,796,883 requests at ~40k/s with 699µs average latency and 100% failures were fast 503s from a server that never touched a database. Likewise crud's 12,139 TypeErrors — those were error bodies, not resources.

Root cause: Postgres was started with no volume, so its data lived in the container's writable layer on the daemon's own storage — precisely the filesystem that reported disk quota exceeded. The runner was then killed at 15:04:28, and a killed runner does not run if: always() steps, so the cleanup at the end of the job never executed and the containers were orphaned still holding the import.

The four changes

1. Postgres data on a named, labelled volume (-v "$PG_VOLUME:/var/lib/postgresql").

Mounted at the image's declared VOLUME root rather than at $PGDATA, so it keeps working across postgres image versions that relocate PGDATA within that tree. The point is reclaimability: container-layer data cannot be freed independently of the container, so an orphan pins the space forever; a named volume can be dropped by label long after the container is gone.

2. A reaper at the START of the job.

Removes hfs-bench-* containers and dangling labelled volumes left by earlier runs. This is the load-bearing change — a cleanup step cannot defend against its own step never running, so the durable control has to be at the start.

Both conditions are re-checked in the loop rather than trusting --filter name= to anchor, because a reaper that deletes someone else's container would be worse than the leak it fixes:

case "$name" in hfs-bench-*) ;; *) continue ;; esac   # only ours
case "$name" in *"$RUN_ID"*) continue ;; esac          # never a live sibling

The run-id exclusion is also what stops the concurrent sqlite and postgres matrix legs — which share a run id — from killing each other. I verified the filter against a hostile candidate list including some-other-teams-postgres, my-hfs-bench-lookalike, prod-db, and this run's own three containers: only the two genuinely stale ones are reaped, even if the docker filter returns everything.

3. A disk canary before anything is allocated.

docker system df reports image and volume usage but not a filesystem quota, which is what actually bit us. The only reliable check is to ask the daemon to do the thing we need it to do — create a container and write to its layer. Catches an exhausted host in ~10 seconds with the remedy printed, rather than 20 minutes in with a confusing symptom and more debris.

It does not catch a merely tight host — 256 MiB is a floor, and a host with 1 GB free would pass here and still fail during the import. That trade is documented at the call site.

4. Artifact uploads move ahead of the post-processing steps.

Upload results and Upload server log sat after per-shape attribution and summary generation — which is exactly where this run died. So the server log and raw results, the only outputs that cannot be recomputed, were lost for precisely the run that needed them. I could not retrieve HFS's own log for this investigation because of that ordering. Everything after the uploads is derived data; upload first, analyse second.

Scope — what this does not do

• It does not make the import faster, and it does not tell us whether it is slow. The median request was 120ms, but 20 VUs over 22 minutes producing only 497 bundles means most iterations were far slower than the median. "HFS is slow" cannot be separated from "the host was already degrading underneath it" without a clean run. Getting that measurement is what these changes make possible; it is the natural follow-up.
• It does not bound the corpus. A named volume still consumes host disk. What changes is that the space is addressable and reaped, not that it is smaller.
• crud.js:47 still derefs the response body unguarded (getRtId), turning a dead backend into 12,139 stack traces instead of a clean failure count. That file is vendored from HealthSamurai/fhir-server-performance-benchmark, so it needs an upstream fix or a deliberate local patch — out of scope here.
• No health gate between suites. crud and search ran for six minutes against a dead database. Even at best only 496/1000 bundles had loaded, so those numbers were meaningless regardless — nothing asserts "import succeeded" before measuring search. Worth adding, but it changes what the workflow measures, so it belongs in its own change.

Verification

Workflow-only; no Rust touched. yaml.safe_load parses; every modified run: block passes bash -n with ${{ }} expressions stubbed; the reaper's filter logic was unit-tested against an adversarial name list as described above.

The real proof is a manual dispatch on a healthy host, which I have not run — it consumes the Docker host for ~50 minutes and I did not want to take it out from under in-flight CI without asking. Worth doing before merge, ideally after confirming the orphaned hfs-bench-* containers from run 30550776427 are actually gone (CI went green again at 17:53, so the host was reclaimed, but I cannot see whether that was a clean prune or a partial one).

[mauripunzueta]

Overlaps #452 (fix/ci-docker-volume-leak, opened minutes before this), which changes docker rm -f → docker rm -fv across 16 workflow files — including the same four lines of fhir-benchmark.yml this PR rewrites.

Same conclusion reached independently. #452 supplies a fact I didn't have and have now incorporated: the shared host runs a 200G ZFS quota, and the leaking objects are anonymous volumes, because the postgres/Elasticsearch/mongo images declare VOLUME and plain docker rm -f orphans them permanently.

Merge order: #452 first, then rebase this. For fhir-benchmark.yml this PR is a strict superset (it already does -v at all four sites), so the conflict resolution is "take this branch's version of that file". #452 fixes 15 workflows this PR does not touch, so it should not wait.

Reading #452 also surfaced a real bug here, now fixed in a101d0e6a: the reaper had no age guard. Excluding this run's own containers by run id covers the sqlite/postgres matrix legs (same run id), but not two different runs — the concurrency group only cancels in-progress runs on the same ref, so a dispatch on main and one on a PR branch run concurrently with different run ids and would have reaped each other's containers mid-benchmark. Now uses the same age-guard idiom as ci.yml's orphan sweep, at 120 minutes (a full leg is ~50 min; the import suite alone is ~22).

[mauripunzueta]

Verified end to end — dispatched on this branch

Run 30576511473, backend=postgres tests=prewarm, success.

Ran prewarm-only deliberately: it exercises every change in this PR (reaper → canary → volume → suites → uploads → cleanup) while writing almost nothing, so it was safe to run alongside in-flight CI on the shared host. The multi-GB import run is the one that needs a quiet host.

The reaper found the orphans from the failed run — they were still there.

── Reaping hfs-bench-* containers older than 120m ──
  removing hfs-bench-pg-30550776427 (b62225436d72, age 333m)
  removing hfs-bench-tgz-postgres-30550776427 (0cfecd355464, age 334m)
  reaped: 2

Those are exactly the two containers orphaned when run 30550776427's runner was killed, still holding the Synthea import 5½ hours later. CI going green earlier did not mean the host had been cleaned — enough space was freed elsewhere to unblock it, but the benchmark's own debris survived until this reaper removed it. That is the failure mode this PR exists to close, reproduced and fixed on the first real run.

Canary: passed, host has writable space.

Named volume, full lifecycle:

Container name: hfs-bench-pg-30576511473
Data volume:    hfs-bench-pgdata-30576511473
Postgres ready on ***:55142 after 6s
…
hfs-bench-pg-30576511473        ← container removed
hfs-bench-pgdata-30576511473    ← volume removed

Artifacts, which is the point of the reordering:

artifact	size
fhir-bench-server-log-postgres-30576511473	1,697 B
fhir-benchmark-postgres-30576511473	9,549 B

Run 30550776427 produced only the binary artifact — the log and results were lost because the uploads sat behind the step that died. Both now exist.

One finding for #452

docker system df on the host:

TYPE            TOTAL   ACTIVE   SIZE      RECLAIMABLE
Local Volumes   187     19       23.73GB   22.13GB (93%)

22 GB of reclaimable anonymous volumes against a 200 G quota. That is #452's leak, quantified — and it is untouched by this PR, which only manages its own named volume. Worth pruning on the host now, independently of either PR.

Still not done

The full import run, for an actual measurement — it needs a quiet host and ~50 minutes. Say the word and I'll dispatch it.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[claude]

Code review

Found 3 high-confidence issues, all in the new "Reap stale benchmark containers and verify Docker host storage" step. (Note: inline per-line comments were not available in this environment, so all findings are consolidated here with direct line links.)

1. Named Postgres volume is never actually pruned by the reaper

hfs/.github/workflows/fhir-benchmark.yml

Lines 304 to 308 in 43fd3c6

	# without touching anything another workflow owns.
	echo "── Pruning this workflow's dangling volumes ──"
	docker volume prune -f --filter "label=hfs-bench=1" 2>/dev/null || true
	echo "── Docker host usage ──"

Since Docker Engine 23.0, docker volume prune without -a/--all removes only anonymous volumes by default — a --filter label=... narrows that set further, it does not override the anonymous-only default. The Postgres data volume created earlier in this job is explicitly named and labelled (hfs-bench-pgdata-<run_id>, via docker volume create --label hfs-bench=1 ...), so it is never anonymous and this prune will never remove it. That is the exact multi-GB volume this PR is trying to reclaim (per its own root-cause analysis of run 30550776427) — it survives every reap.

Fix: docker volume prune -af --filter "label=hfs-bench=1" (still safe — prune only ever touches unreferenced volumes).

2. Canary cannot detect a genuinely full disk

hfs/.github/workflows/fhir-benchmark.yml

Lines 321 to 326 in 43fd3c6

	# debris" into "fails in 10 seconds with the remedy printed".
	echo "── Canary: create a container and write 256 MiB ──"
	if ! docker run --rm --name "hfs-bench-canary-$RUN_ID" alpine:3 \
	sh -c 'dd if=/dev/zero of=/canary bs=1M count=256 2>&1 | tail -1'; then
	echo "::error::Docker host cannot create a container or has no writable space."
	echo "The benchmark writes several GB (Synthea corpus + Postgres data), so it"

sh -c 'dd if=/dev/zero of=/canary bs=1M count=256 2>&1 | tail -1' runs under Alpine's default /bin/sh (BusyBox ash), which does not enable pipefail. Per POSIX, a pipeline's exit status is the status of its last command — here tail, which succeeds even when its only input is dd's "No space left on device" error. So on a genuinely full host, dd fails but tail still returns 0, docker run exits 0, the if ! check is false, and the step prints "Canary OK — host has writable space" — the exact disk-full case this canary exists to catch, per the comment directly above it.

Fix: drop the pipe so dd's own exit status propagates, e.g. sh -c 'dd if=/dev/zero of=/canary bs=1M count=256'.

3. Canary container name is not scoped per matrix leg — concurrent legs can collide

hfs/.github/workflows/fhir-benchmark.yml

Lines 321 to 326 in 43fd3c6

	# debris" into "fails in 10 seconds with the remedy printed".
	echo "── Canary: create a container and write 256 MiB ──"
	if ! docker run --rm --name "hfs-bench-canary-$RUN_ID" alpine:3 \
	sh -c 'dd if=/dev/zero of=/canary bs=1M count=256 2>&1 | tail -1'; then
	echo "::error::Docker host cannot create a container or has no writable space."
	echo "The benchmark writes several GB (Synthea corpus + Postgres data), so it"

The sqlite and postgres matrix legs share the same ${{ github.run_id }} and run concurrently by design (the TGZ_CONTAINER name a few lines below is explicitly scoped by ${{ matrix.backend }} for exactly this reason, and this step's own comment notes "The matrix legs (sqlite, postgres) share a run id and run concurrently"). But the canary's name is hfs-bench-canary-$RUN_ID — run-id only, no backend. Both legs reach this step at roughly the same point (both start from build, no serialization or max-parallel), so the second docker run --name ... can fail with a container-name conflict, and that leg then reports the misleading "Docker host cannot create a container or has no writable space" error even though the host is fine.

Fix: scope the name by backend (e.g. hfs-bench-canary-${{ matrix.backend }}-$RUN_ID, matching the TGZ_CONTAINER pattern), or drop --name entirely since the container is --rm and nothing looks it up by name.