diff --git a/conformance/tests/l0-static/ha-control-plane-as-code/check.sh b/conformance/tests/l0-static/ha-control-plane-as-code/check.sh index 03e44f1..a22b60b 100755 --- a/conformance/tests/l0-static/ha-control-plane-as-code/check.sh +++ b/conformance/tests/l0-static/ha-control-plane-as-code/check.sh @@ -445,8 +445,21 @@ join = re.search(r"--server https://\{\{\s*k3s_api_host", K3S_TEXT) check(join is not None, "roles/k3s_server does not join secondaries with `--server https://{{ k3s_api_host }}` " "— without a join target the servers are separate single-node clusters") -join_when = re.search(r"--server https://.*?\n(?:.*\n){0,10}?\s*when:\s*k3s_server_mode\s*==\s*'secondary'", - K3S_TEXT, re.S) +# LINE-BOUNDED ON PURPOSE -- DO NOT "SIMPLIFY" BACK TO `.*` + re.S. +# This was `--server https://.*?\n(?:.*\n){0,10}?\s*when:...` with re.S, which made every `.` +# match newlines too. That is nested unbounded repetition (a `{0,10}` of a quantifier that can each +# swallow the whole document), so a FAILED or late match backtracks exponentially. It cost +# **19m54s of CPU in CI** (measured 2026-10-18 via `docker top` on the live job container) and made +# the whole merge gate take ~28 minutes instead of ~2 on any change touching `infra/**` -- because +# `ci/touched-requirements.sh` maps `infra/*` to R-ENV-01, which pulls in this whole-repo check. +# It was latent for as long as the regex existed and only crossed the threshold when +# roles/k3s_server grew by ~100 lines (the kubelet graceful-shutdown config, PR #71): K3S_TEXT is +# every *.yml in that role concatenated, so the role growing is the input growing. +# `[^\n]*` cannot cross a line, so the `{0,10}` is a genuine ten-line window with nothing to +# backtrack into. Measured on the same input: 0.2 ms, same verdict (match found). +join_when = re.search( + r"--server https://[^\n]*\n(?:[^\n]*\n){0,10}?[ \t]*when:[ \t]*k3s_server_mode[ \t]*==[ \t]*'secondary'", + K3S_TEXT) check(join_when is not None, "the join install task is not gated on k3s_server_mode == 'secondary' — the founder " "would also try to join itself") diff --git a/decisions/2026-10-18-ha-check-catastrophic-regex.md b/decisions/2026-10-18-ha-check-catastrophic-regex.md new file mode 100644 index 0000000..f7594a9 --- /dev/null +++ b/decisions/2026-10-18-ha-check-catastrophic-regex.md @@ -0,0 +1,76 @@ +--- +date: 2026-10-18 +branch: fix/ha-check-catastrophic-regex +title: "One regex made the merge gate take 28 minutes instead of 2" +--- + +## What changed + +The merge gate started taking **~28 minutes** instead of its usual **<2 minutes**, on every change +touching `infra/**`. One regex was responsible for essentially all of it. + +`conformance/tests/l0-static/ha-control-plane-as-code/check.sh` contained: + +```python +join_when = re.search(r"--server https://.*?\n(?:.*\n){0,10}?\s*when:\s*k3s_server_mode\s*==\s*'secondary'", + K3S_TEXT, re.S) +``` + +With `re.S`, every `.` also matches newlines, so `.*?` and the nested `(?:.*\n){0,10}?` can each +consume the entire document. That is **nested unbounded repetition** — ten levels of a quantifier +that can each swallow everything — and a failed or late match backtracks exponentially. + +Replaced `.` with `[^\n]*` and dropped `re.S`, so the `{0,10}` is a genuine ten-line window with +nothing to backtrack into. Same intent, same verdict, no DOTALL. + +**Why it surfaced now, and it was mine.** `K3S_TEXT` is every `*.yml` in `roles/k3s_server` +concatenated. The regex was latent for as long as it existed; **PR #71 added ~100 lines to that role +(the kubelet graceful-shutdown config), which grew the input past the point where exponential +backtracking stopped being survivable.** Nothing about the check changed — my change grew its input. + +**Why only `infra/**` changes were slow.** `ci/touched-requirements.sh:268` maps `infra/*` to +`R-ENV-01`, and the resulting L0 run is `--include-scope whole-repo --requirements R-ENV-01`, which +pulls in this whole-repo check. Doc-only and `platform/**` changes never selected it, which is why +#69 (1m55s) and #72 (1m53s) stayed fast while #71 (27m44s), the #73 merge (27m46s) and #75 all ran +~28 minutes. + +## What I verified + +* **The hot spot, by measurement rather than reading.** `docker top` on the live CI job container for + run #299 showed `python3 -` with **19:54 of CPU time** under + `ha-control-plane-as-code/check.sh`, inside + `conformance/run.sh --layer l0 --include-scope whole-repo --requirements R-ENV-01`. +* **Reproduced locally**: the check did not finish within a 100-second budget on an unmodified + checkout. +* **Localised to the exact line** by extracting the inline Python and running it under + `faulthandler.dump_traceback_later(25)`, which named `re/__init__.py:177 in search` called from + the `join_when` line — not inferred from reading the patterns. +* **The pattern is unique in the repo**: `grep -rlnE "\{0,[0-9]+\}\?" conformance/tests/l0-static/*/check.sh` + returns only this file, so there is no sibling with the same hazard. +* **The fix is fast AND verdict-preserving**, tested against the real `K3S_TEXT` (22,179 chars, 403 + lines): the new regex matches (`YES`) in **0.2 ms**. Sanity-checked that the thing it looks for + genuinely exists — one `--server https://` and three `k3s_server_mode == 'secondary'` occurrences. +* **The whole check now runs in 0.199s, rc 0, 115 assertions**, with the same success message as + before. + +## What I did NOT verify + +* **I did not re-time the full gate in CI after the fix.** The expectation is that `infra/**` changes + return to the ~2-minute range, but that is arithmetic from the 19m54s figure, not a measured + post-fix run. The PR's own CI run is the first real datapoint — read it. +* **I did not audit the other 23 L0 checks for different performance pathologies.** This one was + found because it was actively burning CPU in a live job; a check that is merely slow (not + exponential) would not have stood out, and `ci/gate.sh` has **no per-stage timing** at all + (`grep -nE "SECONDS|date \+|elapsed"` finds nothing), so there is no data on where the remaining + time goes. +* **Nothing prevents this class of bug returning.** No check asserts gate stage runtime, and no lint + forbids nested quantifiers under `re.S`. A timing budget per stage, or a `re` lint, would close it; + neither is written here. +* **I did not verify my claim about *why* only `infra/**` was affected beyond the path mapping and + the observed durations.** The correlation is strong (5 runs, two clear clusters) and the live + process list confirms the `--requirements R-ENV-01` scope, but I did not read + `conformance/run.sh`'s scope-selection logic end to end. +* The regex rewrite assumes the `when:` clause is indented with spaces or tabs only (`[ \t]*`). A + file using some other whitespace before `when:` would now fail to match where the old DOTALL + version would have succeeded. Unlikely in YAML, and the current input matches, but it is a + narrowing.