From 785d97ba9e3ae4389f214c9861c904c31dd65e99 Mon Sep 17 00:00:00 2001 From: Nicolas Date: Sun, 6 Sep 2026 09:27:12 +0200 Subject: [PATCH] =?UTF-8?q?ci:=20s=C3=A9rialise=20vraiment=20le=20job=20te?= =?UTF-8?q?st=20via=20un=20verrou=20docker=20inter-run?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Le run 122 (branche avec l'isolation par conteneur du commit précédent) a quand même échoué : sa base `postgres-122` était bien isolée et migrée proprement, mais le run 123 (ancien ci.yml) tournait en parallèle sur le même runner. Deux jobs `test` concurrents saturent le CPU (chacun entraîne un modèle spaCy pour son intent-service *et* déroule toute la suite Mocha DB) : un hook `resetDatabase()` dépasse alors le timeout Mocha de 10 s, Mocha enchaîne sur le hook suivant pendant que le TRUNCATE + reseed du précédent tourne encore, et les deux se percutent *dans la base d'un seul run* (`deadlock detected`, `Unique constraint failed on (key)`, intent-service affamé → F1 0.000). L'isolation par base ne suffit donc pas ; il faut « un seul job test à la fois », ce que `concurrency:` ferait nativement — indisponible ici (Forgejo 9.0.3). À la place : un verrou via le seul point commun à tous les jobs du runner, le démon docker. `docker network create ci-test-lock` échoue si le nom est pris → le premier run le détient, les autres bouclent jusqu'à libération (file d'attente, pas d'annulation). Libération en étape `if: always()` gardée par un marqueur `/tmp` (un run qui a renoncé ne doit pas supprimer le verrou du détenteur) + purge d'un verrou de plus de ~35 min (détenteur tué avant sa libération). `timeout-minutes` passe à 60 pour couvrir l'attente en file. L'isolation `postgres-${GITHUB_RUN_ID}` reste en défense en profondeur (fenêtre de transition où d'autres branches sont encore sur l'ancien fichier, sans verrou). Co-Authored-By: Claude Sonnet 5 --- .github/workflows/ci.yml | 120 ++++++++++++++++++++++++++++++--------- 1 file changed, 94 insertions(+), 26 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c08a9c9..91f68eb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -66,18 +66,86 @@ jobs: test: runs-on: ubuntu-latest - # Runaway ceiling, not a target: the job takes ~16-19 min normally (runs - # 117-121). Mocha runs with no `--exit` (`apps/api/.mocharc.json`), so a - # single leaked handle would otherwise hang this job until the runner's - # global timeout — a `test` job zombied for ~1h once before being - # cancelled by hand. 40 min leaves roughly 2x headroom for a slow / - # contended runner while still capping a hang at well under that hour. - timeout-minutes: 40 + # The active job is ~16-19 min (runs 117-121); the rest of the budget is + # for time spent queued on the serialization lock below (a realistic + # pile-up is two, sometimes three, runs). Also a runaway ceiling: Mocha + # runs with no `--exit` (`apps/api/.mocharc.json`), so one leaked handle + # would otherwise hang the job until the runner's global timeout — a + # `test` job zombied ~1h once before being cancelled by hand. If a hung + # holder ever hits this ceiling its `if: always()` "Release the test-job + # lock" step still frees the queue (and the stale-lock sweep frees it + # sooner, at ~35 min). + timeout-minutes: 60 steps: - name: Install curl and the docker CLI run: | apt-get update && apt-get install -y curl docker.io + # ── Serialize the `test` job across every concurrent run ───────────── + # GitHub's `concurrency:` is the native "only one `test` job at a time, + # across all branches", but this Forgejo (9.0.3) predates Actions + # `concurrency` entirely — it landed in Forgejo v14 / Gitea 1.26, and + # the key is silently ignored on anything older. The one resource every + # job on this runner shares is the docker daemon, so a uniquely-named + # docker object is the mutex: `docker network create` fails if the name + # is taken, so the first run to create `ci-test-lock` holds it and the + # others spin until it is freed — queue, don't cancel (the task's + # preferred `cancel-in-progress: false`: a half-finished `main` run + # shouldn't be killed by a feature push). + # + # Why serialize and not just give each run a private database (it gets + # one too, next step): two `test` jobs on this single runner also + # contend for CPU — each trains a spaCy model for its own + # intent-service *and* runs the full Mocha DB suite — and under that + # load a `resetDatabase()` hook overran Mocha's 10s timeout; Mocha then + # started the next file's hook while the timed-out one's TRUNCATE + + # reseed was still in flight, and the two collided inside one run's own + # database (`deadlock detected`, `Unique constraint failed on (key)` in + # `seedReferenceData`, intent-service starved to F1 0.000). Run 122 + # failed exactly that way while run 123's `test` job overlapped it. + - name: Acquire the test-job lock + run: | + # Force-release a lock older than ~35 min (well past a normal + # ~19-min run): its holder was killed outright before the release + # step could run (SIGKILL skips `if: always()`), or is wedged. + # `timeout-minutes` on this job is the backstop for a holder that + # is merely slow. Fail safe: if the age can't be read, leave the + # lock alone rather than risk nuking a live holder's lock. + created="$(docker network inspect -f '{{.Created}}' ci-test-lock 2>/dev/null || true)" + if [ -n "$created" ]; then + # `.Created` renders as RFC3339 on newer daemons and as Go's + # "2006-01-02 15:04:05 -0700 MST" on older ones — strip a trailing + # timezone name so GNU `date -d` accepts either. + created="$(printf '%s' "$created" | sed 's/ [A-Za-z][A-Za-z]*$//')" + if created_epoch="$(date -d "$created" +%s 2>/dev/null)"; then + age=$(( $(date +%s) - created_epoch )) + if [ "$age" -gt 2100 ]; then + echo "test-job lock is ${age}s old — assuming abandoned, releasing it" + docker network rm ci-test-lock >/dev/null 2>&1 || true + fi + else + echo "could not read the test-job lock's age ('$created') — leaving it in place" + fi + fi + # Up to ~40 min of waiting (80 * 30s); past that the stale-release + # above would have fired for any real hang, so give up and let the + # re-run get through. + for i in $(seq 1 80); do + if docker network create ci-test-lock >/dev/null 2>&1; then + echo "test-job lock acquired" + # Marker checked by the release step so a job that gave up + # waiting (and never held the lock) can't delete another run's. + # /tmp persists across steps (same job container) and, unlike + # the workspace, isn't wiped by actions/checkout. + touch /tmp/ci-test-lock-held + exit 0 + fi + echo "waiting for the test-job lock, held by another run… attempt ${i}" + sleep 30 + done + echo "could not acquire the test-job lock within ~40 min" >&2 + exit 1 + # Postgres as a plain background container instead of a `services:` # block — see this workflow's top-level `env.DATABASE_URL` comment for # why. The runner mounts the docker socket and runs every job on one @@ -85,25 +153,13 @@ jobs: # the network is discovered from the job container itself rather than # hardcoded. # - # The container name carries the run id (`postgres-${GITHUB_RUN_ID}`) - # rather than a bare `postgres` *because* that network is shared by - # every job on this runner, across all branches. With a fixed name, - # two `test` jobs overlapping — a feature push landing within minutes - # of a `main` merge — both connected to the *same* container, and - # their `prisma migrate deploy` + `seedReferenceData` (root Mocha - # `beforeAll`) raced: `Unique constraint failed on the fields: (key)` - # in `seedReferenceData`, "No pending migrations to apply" when there - # should be some, `tech_step`/`diet` reference-row id shifts (+5) - # breaking unrelated assertions. Hit three times in one session (PRs - # #47, #53, #54), each needing an empty-commit re-trigger once the - # runner was idle. A GitHub-style `concurrency:` group on this job is - # the natural fix, but this Forgejo (9.0.3) predates Actions - # `concurrency` support entirely — it arrived in Forgejo v14 / Gitea - # 1.26 and the key is silently ignored on anything older. A per-run - # container name reaches the same end (no two runs share a database) - # with zero runner support required; the `if: always()` teardown step - # at the end of the job, plus the age-gated reap below, keep these - # per-run containers from piling up on the runner host. + # The container name still carries the run id (`postgres-${GITHUB_RUN_ID}`) + # even though the lock above already keeps `test` jobs from overlapping: + # defence in depth for the window where another branch is still on the + # pre-lock version of this file (its `test` job takes no lock and would + # collide on a bare `--name postgres`), and for a mis-fired stale-lock + # release. The `if: always()` teardown plus the age-gated reap below + # keep these per-run containers from piling up on the runner host. - name: Start Postgres in the background run: | pg_host="postgres-${GITHUB_RUN_ID}" @@ -190,6 +246,18 @@ jobs: - run: pnpm --filter api exec prisma migrate deploy - run: pnpm --filter api test + # Release the serialization lock as early as possible so the next + # queued `test` job can start — `always()` so a failed or timed-out job + # frees it too (the stale-lock sweep in "Acquire the test-job lock" + # only covers a job killed outright). Only if this job actually holds + # it: a job that gave up waiting must not remove the holder's lock. + - name: Release the test-job lock + if: always() + run: | + if [ -f /tmp/ci-test-lock-held ]; then + docker network rm ci-test-lock >/dev/null 2>&1 || true + fi + # Remove this run's Postgres even when a step above failed or the job # hit its `timeout-minutes` — with a per-run name nothing else ever # reuses or cleans it, and a bare `docker run` sibling outlives the job