ci(406): disable persistent compiler servers, cap the services: mysql, report peak RSS #411

Merged

timothy merged 4 commits from ci/406-roslyn-mysql-caps into main 2026-07-17 17:17:55 +02:00

Conversation 2 Commits 4 Files Changed 6

+392 -11

timothy commented 2026-07-17 15:24:53 +02:00

Owner

Copy Link

Copy Source

Items 1 and 2 of #406. Item 3 landed in #407, so this PR says Refs #406 — see "What's left" for why it doesn't close the issue yet.

Refs #406, server-management#604.

Why

The 2026-07-17 bumblebee incident (load 340, 21 GiB swapped, prod media down until reboot) was CI-induced — prod ersatztv itself was healthy at 168 MiB throughout. Infra sizing is #604's boundary; these two levers live here and each does more than any capacity knob.

The root cause of the memory spike is that VBCSCompiler is a compiler server: it deliberately outlives the dotnet build that started it, keeping its heap warm for the next one. Locally that's a real speedup. In CI there is no next build — each job container is torn down at the end of the run — so the 7.8 GB RSS measured on bumblebee is pure cost. It is also the actual reason each job needs a 10 GiB cap in the first place.

1. No persistent compiler servers

Top-level workflow env:, so every dotnet call in every job is covered without touching each call site:

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

These are MSBuild properties set as env vars — MSBuild surfaces env vars as properties, and UseSharedCompilation is only defaulted to true when empty, so the env var wins.

The part the issue's suggestion would have missed

Workflow env: reaches the runner-side dotnet jobs only. The build job compiles inside docker build, where it does not propagate — so the same three are set as ENV in the SDK stage of docker/Dockerfile.

That matters more than the rest: build is the job server-management#570 measured pegging 5.999/6 GiB, i.e. the single job the entire 10 GiB cap is derived from. Fixing only the workflow would have left it untouched. Build-stage only — the final image is FROM runtime-base, so nothing lands in the shipped image or affects runtime.

Verified, not assumed

The whole premise is that UseSharedCompilation=false as an env var actually suppresses the server. Proven on a throwaway project:

• default build → 1 VBCSCompiler alive afterward
• same build with these env vars → 0
• ErsatzTV.sln full build under them → Build succeeded. 0 Error(s)

Trade-off accepted: each project's csc becomes a fresh process, costing some build time. Worth it — the memory spike is what takes prod down, and the cap that spike forces is what starves the lanes.

2. The services: mysql is capped

A runner's container.options (--cpus=4 --memory=10g) applies to the job container only. Verified on a live migrations job:

Container	Reported
job container	HostConfig.Memory=10737418240 ✅
its mysql:8.4 service	mem=0 nanocpus=0 — unbounded ❌

Now --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). Migrations are DDL-heavy, not data-heavy, and the InnoDB buffer pool defaults to 128M — so 2g is ~4× headroom.

Standing rule recorded in decisions.md: any new services: container needs its own explicit cap. It will not inherit one.

3. Peak RSS is now measured every run

#604 asked for peak RSS back after item 1 — the number it sizes both runners' caps on. Rather than a one-off manual docker stats, the test job now ends with a Report peak container memory step reading the cgroup's memory.peak high-water mark (always(), tolerates absence, never fails a build).

This closes a real gap: the 10g cap was inherited, not measured — it traces back to #570 observing a different job. This repo's CI perf issues keep stating numbers from plausibility (#390's "2–4 min" apt-ffmpeg estimate; real 110s, and not load-bearing). Now the number is just there on every run.

Incidental findings

• The format job container is at 3.95 GiB (live docker stats). It would be OOM-killed outright by #604's re-sized 2 GiB small lane — independent confirmation that #407's revert was correct and needed to land before #604 tightens the caps.
• Docs drift from #407: it changed the lane routing without updating docs/ci-cd.md, which still claimed api-docs/format live on small. Corrected here, since I merged that PR.

Docs

