fix(conformance): one regex made the merge gate take 28 minutes instead of 2 #76

Merged
jakman85 merged 1 commits from fix/ha-check-catastrophic-regex into main 2026-10-18 07:51:51 +00:00
2 changed files with 91 additions and 2 deletions

No files matched your search

@@ -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")
@@ -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.