From 469d72555929d648ec39ddb7c896dffa810b4a6d Mon Sep 17 00:00:00 2001 From: Timothy Date: Fri, 17 Jul 2026 16:13:39 +0200 Subject: [PATCH] ci(406): apply the memory-swap rule to our own two sites; stop leaning on the peak reading [decisions-edit] Re-review of the fix commits came back MERGEABLE with three findings worth acting on. MEDIUM -- the PR documented a standing rule ("--memory without --memory-swap silently grants 2x in swap") and then didn't apply it to the two sites this repo owns: docker-build.yml's smoke container and scripts/migration-smoke.sh, both `--memory 2g` with no --memory-swap. Pre-existing rather than a regression, but a rule you don't follow in your own repo isn't a rule. The migration-smoke one matters most: it runs on the PROD host in the release path, so a runaway migration should die against its cap rather than quietly swap out the box serving media. LOW -- and this is the important one: the docs leaned "peak 8305 MiB is probably mostly reclaimable cache". An independent probe (full solution build, same CI image, shared compilation off) measured peak 9457 MiB / anon 7134 MiB / file 421 MiB. ANON DOMINATED. Having verified the *mechanism* (peak overstates because it counts page cache), I guessed the *magnitude* in the direction I preferred -- the exact failure this entry criticises, committed inside the entry criticising it. Corrected in ci-cd.md, decisions.md and on server-management#604 (where the previous comment could have led to an unsafe 6g cap). Consequences now recorded honestly: a 6g cap looks UNSAFE, #570's "6g proved too tight" is the rule not an outlier, and #406's premise ("if this brings peak RSS well under 6 GiB the whole budget loosens") is looking DEAD -- the 7134 MiB anon was measured with shared compilation already off. The switches remain right; the looser budget they were meant to buy does not follow. NIT -- dropped the unverified claim that this also disables the Razor build server. The UseRazorBuildServer -> UseSharedCompilation fallback is .NET 5-era; Razor has been an in-process source generator since .NET 6, so there is likely no separate server to disable on .NET 10. Unverified, zero impact, so it has no business in a doc arguing for measurement over assumption. [decisions-edit]: the touched docs/decisions.md lines were added by this PR's own earlier commits, not settled entries on main -- net vs origin/main remains a pure insertion (0 deletions, verified). Also the sanctioned reason: the entry was factually wrong (see LOW). Verified: both workflows parse; migration-smoke.sh passes bash -n; the parsed mysql option string is `--memory=2g --memory-swap=2g --cpus=2`. Co-Authored-By: Claude Opus 4.8 (1M context) --- .gitea/workflows/docker-build.yml | 4 +++- docs/ci-cd.md | 27 ++++++++++++++++-------- docs/decisions.md | 34 +++++++++++++++++++------------ scripts/migration-smoke.sh | 7 ++++++- 4 files changed, 48 insertions(+), 24 deletions(-) diff --git a/.gitea/workflows/docker-build.yml b/.gitea/workflows/docker-build.yml index f15298291..1c308ac33 100644 --- a/.gitea/workflows/docker-build.yml +++ b/.gitea/workflows/docker-build.yml @@ -515,7 +515,9 @@ jobs: trap 'docker rm -f "$NAME" >/dev/null 2>&1 || true' EXIT echo "Pulling ${IMG}" docker pull "$IMG" - docker run -d --name "$NAME" --memory 2g \ + # --memory-swap equal to --memory disables swap. Without it Docker defaults --memory-swap + # to 2x --memory, so `--memory 2g` alone silently grants 2g RAM + 2g swap (ersatztv#406). + docker run -d --name "$NAME" --memory 2g --memory-swap 2g \ -e ETV_CONFIG_FOLDER=/tmp/etv/config \ -e ETV_TRANSCODE_FOLDER=/tmp/etv/transcode \ "$IMG" diff --git a/docs/ci-cd.md b/docs/ci-cd.md index a9aaa1e99..8af1eea0b 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -139,8 +139,7 @@ So the workflow's top-level `env:` disables the servers for every runner-side do These are MSBuild properties set as environment variables so they apply to every `dotnet` invocation without touching each call site (MSBuild surfaces env vars as properties, and -`UseSharedCompilation` is only defaulted to `true` when empty, so the env var wins). Setting -`UseSharedCompilation` also disables the Razor build server, which falls back to it. +`UseSharedCompilation` is only defaulted to `true` when empty, so the env var wins). **What this does and does not shrink.** It helps the jobs that *compile* — `test`, `migrations`, `api-docs` on an API-touching PR, and the in-Docker `build`. It does **not** help `format`: `dotnet @@ -171,13 +170,23 @@ step summary. Read it off a recent run instead of re-deriving it by hand — but > `peak`.** The `anon`/`file` split is read at end-of-job, so it is indicative, not the composition > at the peak instant — sampling a true peak-anon is **ersatztv#412**. -**First measurement (PR #411, `test` job):** `peak 8305 MiB` — but with an unknown and probably -large reclaimable-cache share, so it does **not** by itself justify keeping the 10g cap. Note also -that there is **no pre-change baseline from this instrument** (the 7.8 GB `VBCSCompiler` figure was -measured host-wide across concurrent jobs, not inside one job container), so #406's premise — *"if -disabling shared compilation brings peak RSS well under 6 GiB, the whole budget loosens"* — is as -yet **neither confirmed nor killed**. #412 covers getting the delta. What *is* established: no -persistent compiler server survives a build. +**First measurement (PR #411, `test` job):** `peak 8305 MiB`, cache share **unquantified** by that +run. Do not fill the gap with a guess in either direction — an independent probe (full solution +`dotnet build`, same CI image, shared compilation off) came back `peak 9457 MiB` / **`anon 7134 +MiB`** / `file 421 MiB`, i.e. **anon dominated and cache did not**. On that evidence a 6g cap looks +*unsafe* rather than safe, and #570's "6g proved too tight" reads as the rule rather than an +outlier. + +That also means **#406's premise — *"if disabling shared compilation brings peak RSS well under 6 +GiB, the whole budget loosens"* — is looking dead**: the 7134 MiB anon was measured *with* shared +compilation already off. Disabling the compiler servers remains right (no persistent 7.8 GB server +sits on the host between builds) but **do not bank a looser cap budget on it**. + +Treat all of the above as strong-but-not-final: it is one probe, not the `test` job, and there is +**no pre-change baseline from this instrument** (the 7.8 GB `VBCSCompiler` figure was measured +host-wide across concurrent jobs, not inside one job container). #412 covers the real A/B. +What *is* established: no persistent compiler server survives a build, and `migrations` is green +with mysql capped at 2g with swap off. ### `services:` containers are capped explicitly (ersatztv#406) diff --git a/docs/decisions.md b/docs/decisions.md index 49effaa20..e39aff7cb 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -1308,21 +1308,29 @@ only up to N, so an earlier placement silently excludes the job's later workload `continue-on-error` — not `if: always()` — is what makes it advisory (`always()` controls whether a step *runs*, not whether its failure fails the job, and `defaults.run.shell: bash` means `-e` is on). -**And then the instrument taught us the lesson again, at our own expense.** The first reading came -back `peak 8305 MiB`, which reads as "the `test` job needs 8.1 GiB, so the 10g cap must stay and -#406's premise is dead." That reading is probably **wrong**, because `memory.peak` is the high-water -mark of `memory.current` — which charges **page cache**, not just anonymous memory. Proven: a -container with `anon=0` that merely reads an 800 MB file reports `memory.peak=826 MiB`, `file=800 -MiB`. Page cache is *reclaimed* under a tighter cap, not OOM-killed, so **a large peak that is -mostly `file` is not evidence that a cap must stay high** — the naive reading inverts the decision. -`anon` is what forces an OOM; size caps on it. The step now prints the split (end-of-job, so -indicative rather than peak-instant); a true peak-anon sample is **#412**. +**And then the instrument taught us the lesson twice, both times at our own expense.** The first +reading came back `peak 8305 MiB`. `memory.peak` is the high-water mark of `memory.current`, which +charges **page cache** as well as anonymous memory — proven: a container with `anon=0` that merely +reads an 800 MB file reports `memory.peak=826 MiB`, `file=800 MiB`. Page cache is *reclaimed* under +a tighter cap, not OOM-killed, so a large peak that is mostly `file` is **not** evidence a cap must +stay high. `anon` is what forces an OOM; size caps on it. The step now prints the split (end-of-job, +so indicative rather than peak-instant); a true peak-anon sample is **#412**. -**What is NOT established:** there is no pre-change baseline from this instrument — the 7.8 GB +**Then we made the same mistake in the opposite direction.** Having established that peak +*overstates*, the docs (and a report to #604) leaned to "so this number is probably mostly cache." +That was a guess about *magnitude* dressed in a verified fact about *mechanism* — and an independent +probe killed it: a full solution build in the CI image with shared compilation off measured `peak +9457 MiB` / **`anon 7134 MiB`** / `file 421 MiB`. **Anon dominated.** So a 6g cap looks *unsafe*, +#570's "6g proved too tight" is the rule rather than an outlier, and **#406's premise ("if this +brings peak RSS well under 6 GiB, the whole budget loosens") is looking dead** — the 7134 MiB was +measured *with* shared compilation already off. The switches are still right (no persistent 7.8 GB +server between builds); the looser budget they were supposed to buy is not. + +**What is NOT established:** there is still no pre-change baseline from this instrument — the 7.8 GB `VBCSCompiler` figure was measured host-wide across concurrent jobs, not inside one job container — -so #406's "if this brings peak RSS well under 6 GiB the whole budget loosens" is neither confirmed -nor killed, and #412 covers the delta. What *is* established: no persistent compiler server survives -a build, and `migrations` is green with mysql capped at 2g with swap disabled. +and the anon figure above is one probe, not the `test` job. #412 covers the real A/B. What *is* +established: no persistent compiler server survives a build, and `migrations` is green with mysql +capped at 2g with swap disabled. The lesson generalizes, and note it bit *this* change twice — once in the issue's premise and once in our own instrument: this repo's CI perf work keeps stating numbers from plausibility rather than diff --git a/scripts/migration-smoke.sh b/scripts/migration-smoke.sh index bc12a9478..388c63065 100755 --- a/scripts/migration-smoke.sh +++ b/scripts/migration-smoke.sh @@ -99,7 +99,12 @@ echo "migration-smoke: rehearsing $IMAGE ($IMG_ID)" # pending migrations on startup; it logs "Applying database migrations" then "Done applying database # migrations", and on failure the host stops (default BackgroundServiceExceptionBehavior = StopHost), # so the container exits. We gate PASS on the "Done" line, FAIL on early exit / a migration exception. -docker run -d --name "$NAME" --memory 2g \ +# --memory-swap equal to --memory disables swap for this container. Without it Docker defaults +# --memory-swap to 2x --memory, so `--memory 2g` alone silently grants 2g RAM + 2g of swap +# (ersatztv#406). That matters here specifically: this smoke runs on the PROD host in the release +# path, so a runaway migration should die loudly against its cap rather than quietly swap out the +# box that is serving media. +docker run -d --name "$NAME" --memory 2g --memory-swap 2g \ -e ETV_CONFIG_FOLDER=/config \ -e ETV_TRANSCODE_FOLDER=/tmp/etv/transcode \ -v "$WORK/config:/config" \