Conversation
Fixes cleanup problems that exist on main today, before #1475 adds more backend legs to the same shared Docker host: - Reap this workflow's named volumes explicitly. On Docker >= 23, `docker volume prune --filter label=hfs-bench=1` only considers anonymous volumes, so it never matched the Postgres data volume; runs 34187014872 and 35867150861 logged "0B" reclaimed next to 76-84 GB of reclaimable volumes. - Guard the reaper's `docker inspect` so a container vanishing between `ps` and `inspect` no longer aborts the step under bash -e. - Put the backend in the canary and df container names; the sqlite and postgres legs of one run started in the same second and collided on them. - Stop suggesting `docker system prune -af --volumes` on a host that runs long-lived services. - Run teardown right after the artifact uploads. On run 33515369645 the runner was lost during "Attribute search latency", every later step was skipped, and its containers survived 6.4 days. - Give the benchmark job a 150-minute timeout from one setup output, derive the reaper's age guard from it (+30), and label the tgz and Postgres containers hfs-ci=true so docker-host-gc reaps orphans after 180 minutes. - Add a per-leg leak check at the end of the job, keyed on a new hfs-bench-leg label, since the shared host can't be inspected by hand. Refs #1475 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Extends fhir-benchmark.yml from sqlite + postgres to every storage backend with and without Elasticsearch: sqlite, sqlite-elasticsearch, postgres, postgres-elasticsearch, mongodb and mongodb-elasticsearch. Bare s3 is left out because it has no search, so the import and search suites cannot run on it; s3-elasticsearch is a follow-up. Dispatch: - backend: core (default; sqlite + postgres), all (6 legs), elasticsearch (the 3 composites), or any single backend. - max_parallel (default 1, max 2): legs share one 12 GB / 4-CPU Docker host, so running them together skews every leg's numbers. - es_heap, es_sync_mode (asynchronous = HFS default | synchronous), mongo_wt_cache_gb, hfs_mongo_max_connections. The setup job validates them all before the build starts. Per leg: - MongoDB runs as a single-member replica set (transaction bundles need one), with a capped WiredTiger cache and a 900 s transaction lifetime to match HFS_REQUEST_TIMEOUT. - Elasticsearch runs single-node. Yellow health is expected, because HFS creates every index with one replica. - A capacity gate waits up to 10 minutes for enough free memory on the Docker host and fails the leg rather than risk an OOM. - Containers and volumes carry the hfs-bench, hfs-ci and leg labels, so the reaper, docker-host-gc and the leak check all cover them. Measurement: - On ES legs, a drain gate runs before the search suite: a conditional DELETE that matches nothing acts as a barrier on the composite's sync queue, then ES counters must settle, then live primary and ES resource counts are compared. - Import completeness, per-type crud leftovers and a _summary=count cross-check are recorded, so legs that did not load the same data are flagged rather than silently compared. - Backend stats, a "how to read this leg" note and the tuning knobs go into the step summary and runner-info.txt. The new bash and Python live in .github/scripts/fhir-bench/, following .github/scripts/obs-ab, so the workflow keeps its orchestration and the scripts can be linted directly. Closes #1475 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013NudzWDu2yTGExYaxdTQYJ
Run 36410157709 showed the gate measuring the wrong machine: `docker info` reported MemTotal 12000 MB while a plain container's /proc/meminfo showed MemAvailable 61324 MB, so the gate would pass almost every time. The Docker daemon evidently runs inside a 12 GB limit that containers' /proc/meminfo does not reflect (likely an lxcfs-virtualised LXC, where even a bind-mounted meminfo would report free memory relative to the reading container's own cgroup). host-mem.sh now derives available memory from one source it can trust: docker info MemTotal minus the usage `docker stats` reports for every running container, minus 1024 MB for dockerd and non-container processes. If `docker stats` fails the reading is `unknown` rather than "nothing running", so the gate keeps polling and fails at its 10-minute budget instead of passing on a guess. The capacity gate, probe_host (host_mem_avail_mb + host_mem_source in host-contention.txt), runner-info.txt (capacity_mem_source), the step summary and diagnose.sh all use it. probe_host now runs before SUITE_START, so suite wall time no longer includes the probe itself. Refs #1475 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013NudzWDu2yTGExYaxdTQYJ
Run 36412022609 (sqlite-elasticsearch, asynchronous, prewarm+import+ search) showed that async mode cannot finish the import corpus inside k6's 60-minute cap: 313 of 1000 bundles succeeded, 14 hit the 900 s client timeout, and bundle latency p95 was 489 s. The drain gate then reported status=incomplete: SQLite held 562,649 live resources and Elasticsearch 367,985, with composite_secondary_sync_needs_reindex=0. As agreed for this case, es_sync_mode now defaults to synchronous (bundles batched via _bulk, refresh=wait_for). asynchronous, HFS's own out-of-the-box mode, stays available as an input, and its "How to read this leg" note now says import is expected to hit the cap. Refs #1475 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013NudzWDu2yTGExYaxdTQYJ
Dispatching cffc076 failed before any job ran: "failed to parse workflow: (Line: 1145, Col: 14): Exceeded max expression length 21000". A run: block that contains any ${{ }} is evaluated as one expression, and GitHub caps expressions at 21,000 characters. The "Run benchmark suites" block had grown to ~26,300 characters with 31 substitutions. Every matrix/inputs/github/needs value that block uses now comes in through the step's env: (BACKEND, BENCH_PORT, BENCH_RUN_ID, BENCH_IN_*, BENCH_SHA, BENCH_REF_NAME, BENCH_MAX_PARALLEL, BENCH_LEG_TIMEOUT_MIN), with the same `||` fallbacks done in bash, so the block is a plain string with no length cap. A comment above the env: block records the rule. No behaviour change. Refs #1475 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013NudzWDu2yTGExYaxdTQYJ
…d-matrix ci(bench): benchmark all six storage backends (#1475)
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Preparatory PR for #1475. This PR fixes cleanup problems in
.github/workflows/fhir-benchmark.ymlthat exist onmaintoday. #1475 will put up to six backend legs on the same shared Docker host, so these need fixing first. This PR adds no new backends, inputs or features.What changes
maindocker volume prune --filter label=hfs-bench=1never removes the Postgres data volume. On Docker ≥ 23, prune only considers anonymous volumes unless--allis passed.Total reclaimed space: 0Bwhile 76–84 GB of volumes were reclaimable-f, so Docker refuses to delete a volume still in use.docker inspect … | sedaborts the whole step underbash -e+ pipefail if a container disappears betweenpsandinspect--rmcanary finishes in exactly that window… || continuedocker system prune -af --volumesdocker-host-gc/action.yml:54-64rules that out on this host, because it runs long-lived serviceshfs-bench-*is this workflow's next runleg_timeout_min=150, which becomes the benchmark job'stimeout-minutesand the reaper's guard (+30). The tgz and Postgres containers are labelledhfs-ci=true, sodocker-host-gc(default age 180 min, compared againstStartedAt) reaps any orphan within hours. 150 plus the 5-minute cancel window stays under 180.hfs-bench-run+ a newhfs-bench-leg), and emits::warning::if any remainVerification
main. The shellcheck integration hangs on this file on Windows, onmaintoo.run:block (reaper, tgz, Postgres, leak check): 0 findings, same asmain.-f tests=prewarmon the default legs (sqlite + postgres, started together): success.180m(150 + 30). No live containers were in range.pruneline had never matched:hfs-bench-pgdata-33209821414,-33404996556and-33515369645, aged 26–30 days. The last one is the run whose runner was lost.hfs-bench-canary-sqlite-…,hfs-bench-canary-postgres-…) without colliding.Refs #1475
🤖 Generated with Claude Code