From 3dce3d18894c80e346170898150aed608bbc04f7 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Fri, 4 Sep 2026 15:24:41 +0700 Subject: [PATCH] =?UTF-8?q?Features:=20#330=20=E2=80=94=20the=20split=20re?= =?UTF-8?q?load=20class,=20and=20the=20top-level=20coverage=20checker?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- 11-Features.md | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/11-Features.md b/11-Features.md index fbbab28..2641ed7 100644 --- a/11-Features.md +++ b/11-Features.md @@ -4029,3 +4029,69 @@ startup snapshot and live, at different sites, so no single class fits either. A them still under-claims. That count and both verdicts are now in `ConfigRef`'s class doc so the next person does not measure it again. Twice now — `worktreeGroup`, then `primary`/`configReload` — the untriaged kind hid among the correct kind. + +--- + +## A reload can now say "half of this applied" + +**What it does.** `ConfigRef` has a fourth reload class, `split`, for a key that is read **both** +off the startup snapshot and live off the config supplier, at different sites. `health:` and +`coordinator:` are both like that. A reload that changes either now names the key and says which +half is already live and which half waits for a restart. `Outcome` carries a new `split` list +beside `deferred`. + +**On.** Always on (fleetd #330, merged `7b918c5`). Visible in the reload summary: + +``` +config reloaded; partially live — coordinator: the LeadMailbox connection (uri, uriEnv, selfId, +prefetch) is opened once and needs a restart; the broker URI env-var name kept out of a member's +environment is read live on every spawn and already applied +``` + +**Why it exists.** Before this, changing either key reported a bare `config reloaded`. That +under-claims: it tells the operator a change applied when half of it did not. The three existing +classes could not express the truth, and forcing one of them would be wrong in one direction or the +other. The direction matters. Over-claiming a restart costs an unnecessary restart, which the +operator can see and recover from. Under-claiming is what this file's own doc calls "the worst +thing a reload can do to an operator debugging one". `split` is the only option that is simply +true. Making the frozen half live was rejected for `coordinator`: `selfId` names this daemon's own +AMQP inbox queue, so changing it live is a distributed-identity problem, not a reconnect. + +**One thing to know for maintenance.** Membership in `SPLIT_KEYS` is **not** proof that any +reporting code exists for that key. `ConfigRefTopLevelCoverageTest` reads a name in the set as +"triaged" and stops there — it cannot see whether `changedSplitKeys` has a branch for it. Measured: +dropping the `coordinator` branch while leaving the name in the set left both the coverage test and +the `assert` silent; only three hand-written behavioural tests caught it. The same one-way shape is +older than this change — `assert COLD_KEYS.containsAll(changed)` catches "reported but not listed" +and never the reverse. `fleet:` is already an instance of the gap: it is split too and it sits in +the checker's hot-exclusion hatch, so a changed `fleet.leaders` still reports nothing. Both are open +in fleetd #333. + +--- + +## Every top-level config key must be triaged before it ships + +**What it does.** `ConfigRefTopLevelCoverageTest` enumerates `FleetConfig`'s record components by +reflection and requires each one to sit in exactly one of four buckets: `COLD_KEYS`, the deferred +set, `SPLIT_KEYS`, or a pinned hot-exclusion set. A new key in none of them fails the build by name. +It prints its own denominator every run: + +``` +FleetConfig top-level coverage — 22 components total: 5 cold [...], 11 deferred [...], +2 split [health, coordinator], 4 hot-excluded [...] +``` + +**On.** Always on — it is a unit test (fleetd #330). + +**Why it exists.** Three times the same key-was-forgotten bug shipped: `worktreeGroup` (#323), +then `primary` and `configReload` (#326), then `fleet` (#333). Each time a reload reported success +for a change the daemon never picked up. "Not mentioned in `ConfigRef`" looks identical for a key +that is correctly hot and a key nobody triaged, so the forgotten kind kept hiding among the correct +kind. This is the top-level twin of the per-profile checker #323 added. + +**One thing to know for maintenance.** Read the test's own javadoc before trusting a green run. It +says plainly what it cannot do: it proves the record's *shape* is triaged, and it **cannot** prove a +citation is true. "Compared in `changedDeferredKeys`" and "read live off `config.get()`" are facts +about other files that a reflection test over one record cannot inspect. That disclosure is not +modesty — it is what made the two gaps in #333 findable in minutes. Hold any checker in this repo to +the same standard: say what it does not cover, next to what it does.