• docs/ci-cd.md — new sections for the compiler-server switches and the services:-cap rule; lane table corrected for #407's revert.
• docs/decisions.md — appended entry covering all three items + the standing services: rule. Pure insertion (65 lines, 0 deletions), so the append-only guard passes without [decisions-edit].

Review scope

Not a rubber-stamp candidate despite being config-only — worth a look at:

1. Does UseSharedCompilation as an env var hold for every job, or does any project/target set it explicitly and override? (I checked the default-when-empty condition; a second pair of eyes is cheap.)
2. Is 2g genuinely enough for 787 migrations against MySQL, or does the DDL-heavy claim not survive contact? An OOM here turns into a migrations flake, which we already have history with.
3. Anything that makes the Dockerfile ENV leak into the shipped image (it shouldn't — build stage only).

What's left before #406 closes

Refs, not fixes — two ## Done-when boxes need the merged result:

• peak RSS re-measured post-change and reported on server-management#604
• migrations green with the capped mysql (proves 2g isn't too tight)

🤖 Generated with Claude Code

Items **1 and 2** of #406. Item 3 landed in #407, so this PR says `Refs #406` — see "What's left" for why it doesn't close the issue yet. Refs #406, server-management#604. ## Why The 2026-07-17 bumblebee incident (load **340**, **21 GiB swapped**, prod media down until reboot) was **CI-induced** — prod `ersatztv` itself was healthy at 168 MiB throughout. Infra sizing is #604's boundary; these two levers live here and each does more than any capacity knob. The root cause of the memory spike is that **`VBCSCompiler` is a compiler _server_**: it deliberately outlives the `dotnet build` that started it, keeping its heap warm for the next one. Locally that's a real speedup. **In CI there is no next build** — each job container is torn down at the end of the run — so the 7.8 GB RSS measured on bumblebee is pure cost. It is also the actual reason each job needs a 10 GiB cap in the first place. ## 1. No persistent compiler servers Top-level workflow `env:`, so every `dotnet` call in every job is covered without touching each call site: | 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 | These are MSBuild *properties* set as env vars — MSBuild surfaces env vars as properties, and `UseSharedCompilation` is only defaulted to `true` when empty, so the env var wins. ### The part the issue's suggestion would have missed Workflow `env:` reaches the **runner-side** dotnet jobs only. The `build` job compiles inside `docker build`, where it does **not** propagate — so the same three are set as `ENV` in the SDK stage of `docker/Dockerfile`. That matters more than the rest: `build` is the job **server-management#570 measured pegging 5.999/6 GiB**, i.e. the single job the entire 10 GiB cap is derived from. Fixing only the workflow would have left it untouched. Build-stage only — the final image is `FROM runtime-base`, so nothing lands in the shipped image or affects runtime. ### Verified, not assumed The whole premise is that `UseSharedCompilation=false` *as an env var* actually suppresses the server. Proven on a throwaway project: - default build → **1** `VBCSCompiler` alive afterward - same build with these env vars → **0** - `ErsatzTV.sln` full build under them → `Build succeeded. 0 Error(s)` **Trade-off accepted:** each project's `csc` becomes a fresh process, costing some build time. Worth it — the memory spike is what takes prod down, and the cap that spike forces is what starves the lanes. ## 2. The `services:` mysql is capped A runner's `container.options` (`--cpus=4 --memory=10g`) applies to the **job container only**. Verified on a live `migrations` job: | Container | Reported | |---|---| | job container | `HostConfig.Memory=10737418240` ✅ | | its `mysql:8.4` service | `mem=0 nanocpus=0` — **unbounded** ❌ | Now `--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). Migrations are DDL-heavy, not data-heavy, and the InnoDB buffer pool defaults to 128M — so 2g is ~4× headroom. Standing rule recorded in `decisions.md`: **any new `services:` container needs its own explicit cap.** It will not inherit one. ## 3. Peak RSS is now measured every run #604 asked for peak RSS back after item 1 — the number it sizes both runners' caps on. Rather than a one-off manual `docker stats`, the `test` job now ends with a **Report peak container memory** step reading the cgroup's `memory.peak` high-water mark (`always()`, tolerates absence, never fails a build). This closes a real gap: the 10g cap was **inherited, not measured** — it traces back to #570 observing a *different* job. This repo's CI perf issues keep stating numbers from plausibility (#390's "2–4 min" apt-ffmpeg estimate; real 110s, and not load-bearing). Now the number is just there on every run. ## Incidental findings - **The `format` job container is at 3.95 GiB** (live `docker stats`). It would be **OOM-killed outright** by #604's re-sized 2 GiB `small` lane — independent confirmation that #407's revert was correct and needed to land before #604 tightens the caps. - **Docs drift from #407:** it changed the lane routing without updating `docs/ci-cd.md`, which still claimed `api-docs`/`format` live on `small`. Corrected here, since I merged that PR. ## Docs - `docs/ci-cd.md` — new sections for the compiler-server switches and the `services:`-cap rule; lane table corrected for #407's revert. - `docs/decisions.md` — appended entry covering all three items + the standing `services:` rule. Pure insertion (65 lines, 0 deletions), so the append-only guard passes without `[decisions-edit]`. ## Review scope Not a rubber-stamp candidate despite being config-only — worth a look at: 1. **Does `UseSharedCompilation` as an env var hold for every job**, or does any project/target set it explicitly and override? (I checked the default-when-empty condition; a second pair of eyes is cheap.) 2. **Is 2g genuinely enough** for 787 migrations against MySQL, or does the DDL-heavy claim not survive contact? An OOM here turns into a `migrations` flake, which we already have history with. 3. Anything that makes the Dockerfile `ENV` leak into the shipped image (it shouldn't — build stage only). ## What's left before #406 closes `Refs`, not `fixes` — two `## Done-when` boxes need the merged result: - peak RSS re-measured post-change and **reported on server-management#604** - `migrations` green with the capped mysql (proves 2g isn't too tight) 🤖 Generated with [Claude Code](https://claude.com/claude-code)

timothy added 1 commit 2026-07-17 15:24:54 +02:00

ci(406): disable persistent compiler servers, cap the mysql service, report peak RSS ... e33205e8eb

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) <noreply@anthropic.com>

timothy referenced this issue from a commit 2026-07-17 15:36:28 +02:00

ci(406): disable swap on the capped mysql; fix peak-step placement and failure mode [decisions-edit]

timothy added 1 commit 2026-07-17 15:36:28 +02:00

ci(406): disable swap on the capped mysql; fix peak-step placement and failure mode [decisions-edit] ... 04e3a5249d

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) <noreply@anthropic.com>

