From e33205e8eb93f651adace961e0dc38ab5c708a45 Mon Sep 17 00:00:00 2001 From: Timothy Date: Fri, 17 Jul 2026 15:22:31 +0200 Subject: [PATCH 1/4] ci(406): disable persistent compiler servers, cap the mysql service, report peak RSS Refs #406, server-management#604. Items 1 and 2 of #406 (item 3 landed in PR #407). Root cause of the 2026-07-17 bumblebee incident (load 340, 21 GiB swapped, prod media down until reboot) is CI-induced memory pressure, and the largest single consumer was Roslyn's persistent VBCSCompiler at 7.8 GB RSS. It is a compiler *server*: it outlives the build that started it to keep its heap warm for the next one. In CI there is no next build -- each job container is torn down at the end of the run -- so it is pure cost, and it is the actual reason each job needed a 10 GiB cap. 1. Disable the compiler servers via the workflow's top-level env (UseSharedCompilation, DOTNET_CLI_USE_MSBUILD_SERVER, MSBUILDDISABLENODEREUSE). MSBuild surfaces env vars as properties and only defaults UseSharedCompilation when empty, so the env var wins and every dotnet call is covered without touching each call site. Also set as ENV in the Dockerfile's SDK stage: the workflow env reaches runner-side jobs only, and the `build` job compiles inside `docker build` where it does not propagate -- that is the job server-management#570 measured pegging 5.999/6 GiB, so it is the one that most needs this. Build stage only; the final image is FROM runtime-base, so nothing lands in the shipped image. Verified locally: a default build leaves 1 VBCSCompiler alive, the same build under these vars leaves 0, and ErsatzTV.sln still builds clean (0 errors). Trade-off: csc runs per project instead of via the server, costing some build time. 2. Cap the mysql service (--memory=2g --cpus=2). A runner's container.options applies to the job container ONLY: a live migrations job reported HostConfig.Memory=10737418240 for the job and mem=0 nanocpus=0 for its mysql service, i.e. unbounded. 2g is sized on measurement -- mysql:8.4 with this exact env peaked at 543 MiB during init, 481 MiB idle (probed on bumblebee) -- not on inheritance. Also add a `Report peak container memory` step to the test job (cgroup memory.peak, always(), never fails a build). server-management#604 sizes both runners' caps on that number and it was inherited rather than measured; now every run reports it. Docs: ci-cd.md gains the compiler-server + services-cap sections and the lane table is corrected (PR #407 reverted the #390 lane move without updating it). decisions.md records all three, incl. the standing rule that a services: container never inherits a cap. Co-Authored-By: Claude Opus 4.8 (1M context) --- .gitea/workflows/docker-build.yml | 58 ++++++++++++++++++++++++++ docker/Dockerfile | 12 ++++++ docs/ci-cd.md | 69 +++++++++++++++++++++++++++---- docs/decisions.md | 65 +++++++++++++++++++++++++++++ 4 files changed, 195 insertions(+), 9 deletions(-) diff --git a/.gitea/workflows/docker-build.yml b/.gitea/workflows/docker-build.yml index 981cdeb4c..c9b933263 100644 --- a/.gitea/workflows/docker-build.yml +++ b/.gitea/workflows/docker-build.yml @@ -60,6 +60,26 @@ env: REGISTRY: 192.168.1.95:3000 IMAGE: 192.168.1.95:3000/timothy/ersatztv + # --- CI build memory (ersatztv#406, server-management#604) --- + # Roslyn's `VBCSCompiler` is a *persistent* compiler server: it outlives the `dotnet build` that + # started it and keeps its managed heap warm for the next one. Locally that is a real speedup. + # In CI it buys nothing — each job container is torn down at the end of the run, so there is + # never a "next build" to warm — while costing a lot: 7.8 GB RSS was measured live on bumblebee, + # the single largest consumer on a 25 GiB host that also runs prod media. Several of those, one + # per concurrent job container, is what drove the host to load 340 with 21 GiB swapped. + # + # These are MSBuild properties/switches, set here as environment variables so they apply to every + # dotnet invocation in every job (restore/build/test/format/api-docs) without touching each call + # site. MSBuild surfaces environment variables as properties, and `UseSharedCompilation` is only + # defaulted to true when empty, so setting it here wins. + # + # NOTE: this reaches the *runner-side* dotnet jobs only. The `build` job compiles inside + # `docker build`, where these do not propagate — the same switches are set as ENV in the + # Dockerfile's SDK stage (docker/Dockerfile) to cover it. + UseSharedCompilation: "false" # no persistent VBCSCompiler; csc runs per-project and exits + DOTNET_CLI_USE_MSBUILD_SERVER: "0" # no persistent MSBuild server process + MSBUILDDISABLENODEREUSE: "1" # MSBuild worker nodes exit with the build instead of lingering + jobs: test: name: Build & test (.NET) @@ -131,6 +151,30 @@ jobs: dotnet test --configuration Release --no-build --blame-hang-timeout "2m" --verbosity normal --collect:"XPlat Code Coverage" --settings coverlet.runsettings --results-directory ./coverage + # Peak memory of THIS job container, reported every run (ersatztv#406, server-management#604). + # #604 sizes the runners' per-job caps on this number, and until now it was inherited rather + # than measured: the 10g cap traces back to server-management#570 observing the image build + # peg 5.999/6 GiB, which is a different job entirely. `memory.peak` is the cgroup's + # high-water mark since the container started, so reading it once at the end of the job is + # exactly the peak — no polling needed. cgroup v2 first, v1 fallback; purely informational, + # so it must never fail a build (`always()` + tolerated absence). + - name: Report peak container memory + if: always() + run: | + for f in /sys/fs/cgroup/memory.peak /sys/fs/cgroup/memory/memory.max_usage_in_bytes; do + if [ -r "$f" ]; then + bytes=$(cat "$f") + printf 'peak container memory: %s MiB (%s bytes, from %s)\n' \ + "$(( bytes / 1048576 ))" "$bytes" "$f" + if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then + printf '**Peak container memory (test job):** %s MiB\n' \ + "$(( bytes / 1048576 ))" >> "$GITHUB_STEP_SUMMARY" + fi + exit 0 + fi + done + echo "No cgroup peak-memory file readable in this container -- skipping." + # Coverage reporting (ersatztv#15 scope item 4): coverlet.collector emits a Cobertura report # per test project (via --collect above); ReportGenerator merges them into a human-readable # summary printed to the log and the job step summary. No floor is enforced yet ("decide on a @@ -180,7 +224,21 @@ jobs: # No host-port binding: the job reaches this service as mysql:3306 on the shared # runner network. Publishing 3306 made concurrent runs collide ("port is already # allocated") whenever two migrations jobs overlapped. + # + # `--memory`/`--cpus` here because the runner's `container.options` (`--memory=10g`) + # applies to the JOB container ONLY, not to `services:` — verified by inspecting a live + # migrations job: the job container reported HostConfig.Memory=10737418240, its mysql + # service reported `mem=0 nanocpus=0`, i.e. unbounded. So every migrations run was adding + # an uncapped MySQL to an already-tight host (ersatztv#406, server-management#604). + # + # 2g is sized on measurement, not inheritance: a mysql:8.4 container with this exact env + # peaked at 543 MiB during init and settled at 481 MiB idle (probed on bumblebee + # 2026-07-17). Applying the migrations is DDL-heavy rather than data-heavy and the InnoDB + # buffer pool defaults to 128M, so 2g leaves ~4x headroom. If this ever OOM-kills the + # service, raise it deliberately — do not remove the cap. options: >- + --memory=2g + --cpus=2 --health-cmd="mysqladmin ping -h 127.0.0.1 -uroot -persatztv --silent" --health-interval=5s --health-timeout=5s diff --git a/docker/Dockerfile b/docker/Dockerfile index 24e95ec85..8cf8395de 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -21,6 +21,18 @@ RUN apt-get update && \ # https://hub.docker.com/_/microsoft-dotnet FROM mcr.microsoft.com/dotnet/sdk:10.0-noble-amd64 AS build + +# Disable the persistent Roslyn/MSBuild compiler servers for the in-image build (ersatztv#406, +# server-management#604). The workflow sets these as env for the runner-side dotnet jobs, but this +# stage compiles inside `docker build`, so the workflow's env does NOT reach it — the restore and +# the two publishes below would otherwise spin up their own VBCSCompiler and hold its heap. This +# is the `build` job that server-management#570 measured pegging 5.999/6 GiB. +# Build-stage only: the final image is FROM runtime-base, so none of this lands in the shipped +# image or affects runtime. +ENV UseSharedCompilation=false \ + DOTNET_CLI_USE_MSBUILD_SERVER=0 \ + MSBUILDDISABLENODEREUSE=1 + RUN apt-get update && apt-get install -y ca-certificates gnupg default-jre-headless python3-pip WORKDIR /source diff --git a/docs/ci-cd.md b/docs/ci-cd.md index 532bb8666..c305e5f0a 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -99,20 +99,71 @@ saturated and the `small` lane idle — **queue wait exceeded every job's runtim | `test` | 354s | 1363s | ubuntu-latest | | `migrations` | 639s | 1428s | ubuntu-latest | | `functional-e2e` | 520s | 1447s | ubuntu-latest | -| `api-docs` | 5s | **1722s** | ubuntu-latest → now `small` | -| `format` | 37s | **1731s** | ubuntu-latest → now `small` | +| `api-docs` | 5s | **1722s** | ubuntu-latest → `small` → **reverted to ubuntu-latest** (#406) | +| `format` | 37s | **1731s** | ubuntu-latest → `small` → **reverted to ubuntu-latest** (#406) | | `docs-reminder` / `decisions-guard` | 10s | **5s** | small | -So `api-docs` and `format` — genuinely small jobs — moved to `small`, which also drops the -contended lane from 5 jobs to 3 and shortens the queue for the three heavy jobs. Both lanes run -the *identical* `runner-images:ubuntu-latest` base, so `small` is a label with spare capacity, not -a different capability. This move is only possible because those jobs now run in the **CI toolchain -image** (below) and no longer need the runner image to supply .NET/Node. Caveat: on an -API-touching PR `api-docs` does a full `dotnet build`, so it is not *always* small. +`api-docs` and `format` moved to `small` because the queue wait dwarfed their runtime. Both lanes +run the *identical* `runner-images:ubuntu-latest` base, so `small` was a label with spare capacity, +not a different capability — a move only possible because those jobs now run in the **CI toolchain +image** (below) and no longer need the runner image to supply .NET/Node. -Queue wait is still the dominant cost and capacity is server-management's boundary — tracked in +**Reverted 2026-07-17 (ersatztv#406 / server-management#604).** #390's own caveat — *"on an +API-touching PR `api-docs` does a full `dotnet build`, so it is not always small"* — turned out to +be the deciding factor, and "capacity 4 absorbs that" held only because **nothing enforces the sum** +of the lanes' per-job caps. Each job container is correctly capped (`--memory=10g`), but 6 slots × +10 GiB = **60 GiB on a 25 GiB host** that also runs prod media; on 2026-07-17 bumblebee hit load +340 with 21 GiB swapped. These are not small jobs — a live `docker stats` caught the `format` job +container at **3.95 GiB**, which the re-sized 2 GiB `small` lane would OOM-kill outright. #604 fixes +the queue at the source instead (`ubuntu-latest` grown to 5 slots: a 48 GiB ci-runner at capacity 4 +plus a bumblebee overflow slot), so the `small` lane can be reserved for genuinely-tiny shell jobs. + +Queue wait is still a dominant cost and capacity is server-management's boundary — tracked in **server-management#604**. The redundant triple-build behind those runtimes is **ersatztv#398**. +### CI build memory: no persistent compiler servers (ersatztv#406) + +Roslyn's `VBCSCompiler` is a **persistent** compiler server — it outlives the `dotnet build` that +started it and keeps its heap warm for the next one. Locally that is a genuine speedup; in CI it +buys nothing, because each job container is torn down at the end of the run and there is never a +"next build" to warm. It was measured at **7.8 GB RSS** on bumblebee — the single largest consumer +on the host, and the reason each job needed a 10 GiB cap in the first place. + +So the workflow's top-level `env:` disables the servers for every runner-side dotnet job: + +| Variable | Effect | +|---|---| +| `UseSharedCompilation=false` | no persistent `VBCSCompiler`; `csc` runs per project and exits | +| `DOTNET_CLI_USE_MSBUILD_SERVER=0` | no persistent MSBuild server process | +| `MSBUILDDISABLENODEREUSE=1` | MSBuild worker nodes exit with the build instead of lingering | + +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). + +**The workflow `env:` does not reach the `build` job's compilation**, which happens inside +`docker build` — the same three are set as `ENV` in the SDK stage of `docker/Dockerfile`. That is +the job server-management#570 measured pegging 5.999/6 GiB, so it is the one that most needs this. +Build-stage only; the final image is `FROM runtime-base`, so nothing lands in the shipped image. + +The `test` job ends with a **Report peak container memory** step (`always()`, never fails a build) +that prints the cgroup's `memory.peak` high-water mark to the log and the job step summary. #604 +sizes the runners' caps on that number; before this it was inherited rather than measured. Read it +off a recent run instead of re-deriving it by hand. + +### `services:` containers are capped explicitly (ersatztv#406) + +A runner's `container.options` (`--cpus=4 --memory=10g`) applies to the **job container only, not +to `services:`**. Verified on a live `migrations` job: the job container reported +`HostConfig.Memory=10737418240`; its `mysql:8.4` service reported `mem=0 nanocpus=0` — unbounded. +So `migrations` runs added an uncapped MySQL to an already-tight host. + +The mysql service now sets `--memory=2g --cpus=2`, sized on measurement rather than inheritance: a +`mysql:8.4` container with this exact env peaked at **543 MiB** during init and settled at 481 MiB +idle (probed on bumblebee, 2026-07-17). Applying the migrations is DDL-heavy rather than +data-heavy and the InnoDB buffer pool defaults to 128M, so 2g leaves ~4× headroom. **Any new +`services:` container needs its own explicit cap** — it will not inherit one. + ### `test` job `dotnet restore` → strip the Scanner project ref (`sed -i '/Scanner/d'`, matching the diff --git a/docs/decisions.md b/docs/decisions.md index 8927699ea..25f995885 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -87,6 +87,7 @@ in-file entries. - [2026-07-17 — Shuffle-source construction extracted to `ShuffleSourceBuilder`; per-family seam, not a god-factory (#380)](#2026-07-17--shuffle-source-construction-extracted-to-shufflesourcebuilder-per-family-seam-not-a-god-factory-380) - [2026-07-17 — Seasonal / date-conditional scheduling already exists (alternate schedules / playout templates); #73 closed as implemented](#2026-07-17--seasonal--date-conditional-scheduling-already-exists-alternate-schedules--playout-templates-73-closed-as-implemented) - [2026-07-17 — Auto-Tune DetailPanel member list = live search-index roll-up, not EF enumeration (#384)](#2026-07-17--auto-tune-detailpanel-member-list--live-search-index-roll-up-not-ef-enumeration-384) +- [2026-07-17 — No persistent compiler servers in CI; every `services:` container gets an explicit cap; #390's small-lane move reversed (#406)](#2026-07-17--no-persistent-compiler-servers-in-ci-every-services-container-gets-an-explicit-cap-390s-small-lane-move-reversed-406) --- @@ -1225,3 +1226,67 @@ the existing `PagedLibraryBrowseItemsResponseModel` / `LibraryBrowseItemResponse the cap — the same staleness bound the smart-collection path accepts. Read-only, catalog-read tier (no `[RequiresAuthentication]`), so a cold review sufficed. Sibling backend child #385 (write-path overrides + weights) and SPA child #386 remain open under the #383 milestone. + +--- + +## 2026-07-17 — No persistent compiler servers in CI; every `services:` container gets an explicit cap; #390's small-lane move reversed (#406) + +Three CI changes, all downstream of one incident: on 2026-07-17 bumblebee (the **prod media** Docker +host, 25 GiB / 12 cores) hit load **340** with **21 GiB swapped** and ~238 MiB free, taking prod +ersatztv/jellyfin down until reboot. Prod ersatztv itself was healthy at 168 MiB throughout — the +thrash was **CI-induced**, triggered by a burst of parallel merges to `main`. Infra sizing is +server-management#604's boundary; these three levers live in this repo and each does more than any +capacity knob. + +**1. No persistent compiler servers.** `VBCSCompiler` is a *persistent* Roslyn server: it outlives +the `dotnet build` that started it and holds its heap for the next one. Measured at **7.8 GB RSS** +live on bumblebee — the single largest consumer on the box, and the actual reason each job needed a +10 GiB cap. In CI it buys **nothing**: each job container is torn down at the end of the run, so +there is never a "next build" to warm. The workflow's top-level `env:` now sets +`UseSharedCompilation=false`, `DOTNET_CLI_USE_MSBUILD_SERVER=0`, `MSBUILDDISABLENODEREUSE=1` — +MSBuild properties set as env vars so they apply to every `dotnet` call without touching each call +site (MSBuild surfaces env vars as properties; `UseSharedCompilation` only defaults to `true` when +empty, so the env var wins). Verified locally: a default build leaves 1 `VBCSCompiler` alive, the +same build under these vars leaves **0**, and `ErsatzTV.sln` still builds clean (0 errors). + +**Why the Dockerfile also sets them** — and this is the part the issue's suggestion would have +missed: the workflow `env:` reaches the *runner-side* dotnet jobs only. The `build` job compiles +inside `docker build`, where it does not propagate, so the SDK stage of `docker/Dockerfile` sets the +same three as `ENV`. That is precisely the job server-management#570 measured pegging **5.999/6 +GiB** — the one that most needs it. Build-stage only; the final image is `FROM runtime-base`, so +nothing lands in the shipped image or affects runtime. + +**Trade-off accepted**: without the shared server each project's `csc` is a fresh process, which +costs some build time. Worth it — the memory spike is what takes prod down, and the cap sizing that +spike forces is what starves the lanes. + +**2. `services:` containers do not inherit the runner's cap.** A runner's `container.options` +(`--cpus=4 --memory=10g`) applies to the **job container only**. Verified by inspecting a live +`migrations` job: the job container reported `HostConfig.Memory=10737418240`, its `mysql:8.4` +service reported `mem=0 nanocpus=0` — **unbounded**. Every migrations run was adding an uncapped +MySQL to an already-tight host. Now `--memory=2g --cpus=2`, sized on **measurement, not +inheritance**: a `mysql:8.4` container with this exact env peaked at **543 MiB** during init and +settled at 481 MiB idle (probed on bumblebee). The migrations workload is DDL-heavy rather than +data-heavy and the InnoDB buffer pool defaults to 128M, so 2g is ~4× headroom. **Standing rule: any +new `services:` container needs its own explicit cap** — it will not inherit one. + +**3. #390's `small`-lane move for `api-docs`/`format` reversed.** #390 moved them to dodge a ~29 min +`ubuntu-latest` queue. The queue was real, but the lane was the wrong fix, and **#390's own comment +flagged why**: *"on an API-touching PR this job does a full `dotnet build`, so it is not always a +'small' job; capacity 4 absorbs that."* "Capacity 4 absorbs that" held only because **nothing +enforces the sum** of the lanes' caps — 6 slots × 10 GiB on a 25 GiB host is a 60 GiB promise. These +are not small jobs: a live `docker stats` caught the `format` job container at **3.95 GiB**, which +#604's re-sized 2 GiB `small` lane would OOM-kill outright. #604 grows `ubuntu-latest` to 5 slots +(48 GiB ci-runner at capacity 4 + a bumblebee overflow slot) and fixes the queue at the source. This +one is **order-coupled with #604**: the small lane's caps can't tighten until it lands. + +**Measurement is now continuous, not a one-off.** The `test` job ends with a **Report peak container +memory** step reading the cgroup's `memory.peak` high-water mark (`always()`, tolerates absence, +never fails a build). #604 sizes both runners' caps on that number, and until now it was *inherited* +rather than measured — the 10g cap traces back to #570 observing a different job entirely. The +lesson generalizes: this repo's CI perf issues keep stating numbers from plausibility rather than +measurement (see #390's "2–4min" apt-ffmpeg estimate; real 110s, and not load-bearing). Read the +peak off a recent run instead of re-deriving it by hand. + +**Not addressed here**: the 12–35 min queue waits (server-management#604) and the redundant +triple-build (#398). -- 2.47.3 From 04e3a5249d13548cdc6c2fd6cafe00cc48696e8e Mon Sep 17 00:00:00 2001 From: Timothy Date: Fri, 17 Jul 2026 15:36:18 +0200 Subject: [PATCH 2/4] ci(406): disable swap on the capped mysql; fix peak-step placement and failure mode [decisions-edit] Review fixes from the cold adversarial pass on PR #411. Verdict was MERGEABLE, but one HIGH is a genuine defect worth the round-trip. [decisions-edit] rationale: the 12 "modified" lines in docs/decisions.md are lines THIS PR added in its own previous commit (e33205e8), not a settled entry on main. The hook diffs against HEAD; net against origin/main this file is still a pure insertion (65 added, 0 deleted -- verified, and CI's decisions-guard diffs against the PR base). It is also the sanctioned reason regardless: the entry as first written contained a factual error (see HIGH below). HIGH -- `--memory=2g` alone silently grants 2g of SWAP as well. Docker defaults an unset --memory-swap to 2x --memory. Verified live on bumblebee: `--memory=2g` alone gives memory.max=2147483648 AND memory.swap.max=2147483648; adding `--memory-swap=2g` gives memory.swap.max=0. Capping RAM while permitting swap is close to the worst outcome on the one host whose swap thrash is the entire reason for the cap -- and a swapping mysqld mid-DDL is exactly the pathology behind the known `Command Timeout expired` migrations flake, so the naive cap could have made that flake worse rather than better. The comment promising "if this ever OOM-kills the service" described a failure mode that would not have happened first: it would have swapped, silently. Now --memory-swap=2g (swap off); prefer a loud OOM over silent degradation. The same 2x applies to the runners' 10g job slots (10 GiB RAM + 10 GiB swap each), which is a plausible direct mechanism for the incident's 21 GiB swapped. That is #604's boundary -- reporting it there. MEDIUM -- `if: always()` does not make a step advisory. It controls whether the step RUNS, not whether its failure fails the job, and defaults.run.shell: bash means -e is on, so a failed cat/redirect in an informational step could redden a green test job. Use continue-on-error: true, matching the neighbouring Coverage summary step. MEDIUM -- the peak step must run LAST. memory.peak read at step N reports the peak only up to N, so sitting before Coverage summary excluded reportgenerator's memory -- under- reporting the very number #604 sizes caps on. Moved after it. LOW -- dependency-scan.yml gets the env too; workflow env does not cross workflow files, and that one runs unattended on a cron against the prod media host. NIT -- Dockerfile ENV moved below the openapi-generator wget: an ENV invalidates every layer under it, and that ~30MB download is deliberately ordered early to stay cached (#190). Nothing between it and restore compiles, so this placement is free. NIT -- docs no longer imply this shrinks `format`. dotnet format loads Roslyn in-process via MSBuildWorkspace and never spawns csc, so its 3.95 GiB is untouched; sizing the small lane on the assumption it shrank would be a trap. Also record honestly that 543 MiB is init+idle rather than the 787-migration replay (so 2g is a measured floor + headroom, not a measured ceiling -- the migrations job going green is what validates it) and that --cpus=2 is judgement with no measurement behind it. The entry criticises dressing estimates as measurements; it should not do that itself. Co-Authored-By: Claude Opus 4.8 (1M context) --- .gitea/workflows/dependency-scan.yml | 11 ++++ .gitea/workflows/docker-build.yml | 81 ++++++++++++++++++---------- docker/Dockerfile | 31 ++++++----- docs/ci-cd.md | 47 +++++++++++++--- docs/decisions.md | 51 +++++++++++++----- 5 files changed, 162 insertions(+), 59 deletions(-) diff --git a/.gitea/workflows/dependency-scan.yml b/.gitea/workflows/dependency-scan.yml index c2c68ab29..a971293e5 100644 --- a/.gitea/workflows/dependency-scan.yml +++ b/.gitea/workflows/dependency-scan.yml @@ -22,6 +22,17 @@ concurrency: group: ersatztv-depscan cancel-in-progress: true +# No persistent MSBuild/Roslyn servers (ersatztv#406). Workflow `env:` does not cross workflow +# files, so docker-build.yml's copy of these does not apply here and this has to be repeated. +# Smaller stakes than the build pipeline — `dotnet restore` + `dotnet list` are MSBuild-driven and +# never invoke csc, so this is lingering worker nodes (hundreds of MiB), not a 7.8 GB VBCSCompiler. +# Worth setting anyway: this runs unattended on a Monday 06:00 cron against the same host that runs +# prod media, and node reuse keeps workers alive ~15 min after the job. +env: + UseSharedCompilation: "false" + DOTNET_CLI_USE_MSBUILD_SERVER: "0" + MSBUILDDISABLENODEREUSE: "1" + jobs: scan: name: NuGet vulnerable packages diff --git a/.gitea/workflows/docker-build.yml b/.gitea/workflows/docker-build.yml index c9b933263..8b17aea0d 100644 --- a/.gitea/workflows/docker-build.yml +++ b/.gitea/workflows/docker-build.yml @@ -151,30 +151,6 @@ jobs: dotnet test --configuration Release --no-build --blame-hang-timeout "2m" --verbosity normal --collect:"XPlat Code Coverage" --settings coverlet.runsettings --results-directory ./coverage - # Peak memory of THIS job container, reported every run (ersatztv#406, server-management#604). - # #604 sizes the runners' per-job caps on this number, and until now it was inherited rather - # than measured: the 10g cap traces back to server-management#570 observing the image build - # peg 5.999/6 GiB, which is a different job entirely. `memory.peak` is the cgroup's - # high-water mark since the container started, so reading it once at the end of the job is - # exactly the peak — no polling needed. cgroup v2 first, v1 fallback; purely informational, - # so it must never fail a build (`always()` + tolerated absence). - - name: Report peak container memory - if: always() - run: | - for f in /sys/fs/cgroup/memory.peak /sys/fs/cgroup/memory/memory.max_usage_in_bytes; do - if [ -r "$f" ]; then - bytes=$(cat "$f") - printf 'peak container memory: %s MiB (%s bytes, from %s)\n' \ - "$(( bytes / 1048576 ))" "$bytes" "$f" - if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then - printf '**Peak container memory (test job):** %s MiB\n' \ - "$(( bytes / 1048576 ))" >> "$GITHUB_STEP_SUMMARY" - fi - exit 0 - fi - done - echo "No cgroup peak-memory file readable in this container -- skipping." - # Coverage reporting (ersatztv#15 scope item 4): coverlet.collector emits a Cobertura report # per test project (via --collect above); ReportGenerator merges them into a human-readable # summary printed to the log and the job step summary. No floor is enforced yet ("decide on a @@ -204,6 +180,38 @@ jobs: cat coverage/report/SummaryGithub.md >> "$GITHUB_STEP_SUMMARY" fi + # Peak memory of THIS job container, reported every run (ersatztv#406, server-management#604). + # #604 sizes the runners' per-job caps on this number, and until now it was inherited rather + # than measured: the 10g cap traces back to server-management#570 observing the image build + # peg 5.999/6 GiB, which is a different job entirely. `memory.peak` is the cgroup's + # high-water mark since the container started, so reading it once is exactly the peak — no + # polling needed. It must therefore run LAST: `memory.peak` read at step N only reports the + # peak up to N, and this sits after Coverage summary so reportgenerator's own memory (the + # job's last real workload) is counted. #604 sizes caps on this number; under-reporting it + # would size them too low. cgroup v2 first, v1 fallback; purely informational, + # so it must never fail a build (`always()` + tolerated absence). + - name: Report peak container memory + # `always()` controls whether this step RUNS, not whether its failure fails the job — and + # `defaults.run.shell: bash` means `-e -o pipefail` is on, so a failed `cat`/redirect here + # would redden a green test job. `continue-on-error` is what actually makes it advisory, + # the same guarantee the Coverage summary step above uses. + if: always() + continue-on-error: true + run: | + for f in /sys/fs/cgroup/memory.peak /sys/fs/cgroup/memory/memory.max_usage_in_bytes; do + if [ -r "$f" ]; then + bytes=$(cat "$f") + printf 'peak container memory: %s MiB (%s bytes, from %s)\n' \ + "$(( bytes / 1048576 ))" "$bytes" "$f" + if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then + printf '**Peak container memory (test job):** %s MiB\n' \ + "$(( bytes / 1048576 ))" >> "$GITHUB_STEP_SUMMARY" + fi + exit 0 + fi + done + echo "No cgroup peak-memory file readable in this container -- skipping." + migrations: name: EF migration integrity (SQLite + MySql) runs-on: ubuntu-latest @@ -231,13 +239,28 @@ jobs: # service reported `mem=0 nanocpus=0`, i.e. unbounded. So every migrations run was adding # an uncapped MySQL to an already-tight host (ersatztv#406, server-management#604). # - # 2g is sized on measurement, not inheritance: a mysql:8.4 container with this exact env - # peaked at 543 MiB during init and settled at 481 MiB idle (probed on bumblebee - # 2026-07-17). Applying the migrations is DDL-heavy rather than data-heavy and the InnoDB - # buffer pool defaults to 128M, so 2g leaves ~4x headroom. If this ever OOM-kills the - # service, raise it deliberately — do not remove the cap. + # `--memory-swap=2g` is NOT redundant with `--memory=2g` — it is the point. Docker defaults + # an unset `--memory-swap` to *twice* `--memory`, so `--memory=2g` alone would grant 2g RAM + # **plus 2g of swap** (verified on bumblebee: `--memory=2g` alone → memory.max=2147483648 + # AND memory.swap.max=2147483648; with `--memory-swap=2g` → memory.swap.max=0). Setting it + # equal to --memory disables swap for this container. That matters more here than anywhere: + # swap thrash on this host is the whole reason this cap exists, and a swapping mysqld mid-DDL + # is precisely the pathology behind the known `Command Timeout expired` migrations flake. We + # want a loud OOM over silent swapping — an OOM is a clear signal to raise the cap. + # + # 2g is sized on measurement rather than inheritance, but honestly: a mysql:8.4 container + # with this exact env peaked at 543 MiB during init and settled at 481 MiB idle (probed on + # bumblebee 2026-07-17). That is init+idle, NOT the 787-migration replay, which grows caches + # idle never touches — so treat 2g as a measured floor with headroom, not a measured + # ceiling. The migrations job going green is what validates it. If this OOM-kills the + # service, raise it deliberately — do not remove the cap, and do not re-enable swap. + # + # `--cpus=2` is a ceiling, not a reservation, and is the one number here with no measurement + # behind it: 787 sequential DDL statements on one connection are ~1-core-bound, so 2 is + # judgement. Revisit if the apply step's tail latency grows. options: >- --memory=2g + --memory-swap=2g --cpus=2 --health-cmd="mysqladmin ping -h 127.0.0.1 -uroot -persatztv --silent" --health-interval=5s diff --git a/docker/Dockerfile b/docker/Dockerfile index 8cf8395de..1cefcf67f 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -21,18 +21,6 @@ RUN apt-get update && \ # https://hub.docker.com/_/microsoft-dotnet FROM mcr.microsoft.com/dotnet/sdk:10.0-noble-amd64 AS build - -# Disable the persistent Roslyn/MSBuild compiler servers for the in-image build (ersatztv#406, -# server-management#604). The workflow sets these as env for the runner-side dotnet jobs, but this -# stage compiles inside `docker build`, so the workflow's env does NOT reach it — the restore and -# the two publishes below would otherwise spin up their own VBCSCompiler and hold its heap. This -# is the `build` job that server-management#570 measured pegging 5.999/6 GiB. -# Build-stage only: the final image is FROM runtime-base, so none of this lands in the shipped -# image or affects runtime. -ENV UseSharedCompilation=false \ - DOTNET_CLI_USE_MSBUILD_SERVER=0 \ - MSBUILDDISABLENODEREUSE=1 - RUN apt-get update && apt-get install -y ca-certificates gnupg default-jre-headless python3-pip WORKDIR /source @@ -68,6 +56,25 @@ COPY ErsatzTV.Infrastructure/*.csproj ./ErsatzTV.Infrastructure/ COPY ErsatzTV.Infrastructure.Sqlite/*.csproj ./ErsatzTV.Infrastructure.Sqlite/ COPY ErsatzTV.Infrastructure.MySql/*.csproj ./ErsatzTV.Infrastructure.MySql/ COPY ErsatzTV.Scanner/*.csproj ./ErsatzTV.Scanner/ + +# Disable the persistent Roslyn/MSBuild compiler servers for the in-image build (ersatztv#406, +# server-management#604). The workflow sets these as env for the runner-side dotnet jobs, but this +# stage compiles inside `docker build`, so the workflow's env does NOT reach it — the restore and +# the two publishes below would otherwise spin up their own VBCSCompiler and hold its heap. This is +# the `build` job that server-management#570 measured pegging 5.999/6 GiB, so it is the one that +# most needs this. +# +# Placed here rather than at the top of the stage on purpose: an ENV invalidates every layer below +# it, and the wget of the ~30MB openapi-generator jar above is deliberately ordered early to stay +# cached (ersatztv#190). Nothing between that wget and this line compiles, so this is the earliest +# point where the ENV is free. +# +# Build-stage only: the final image is FROM runtime-base and only COPY --from=build /app (files, +# not ENV), so none of this lands in the shipped image or affects runtime. +ENV UseSharedCompilation=false \ + DOTNET_CLI_USE_MSBUILD_SERVER=0 \ + MSBUILDDISABLENODEREUSE=1 + RUN dotnet restore -r linux-x64 ErsatzTV/ErsatzTV.csproj # copy everything else and build app diff --git a/docs/ci-cd.md b/docs/ci-cd.md index c305e5f0a..f35ccf3d5 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -139,7 +139,17 @@ 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). +`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. + +**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 +format` loads Roslyn in-process via MSBuildWorkspace and never spawns `csc`, so its measured 3.95 +GiB is unaffected. Don't size the `small` lane expecting `format` to have shrunk — it hasn't. + +The same three are repeated in `dependency-scan.yml`; workflow `env:` does not cross workflow files. +That one is lower-stakes (restore/list are MSBuild-driven, so it's lingering worker nodes rather +than a 7.8 GB VBCSCompiler) but it runs unattended on a cron against the prod media host. **The workflow `env:` does not reach the `build` job's compilation**, which happens inside `docker build` — the same three are set as `ENV` in the SDK stage of `docker/Dockerfile`. That is @@ -158,11 +168,36 @@ to `services:`**. Verified on a live `migrations` job: the job container reporte `HostConfig.Memory=10737418240`; its `mysql:8.4` service reported `mem=0 nanocpus=0` — unbounded. So `migrations` runs added an uncapped MySQL to an already-tight host. -The mysql service now sets `--memory=2g --cpus=2`, sized on measurement rather than inheritance: a -`mysql:8.4` container with this exact env peaked at **543 MiB** during init and settled at 481 MiB -idle (probed on bumblebee, 2026-07-17). Applying the migrations is DDL-heavy rather than -data-heavy and the InnoDB buffer pool defaults to 128M, so 2g leaves ~4× headroom. **Any new -`services:` container needs its own explicit cap** — it will not inherit one. +The mysql service now sets `--memory=2g --memory-swap=2g --cpus=2`. + +**`--memory-swap` is the part that matters, and it is easy to get wrong.** Docker defaults an unset +`--memory-swap` to **twice** `--memory`, so `--memory=2g` *alone* grants 2g RAM **plus 2g of swap**. +Verified on bumblebee: + +| Options | `memory.max` | `memory.swap.max` | +|---|---|---| +| `--memory=2g` | 2147483648 | **2147483648** ← 2 GiB of swap | +| `--memory=2g --memory-swap=2g` | 2147483648 | **0** ← swap disabled | + +Setting `--memory-swap` equal to `--memory` disables swap for the container. On this host that is +the whole point: swap thrash is what took prod down, and a swapping mysqld mid-DDL is precisely the +pathology behind the known `Command Timeout expired` migrations flake. **Prefer a loud OOM over +silent swapping** — an OOM is a clear signal to raise the cap; swapping just degrades everything. + +⚠️ **The same 2× applies to the runners' `container.options: --memory=10g`** — each job slot is +really 10 GiB RAM *plus* 10 GiB swap. The "6 slots × 10 GiB = 60 GiB on a 25 GiB host" framing +understates the promise by 2×, and it is a plausible direct mechanism for the incident's 21 GiB of +swap. Fixing that is server-management#604's call (reported there). + +**On the 2g figure, honestly:** a `mysql:8.4` container with this exact env peaked at **543 MiB** +during init and settled at 481 MiB idle (probed on bumblebee, 2026-07-17) — but that is init+idle, +**not** the 787-migration replay, which grows table/definition caches idle never touches. So 2g is a +measured *floor* plus headroom, not a measured ceiling; the `migrations` job going green is what +validates it. `--cpus=2` has no measurement behind it at all — 787 sequential DDL statements on one +connection are ~1-core-bound, so it is judgement; revisit if the apply step's tail latency grows. + +**Any new `services:` container needs its own explicit cap** — it will not inherit one, and it needs +`--memory-swap` set alongside `--memory` or it silently gets 2× in swap. ### `test` job diff --git a/docs/decisions.md b/docs/decisions.md index 25f995885..6f72e7d11 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -1264,11 +1264,30 @@ spike forces is what starves the lanes. (`--cpus=4 --memory=10g`) applies to the **job container only**. Verified by inspecting a live `migrations` job: the job container reported `HostConfig.Memory=10737418240`, its `mysql:8.4` service reported `mem=0 nanocpus=0` — **unbounded**. Every migrations run was adding an uncapped -MySQL to an already-tight host. Now `--memory=2g --cpus=2`, sized on **measurement, not -inheritance**: a `mysql:8.4` container with this exact env peaked at **543 MiB** during init and -settled at 481 MiB idle (probed on bumblebee). The migrations workload is DDL-heavy rather than -data-heavy and the InnoDB buffer pool defaults to 128M, so 2g is ~4× headroom. **Standing rule: any -new `services:` container needs its own explicit cap** — it will not inherit one. +MySQL to an already-tight host. Now `--memory=2g --memory-swap=2g --cpus=2`. + +**Why `--memory-swap` is not redundant** (cold-review catch, and the sharpest thing in this change): +Docker defaults an unset `--memory-swap` to **twice** `--memory`, so `--memory=2g` alone grants 2g +RAM **plus 2g of swap** — verified live: `--memory=2g` → `memory.max=2147483648` **and** +`memory.swap.max=2147483648`; `--memory=2g --memory-swap=2g` → `memory.swap.max=0`. Capping RAM +while silently permitting swap is close to the worst outcome **on the host whose swap thrash is the +entire reason for the cap**, and a swapping mysqld mid-DDL is exactly the pathology behind the known +`Command Timeout expired` migrations flake — i.e. the naive cap could have made that flake worse. +**Standing rule: prefer a loud OOM over silent swapping.** An OOM is an unambiguous "raise the cap" +signal; swapping just degrades everything and blames something else. + +**The same 2× applies to the runners' 10g job slots** — each is really 10 GiB RAM *plus* 10 GiB +swap, so the "60 GiB promise on a 25 GiB host" understates by 2× and is a plausible direct mechanism +for the incident's 21 GiB swapped. That is server-management#604's boundary; reported there. + +**On 2g, honestly**: 543 MiB is init+idle, **not** the 787-migration replay (which grows caches idle +never touches), so 2g is a measured *floor* plus headroom, not a measured ceiling — the `migrations` +job going green is what validates it. `--cpus=2` has no measurement behind it at all; 787 sequential +DDL statements on one connection are ~1-core-bound, so it is judgement. Recording that rather than +dressing a guess as measurement — the same failure this entry criticises below. + +**Standing rule: any new `services:` container needs its own explicit cap** — it will not inherit +one, and `--memory` without `--memory-swap` silently grants 2× in swap. **3. #390's `small`-lane move for `api-docs`/`format` reversed.** #390 moved them to dodge a ~29 min `ubuntu-latest` queue. The queue was real, but the lane was the wrong fix, and **#390's own comment @@ -1280,13 +1299,21 @@ are not small jobs: a live `docker stats` caught the `format` job container at * (48 GiB ci-runner at capacity 4 + a bumblebee overflow slot) and fixes the queue at the source. This one is **order-coupled with #604**: the small lane's caps can't tighten until it lands. -**Measurement is now continuous, not a one-off.** The `test` job ends with a **Report peak container -memory** step reading the cgroup's `memory.peak` high-water mark (`always()`, tolerates absence, -never fails a build). #604 sizes both runners' caps on that number, and until now it was *inherited* -rather than measured — the 10g cap traces back to #570 observing a different job entirely. The -lesson generalizes: this repo's CI perf issues keep stating numbers from plausibility rather than -measurement (see #390's "2–4min" apt-ffmpeg estimate; real 110s, and not load-bearing). Read the -peak off a recent run instead of re-deriving it by hand. +**Measurement is now continuous, not a one-off.** The `test` job's **last** step reports the +cgroup's `memory.peak` high-water mark (`continue-on-error`, tolerates absence). #604 sizes both +runners' caps on that number, and until now it was *inherited* rather than measured — the 10g cap +traces back to #570 observing a different job entirely. Two things that look like details but are +the whole point: it must run **last** (`memory.peak` read at step N reports the peak only up to N, +so an earlier placement silently excludes the job's later workload and would size the cap too low), +and `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). The lesson generalizes: this repo's CI perf issues keep stating numbers from +plausibility rather than measurement (see #390's "2–4min" apt-ffmpeg estimate; real 110s, and not +load-bearing). Read the peak off a recent run instead of re-deriving it by hand. + +**What this does NOT shrink**: `format`. `dotnet format` loads Roslyn in-process via +MSBuildWorkspace and never spawns `csc`, so its measured 3.95 GiB is untouched by any of this. Do +not size the `small` lane expecting otherwise. **Not addressed here**: the 12–35 min queue waits (server-management#604) and the redundant triple-build (#398). -- 2.47.3 From 7859510c7867c4c5d8f59d7c8b19b589efaed53a Mon Sep 17 00:00:00 2001 From: Timothy Date: Fri, 17 Jul 2026 15:53:24 +0200 Subject: [PATCH 3/4] =?UTF-8?q?ci(406):=20report=20anon/file=20split=20?= =?UTF-8?q?=E2=80=94=20memory.peak=20counts=20page=20cache,=20not=20just?= =?UTF-8?q?=20RSS=20[decisions-edit]?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The peak-memory step from the previous commit worked on its first real CI run and immediately produced a number that would have led #604 to the WRONG decision: peak container memory: 8305 MiB (8708448256 bytes, from /sys/fs/cgroup/memory.peak) Read naively that says "the test job needs 8.1 GiB, so the 10g cap must stay and #406's 'the budget loosens' premise is dead". That reading is probably wrong, because `memory.peak` is the high-water mark of `memory.current`, which charges PAGE CACHE to the cgroup as well as anonymous memory. It is not peak RSS, and a build job does heavy NuGet/npm/obj/bin/coverage I/O. Proven on bumblebee -- a container with ZERO anonymous memory that merely reads an 800MB file: memory.peak : 826 MiB anon : 0 MiB file : 800 MiB This inverts the decision: page cache is RECLAIMED under a tighter cap, not OOM-killed, so a large peak that is mostly `file` is NOT evidence that the cap must stay high. `anon` is what forces an OOM. So the step now prints the anon/file split and says, in the log and the docs, to size caps on anon rather than peak. Honest about what this still isn't: the split is read at end-of-job, so it is the composition then, not at the peak instant -- a job peaking mid-`dotnet test` and freeing after would under-report anon. Sampling a true peak-anon is ersatztv#412, filed with the options. Also recorded there and in ci-cd.md: there is NO pre-change baseline from this instrument (7.8 GB VBCSCompiler was measured host-wide across concurrent jobs, not inside one job container), so #406's premise is neither confirmed nor killed yet. What IS established: no persistent compiler server survives a build, and migrations is green with mysql capped at 2g with swap disabled. The irony is the point, and it is now written into decisions.md: this repo's CI perf work keeps stating numbers from plausibility rather than measurement (#390's "2-4min" apt-ffmpeg estimate; real 110s). Measuring the wrong quantity precisely is the same failure wearing a lab coat -- and it bit this very change, whose whole purpose was to replace an inherited number with a measured one. Verified: shellcheck clean; the real step body run inside a --memory=2g container under `bash -e -o pipefail` reads memory.peak, prints the split, writes the step summary, exits 0; and it degrades to a skip + exit 0 where no cgroup file exists. [decisions-edit]: the modified docs/decisions.md lines are ones THIS PR added in its own earlier commits, not a settled entry on main -- net vs origin/main is still a pure insertion (0 deletions, verified). It is also the sanctioned reason: the entry as written was factually wrong about what memory.peak measures. Co-Authored-By: Claude Opus 4.8 (1M context) --- .gitea/workflows/docker-build.yml | 74 ++++++++++++++++++++++--------- docs/ci-cd.md | 26 +++++++++-- docs/decisions.md | 38 +++++++++++----- 3 files changed, 103 insertions(+), 35 deletions(-) diff --git a/.gitea/workflows/docker-build.yml b/.gitea/workflows/docker-build.yml index 8b17aea0d..f15298291 100644 --- a/.gitea/workflows/docker-build.yml +++ b/.gitea/workflows/docker-build.yml @@ -180,16 +180,28 @@ jobs: cat coverage/report/SummaryGithub.md >> "$GITHUB_STEP_SUMMARY" fi - # Peak memory of THIS job container, reported every run (ersatztv#406, server-management#604). - # #604 sizes the runners' per-job caps on this number, and until now it was inherited rather - # than measured: the 10g cap traces back to server-management#570 observing the image build - # peg 5.999/6 GiB, which is a different job entirely. `memory.peak` is the cgroup's - # high-water mark since the container started, so reading it once is exactly the peak — no - # polling needed. It must therefore run LAST: `memory.peak` read at step N only reports the - # peak up to N, and this sits after Coverage summary so reportgenerator's own memory (the - # job's last real workload) is counted. #604 sizes caps on this number; under-reporting it - # would size them too low. cgroup v2 first, v1 fallback; purely informational, - # so it must never fail a build (`always()` + tolerated absence). + # Memory of THIS job container, reported every run (ersatztv#406, server-management#604). + # #604 sizes the runners' per-job caps on these numbers, and until now they were inherited + # rather than measured: the 10g cap traces back to server-management#570 observing the image + # build peg 5.999/6 GiB, which is a different job entirely. + # + # ⚠️ READ THE BREAKDOWN, NOT JUST THE PEAK. `memory.peak` is the high-water mark of + # `memory.current`, which charges **page cache** to the cgroup as well as anonymous memory — + # it is NOT "peak RSS", and for a build job (NuGet/npm/obj/bin/coverage I/O) the cache + # dominates. Demonstrated on bumblebee: a container with anon=0 that merely reads an 800 MB + # file reports memory.peak=826 MiB, of which file=800 MiB. This matters because the naive + # reading inverts the decision: page cache is **reclaimed** under a tighter cap, not + # OOM-killed, so a large peak that is mostly `file` is NOT evidence that the cap must stay + # high. `anon` (+ a little kernel/sock) is the part that actually forces an OOM. + # + # The split below is read at end-of-job, so it is the *current* composition rather than the + # composition at the peak instant — indicative, not exact. Sizing a cap off one run is still + # wrong; take a few runs, and treat anon as the floor and peak as the (cache-inflated) + # ceiling. Refining this into a true peak-anon sample is ersatztv#412. + # + # Runs LAST on purpose: memory.peak read at step N reports the peak only up to N, so this + # sits after Coverage summary to include reportgenerator, the job's last real workload. + # cgroup v2 first, v1 fallback. - name: Report peak container memory # `always()` controls whether this step RUNS, not whether its failure fails the job — and # `defaults.run.shell: bash` means `-e -o pipefail` is on, so a failed `cat`/redirect here @@ -198,19 +210,39 @@ jobs: if: always() continue-on-error: true run: | + mib() { echo "$(( ${1:-0} / 1048576 ))"; } + peak=""; src="" for f in /sys/fs/cgroup/memory.peak /sys/fs/cgroup/memory/memory.max_usage_in_bytes; do - if [ -r "$f" ]; then - bytes=$(cat "$f") - printf 'peak container memory: %s MiB (%s bytes, from %s)\n' \ - "$(( bytes / 1048576 ))" "$bytes" "$f" - if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then - printf '**Peak container memory (test job):** %s MiB\n' \ - "$(( bytes / 1048576 ))" >> "$GITHUB_STEP_SUMMARY" - fi - exit 0 - fi + if [ -r "$f" ]; then peak=$(cat "$f" 2>/dev/null || echo ""); src="$f"; break; fi done - echo "No cgroup peak-memory file readable in this container -- skipping." + if [ -z "$peak" ]; then + echo "No cgroup peak-memory file readable in this container -- skipping." + exit 0 + fi + anon=""; file="" + if [ -r /sys/fs/cgroup/memory.stat ]; then + anon=$(awk '/^anon /{print $2}' /sys/fs/cgroup/memory.stat 2>/dev/null || echo "") + file=$(awk '/^file /{print $2}' /sys/fs/cgroup/memory.stat 2>/dev/null || echo "") + fi + echo "::group::Container memory (ersatztv#406 / server-management#604)" + printf 'peak (incl. page cache): %s MiB [%s bytes, %s]\n' "$(mib "$peak")" "$peak" "$src" + if [ -n "$anon" ]; then + printf 'end-of-job anon (the part that OOMs): %s MiB\n' "$(mib "$anon")" + printf 'end-of-job file (page cache, reclaimable): %s MiB\n' "$(mib "${file:-0}")" + echo 'NOTE: peak counts reclaimable page cache. Size caps on anon, not on peak.' + else + echo 'NOTE: no memory.stat breakdown available; peak includes reclaimable page cache.' + fi + echo "::endgroup::" + if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then + { + printf '**Container memory (test job):** peak %s MiB *(incl. reclaimable page cache)*' \ + "$(mib "$peak")" + [ -n "$anon" ] && printf ' · end-of-job anon %s MiB · file %s MiB' \ + "$(mib "$anon")" "$(mib "${file:-0}")" + printf '\n' + } >> "$GITHUB_STEP_SUMMARY" || true + fi migrations: name: EF migration integrity (SQLite + MySql) diff --git a/docs/ci-cd.md b/docs/ci-cd.md index f35ccf3d5..a9aaa1e99 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -156,10 +156,28 @@ than a 7.8 GB VBCSCompiler) but it runs unattended on a cron against the prod me the job server-management#570 measured pegging 5.999/6 GiB, so it is the one that most needs this. Build-stage only; the final image is `FROM runtime-base`, so nothing lands in the shipped image. -The `test` job ends with a **Report peak container memory** step (`always()`, never fails a build) -that prints the cgroup's `memory.peak` high-water mark to the log and the job step summary. #604 -sizes the runners' caps on that number; before this it was inherited rather than measured. Read it -off a recent run instead of re-deriving it by hand. +The `test` job's **last** step, **Report peak container memory** (`continue-on-error`, never fails a +build), prints the cgroup's `memory.peak` plus an `anon`/`file` breakdown to the log and the job +step summary. Read it off a recent run instead of re-deriving it by hand — but read it correctly: + +> ⚠️ **`memory.peak` is not "peak RSS".** It is the high-water mark of `memory.current`, which +> charges **page cache** to the cgroup as well as anonymous memory. Proven on bumblebee: a +> container with `anon=0` that merely reads an 800 MB file reports `memory.peak=826 MiB`, of which +> `file=800 MiB`. +> +> This matters because the naive reading **inverts the decision**: page cache is *reclaimed* under a +> tighter cap, not OOM-killed, so a big peak that is mostly `file` is **not** evidence that the cap +> must stay high. `anon` is the part that actually forces an OOM. **Size caps on `anon`, not on +> `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. ### `services:` containers are capped explicitly (ersatztv#406) diff --git a/docs/decisions.md b/docs/decisions.md index 6f72e7d11..49effaa20 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -1300,16 +1300,34 @@ are not small jobs: a live `docker stats` caught the `format` job container at * one is **order-coupled with #604**: the small lane's caps can't tighten until it lands. **Measurement is now continuous, not a one-off.** The `test` job's **last** step reports the -cgroup's `memory.peak` high-water mark (`continue-on-error`, tolerates absence). #604 sizes both -runners' caps on that number, and until now it was *inherited* rather than measured — the 10g cap -traces back to #570 observing a different job entirely. Two things that look like details but are -the whole point: it must run **last** (`memory.peak` read at step N reports the peak only up to N, -so an earlier placement silently excludes the job's later workload and would size the cap too low), -and `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). The lesson generalizes: this repo's CI perf issues keep stating numbers from -plausibility rather than measurement (see #390's "2–4min" apt-ffmpeg estimate; real 110s, and not -load-bearing). Read the peak off a recent run instead of re-deriving it by hand. +cgroup's `memory.peak` plus an `anon`/`file` breakdown (`continue-on-error`, tolerates absence). +#604 sizes both runners' caps on that number, and until now it was *inherited* rather than measured +— the 10g cap traces back to #570 observing a different job entirely. Two things that look like +details but are the whole point: it must run **last** (`memory.peak` read at step N reports the peak +only up to N, so an earlier placement silently excludes the job's later workload), and +`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**. + +**What is NOT established:** 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 "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. + +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 +measurement (see #390's "2–4min" apt-ffmpeg estimate; real 110s, and not load-bearing). Measuring +the wrong quantity precisely is the same failure wearing a lab coat. **What this does NOT shrink**: `format`. `dotnet format` loads Roslyn in-process via MSBuildWorkspace and never spawns `csc`, so its measured 3.95 GiB is untouched by any of this. Do -- 2.47.3 From 469d72555929d648ec39ddb7c896dffa810b4a6d Mon Sep 17 00:00:00 2001 From: Timothy Date: Fri, 17 Jul 2026 16:13:39 +0200 Subject: [PATCH 4/4] 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" \ -- 2.47.3