timothy referenced this pull request 2026-07-17 15:51:09 +02:00

CI: peak-memory instrument reports cache-inflated `memory.peak`; sample true peak anon instead #412

timothy added 1 commit 2026-07-17 15:53:44 +02:00

ci(406): report anon/file split — memory.peak counts page cache, not just RSS [decisions-edit] ... 7859510c78

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) <noreply@anthropic.com>

timothy referenced this pull request 2026-07-17 16:08:29 +02:00

CI: cut peak build RSS (Roslyn), cap the `services:` mysql, and move `api-docs`/`format` back to `ubuntu-latest` #406

timothy added 1 commit 2026-07-17 16:14:38 +02:00

ci(406): apply the memory-swap rule to our own two sites; stop leaning on the peak reading [decisions-edit] ... 469d725559

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) <noreply@anthropic.com>

timothy referenced this pull request 2026-07-17 16:27:07 +02:00

CI: cut peak build RSS (Roslyn), cap the `services:` mysql, and move `api-docs`/`format` back to `ubuntu-latest` #406

timothy commented 2026-07-17 17:01:50 +02:00

Author

Owner

Copy Link

Copy Source

Independent A/B on the compiler-server switch — measured, and it confirms item 1

Picking up #406 after the handoff. Before merging I ran a controlled A/B against your exact env in the real CI image, and it settles the memory question your PR body flagged for review (#2 on your list). Method matters here, so the details:

• CI toolchain image ersatztv-ci:07048b8, --cpus=4 --memory=10g, cold tree each run (obj/bin wiped), full dotnet build ErsatzTV.sln -c Release.
• --memory-swap pinned to the cap so pressure shows as real anon, not swap. This is load-bearing — see the correction below.
• Metric: peak of cgroup memory.stat anon (excludes page cache), sampled at 0.5s. Every build verified EXIT=0 CS_ERRORS=0.
• Arms interleaved (ON/OFF/ON/OFF) so host-load drift cancels.

Arm	Peak anon	Build time
Shared compilation ON (baseline)	7983 / 8050 MiB (~8.0 GiB)	180–240s
All three switches OFF (this PR)	5653 / 5717 MiB (~5.7 GiB)	272–297s

Your item 1 is right: ~29% lower peak anon, for ~+50% wall time. Memory is the constraint that took prod down (the incident was a swap-thrash, not a slow build), so that trade is correct. Verdict on your review-scope Q2 (is 2g enough / does the memory claim survive): yes on both — migrations is green at head with the 2g cap.

One correction I owe, because I got it backwards first

My first A/B (swap left at Docker's default) showed the opposite — servers-off looked worse on memory. That reading was wrong, and the cause is exactly the swap default the other session flagged on #604: with swap allowed, the shared server's big idle heap gets swapped out, so a resident-anon sampler undercounts the ON arm. Disabling swap in the measurement is what makes the comparison honest. I'm flagging this because the mistake is subtle and I'd rather it be on the record than have someone re-derive the wrong sign later.

A framing nit (not a blocker)

The workflow comment says the server is "pure cost" because it "outlives the build … each job container is torn down." For the three containerized jobs (test/migrations/functional-e2e/api-docs/format all have container:), the server dies with the container regardless — I confirmed on a live host that no CI job's VBCSCompiler persists in a host cgroup; the only one alive was an SSH session's. So the between-builds persistence argument doesn't apply there. The real reduction comes from not running N analyzer-laden csc heaps concurrently (this repo runs AnalysisLevel: latest-All + Roslynator + Sonar + Meziantou). Where the between-builds argument is literally true and correctly handled is dependency-scan.yml's uncontainerized cron job — good catch keeping that covered. Not asking for a code change; just noting the "why" so the next reader isn't misled.

Docs-only follow-up filed as #416 (docs-only PRs run the full matrix; includes the branch-protection trap that makes a naive paths-ignore brick the merge gate).

Running a cold review of the current head before I merge.

## Independent A/B on the compiler-server switch — measured, and it confirms item 1 Picking up #406 after the handoff. Before merging I ran a controlled A/B against **your exact env** in the real CI image, and it settles the memory question your PR body flagged for review (#2 on your list). Method matters here, so the details: - CI toolchain image `ersatztv-ci:07048b8`, `--cpus=4 --memory=10g`, cold tree each run (`obj`/`bin` wiped), full `dotnet build ErsatzTV.sln -c Release`. - **`--memory-swap` pinned to the cap** so pressure shows as real anon, not swap. This is load-bearing — see the correction below. - Metric: peak of cgroup `memory.stat` **`anon`** (excludes page cache), sampled at 0.5s. Every build verified `EXIT=0 CS_ERRORS=0`. - Arms **interleaved** (ON/OFF/ON/OFF) so host-load drift cancels. | Arm | Peak anon | Build time | |---|---|---| | Shared compilation **ON** (baseline) | 7983 / 8050 MiB (~**8.0 GiB**) | 180–240s | | All three switches **OFF** (this PR) | 5653 / 5717 MiB (~**5.7 GiB**) | 272–297s | **Your item 1 is right: ~29% lower peak anon, for ~+50% wall time.** Memory is the constraint that took prod down (the incident was a swap-thrash, not a slow build), so that trade is correct. Verdict on your review-scope Q2 (is 2g enough / does the memory claim survive): yes on both — `migrations` is green at head with the 2g cap. ### One correction I owe, because I got it backwards first My *first* A/B (swap left at Docker's default) showed the **opposite** — servers-off looked *worse* on memory. That reading was wrong, and the cause is exactly the swap default the other session flagged on #604: with swap allowed, the shared server's big idle heap gets **swapped out**, so a resident-anon sampler *undercounts* the ON arm. Disabling swap in the measurement is what makes the comparison honest. I'm flagging this because the mistake is subtle and I'd rather it be on the record than have someone re-derive the wrong sign later. ### A framing nit (not a blocker) The workflow comment says the server is "pure cost" because it "outlives the build … each job container is torn down." For the three **containerized** jobs (`test`/`migrations`/`functional-e2e`/`api-docs`/`format` all have `container:`), the server dies with the container regardless — I confirmed on a live host that no CI job's `VBCSCompiler` persists in a host cgroup; the only one alive was an SSH session's. So the *between-builds persistence* argument doesn't apply there. The real reduction comes from **not running N analyzer-laden `csc` heaps concurrently** (this repo runs `AnalysisLevel: latest-All` + Roslynator + Sonar + Meziantou). Where the between-builds argument **is** literally true and correctly handled is `dependency-scan.yml`'s **uncontainerized** cron job — good catch keeping that covered. Not asking for a code change; just noting the "why" so the next reader isn't misled. Docs-only follow-up filed as **#416** (docs-only PRs run the full matrix; includes the branch-protection trap that makes a naive `paths-ignore` brick the merge gate). Running a cold review of the current head before I merge.

timothy commented 2026-07-17 17:03:08 +02:00

Author

Owner

Copy Link

Copy Source

Cold review of the current head — MERGEABLE

Ran an independent, review-only pass over the current head diff (a fresh reviewer with no implementation role; the prior verdicts pointed at older shas, so this re-establishes the H10 verdict at the head). Focus was correctness that would break CI or the prod release path, not perf. All six areas verified against the actual files — all confirmations, no blockers:

• Dockerfile ENV cannot leak into the shipped image. Final stage is FROM runtime-base and pulls only COPY --from=build /app; the ENV is stage-local to build and does not cross the FROM.
• Cache placement is fine. The ENV sits below the openapi-generator wget/jar layer (stays cached) and above dotnet restore (which the csproj COPYs already invalidate) — one expected restore-layer bust on the first post-merge build, unavoidable.
• YAML valid. No duplicate env: key in either workflow; the folded services.mysql.options scalar folds to a well-formed option string with the new --memory/--memory-swap/--cpus at the right indent.
• migration-smoke.sh --memory-swap 2g on the prod release path is safe — throwaway rehearsal container against a DB copy; an OOM fails the release closed by design and never touches the live prod container. 2g is ample for a migration-apply runtime (the 5.7 GiB figure is a full solution build, a different workload).
• Peak-memory step is truly advisory — continue-on-error: true overrides the -e -o pipefail from defaults.run.shell: bash; internal cat/awk/redirects all guarded. Cannot redden a green job.
• No build-correctness delta — the three switches change process lifetime/reuse only; csc runs with identical analyzer/warning behavior and output.

Docs (ci-cd.md, decisions.md) match the code and describe the --memory-swap 2× default and the memory.peak-vs-anon caveat accurately.

CI: all 9 checks green on 469d7255 (incl. migrations with the 2g mysql cap, and functional-e2e).

Review-verdict: MERGEABLE @ 469d725559

Merging, then closing #406.

## Cold review of the current head — MERGEABLE Ran an independent, review-only pass over the current head diff (a fresh reviewer with no implementation role; the prior verdicts pointed at older shas, so this re-establishes the H10 verdict at the head). Focus was correctness that would break CI or the prod release path, not perf. All six areas verified against the actual files — all confirmations, no blockers: - **Dockerfile `ENV` cannot leak into the shipped image.** Final stage is `FROM runtime-base` and pulls only `COPY --from=build /app`; the `ENV` is stage-local to `build` and does not cross the `FROM`. - **Cache placement is fine.** The `ENV` sits below the openapi-generator `wget`/jar layer (stays cached) and above `dotnet restore` (which the csproj COPYs already invalidate) — one expected restore-layer bust on the first post-merge build, unavoidable. - **YAML valid.** No duplicate `env:` key in either workflow; the folded `services.mysql.options` scalar folds to a well-formed option string with the new `--memory`/`--memory-swap`/`--cpus` at the right indent. - **`migration-smoke.sh --memory-swap 2g` on the prod release path is safe** — throwaway rehearsal container against a DB copy; an OOM fails the release *closed* by design and never touches the live prod container. 2g is ample for a migration-*apply* runtime (the 5.7 GiB figure is a full solution *build*, a different workload). - **Peak-memory step is truly advisory** — `continue-on-error: true` overrides the `-e -o pipefail` from `defaults.run.shell: bash`; internal `cat`/`awk`/redirects all guarded. Cannot redden a green job. - **No build-correctness delta** — the three switches change process lifetime/reuse only; `csc` runs with identical analyzer/warning behavior and output. Docs (`ci-cd.md`, `decisions.md`) match the code and describe the `--memory-swap` 2× default and the `memory.peak`-vs-anon caveat accurately. CI: all 9 checks green on `469d7255` (incl. `migrations` with the 2g mysql cap, and `functional-e2e`). Review-verdict: MERGEABLE @ 469d72555929d648ec39ddb7c896dffa810b4a6d Merging, then closing #406.

timothy referenced this pull request 2026-07-17 17:04:17 +02:00

CI: cut peak build RSS (Roslyn), cap the `services:` mysql, and move `api-docs`/`format` back to `ubuntu-latest` #406

timothy merged commit b3dbbd0528 into main 2026-07-17 17:17:55 +02:00

timothy deleted branch ci/406-roslyn-mysql-caps 2026-07-17 17:17:55 +02:00

timothy referenced this issue from a commit 2026-07-17 17:17:55 +02:00

Merge pull request 'ci(406): disable persistent compiler servers, cap the `services:` mysql, report peak RSS' (#411) from ci/406-roslyn-mysql-caps into main

timothy referenced this pull request 2026-07-17 17:18:31 +02:00

CI: cut peak build RSS (Roslyn), cap the `services:` mysql, and move `api-docs`/`format` back to `ubuntu-latest` #406

timothy referenced this issue from a commit 2026-07-19 20:49:19 +02:00

ci(412): sample true peak-anon in the test job, not cache-inflated memory.peak

timothy referenced this pull request 2026-07-19 21:10:00 +02:00

CI: peak-memory instrument reports cache-inflated `memory.peak`; sample true peak anon instead #412

timothy referenced this pull request 2026-07-21 01:05:00 +02:00

Make decision knowledge lifecycle-addressable and retrieval-efficient #521

timothy referenced this issue from a commit 2026-07-21 03:01:07 +02:00

feat(521): reconcile #390/#406 + #411 + append-only supersession; seed exemplars [decisions-edit]

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

ad-hoc

One-off / ad-hoc work not tracked by a dedicated issue

api

REST API / HTTP endpoints

bug

Something isn't working

ci-cd

Build, test, deploy pipeline

content

Channel content / schedules / playlists

dependencies

Dependency updates (Renovate)

enhancement

New feature or improvement

frontend

ChicoryTV React SPA frontend

in-progress

Claimed by an active session — do not pick up

jellyfin

Jellyfin tuner / IPTV integration

parked

Excluded from automatic queue pickup; work only when explicitly selected

priority: high

priority: low

priority: medium

review

Adversarial review finding

security

Security / vulnerability fix

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

renovate timothy (Timothy)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: timothy/ersatztv#411