From 553d81be60fc66ceb345728ee6b3fa36e82bcf98 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 7 Oct 2026 13:16:55 +0200 Subject: [PATCH] Issues 290-291: a delivery adopted while it waited never got its check; a planned maintenance window failed the applies that met it --- .../00-report.md | 71 ++++++++++++++++ .../00-report.md | 82 +++++++++++++++++++ 2 files changed, 153 insertions(+) create mode 100644 04-ISSUES/290-a-delivery-adopted-while-it-waited-never-got-its-check/00-report.md create mode 100644 04-ISSUES/291-a-planned-maintenance-window-failed-the-applies-that-met-it/00-report.md diff --git a/04-ISSUES/290-a-delivery-adopted-while-it-waited-never-got-its-check/00-report.md b/04-ISSUES/290-a-delivery-adopted-while-it-waited-never-got-its-check/00-report.md new file mode 100644 index 00000000..94af3582 --- /dev/null +++ b/04-ISSUES/290-a-delivery-adopted-while-it-waited-never-got-its-check/00-report.md @@ -0,0 +1,71 @@ +--- +status: located +opened: 2026-10-07 +located-in: [mesh-catalog modules/mesh-delivery] +fixed-by: mesh-catalog PR #101 +amended-design: +--- + +# 290. A delivery adopted while it waited never got its check + +## Symptom + +On 2026-10-07, an hour after the delivery owner (ADR 0239, to-be 47) first started, its self-check raised +`delivery.@.stalled` for the operator. The delivery was a pull request in this +repository, opened before merge checks existed. `show` said it was `proposed`, waiting for "its check". +Nothing in the controller's journal had ever asked for that check. + +## Diagnosis + +The delivery owner learns of a pull request's head from the forge's `pull.updated` announcement. The +controller hears the same announcement and asks the build seat for the head's check. The delivery owner +then waits for the controller's `checked` verdict. + +When the delivery owner first started, its consumer read the bus's history of announcements. It made one +`proposed` delivery for each open pull request it found there. The controller had taken those +announcements long before, and for this head it had never run a check. So the delivery was proposed and +waiting for a verdict that no one would produce. The delivery owner itself never asks for a head's own +check. It only asks for a group's composed check and for a person's `recheck`. The `proposed` bound +counted from when the delivery was proposed, so the first thing to notice was the hour-long stall, and +it handed the delivery to the operator. + +A missed announcement would end the same way: the controller restarting at the wrong moment, or an +announcement it dropped. The delivery would be proposed, nobody would ask for its check, and an hour +later the operator would be paged. + +Ruled out: the controller's check path. Asking for the check through `delivery-check` with one head runs +the same path a fresh announcement does. + +## Fix + +- **A delivery that waits for a check nobody asked for gets one asked.** A delivery that is `proposed`, + has no verdict, is not merged, and has had no check asked by its owner gets one once it has waited past + a grace period. The grace (15 minutes) is longer than a check takes from announcement to verdict. The + check is asked through `delivery-check` for that head alone. The rule covers a head read back after a + restart, a head replayed at first start, and a missed announcement. An ask the controller does not + accept is not recorded, and it is tried again a minute later. The ask is kept on the delivery and noted + on its commit. +- **An announcement that already carries a verdict uses it.** If the gate's status on that head is + already decided (passed, warned or failed), the delivery takes that verdict. A pending or errored + status is asked again. +- **The `proposed` bound now means "asked and not answered".** It counts from the ask. A delivery never + asked counts from the end of the grace period, so a check that cannot be asked at all is still listed. +- **Healer H2 may ask once more.** A new table row, `proposed —re-ask→ proposed`, lets H2's `close` + ask a stalled check a second time. Its guard refuses a third ask. After that, `stalled` says the + delivery belongs to the operator. + +## How it is checked + +The mesh-delivery tests `checkask_test.go` cover: + +- A proposed delivery read back after a restart has its check asked once, for its head alone, and a + passing verdict makes it ready. +- A missed announcement is asked after the grace period, but not within it, not while the controller is + down, and never for a head that already has a verdict. +- An announcement that carries a decided gate status uses it. +- H2 re-asks once, a second re-ask is refused by the table, and a late verdict still settles the delivery. + +The table tests walk the new row, and the pairs the table does not hold are refused by name. Live: once +the fix is rolled out, the stalled delivery has its check asked at the owner's first tick. The controller +journal then shows its verdict, and `show` moves from proposed to checked to ready without a person +acting. diff --git a/04-ISSUES/291-a-planned-maintenance-window-failed-the-applies-that-met-it/00-report.md b/04-ISSUES/291-a-planned-maintenance-window-failed-the-applies-that-met-it/00-report.md new file mode 100644 index 00000000..ee8e454e --- /dev/null +++ b/04-ISSUES/291-a-planned-maintenance-window-failed-the-applies-that-met-it/00-report.md @@ -0,0 +1,82 @@ +--- +status: located +opened: 2026-10-07 +located-in: [mesh-host internal/apply, mesh-host internal/store] +fixed-by: mesh-host PR #49 +amended-design: +--- + +# 291. A planned maintenance window failed the applies that met it + +## Symptom + +At 03:30 on 2026-10-07, the artifact store's nightly collection held the store's server still for about +twelve seconds. This is its maintenance window (ADR 0189). In those seconds, the node-engine's own +reconcile on the same machine failed every bundle it declares, one after another: + +> `failed .bundle-code (…): cannot fetch http:///v2//code/blobs/sha256:…: +> dial tcp …: connect: connection refused` + +The failures covered the database, identity, object store, forge and vault bundles, the node tools, and +about thirty more. The engine then held the machine until its next pass. Nothing was lost. But a window +the mesh plans and declares should not fail an apply anywhere. + +## Diagnosis + +Two facts met. + +**Every apply fetches every archive it declares.** It needs the archive's bytes to know its digest, so +an archive that has not changed is fetched too. The store is where those bytes come from. + +**The window and the apply were not ordered.** Issue 224 taught the apply which containers a window +holds still, so it no longer recreates the store's server in the middle of a collection. On purpose, the +window does not take the apply lock: holding the lock for the whole window would make a push that +arrives mid-window wait (issue 185). So the scheduled step opened its window while a reconcile was +already running. That reconcile had started before the window existed, and it reached its archives with +the store's server already stopped under it. Every fetch failed on its first refusal, because the fetch +had no idea a window could explain a refusal. + +On every other machine the same thing can happen with nothing to explain it. The store's window is +recorded only on the store's own machine. + +Ruled out: the store itself. The collector ran and finished as declared, and the server came back. + +## Fix + +The simplest fix that is right on every machine. All of it is in the node-engine: + +- **On the store's machine, a window opens only once no apply is in flight.** The scheduled step waits + until the apply lock is free. It holds the lock only while it writes the window's record, then lets it + go. So an apply that started before the window has finished first, and an apply that starts afterwards + reads the window. A push arriving mid-window still never waits behind the window itself. The wait is + bounded: past 15 minutes the window opens anyway, said, and the applies wait for it instead. +- **On the store's machine, an apply that meets the window waits for it.** Suppose a fetch gets no + answer from the store while a window is open on this machine. Then the apply checks the window again + every few seconds, for up to 20 minutes, and fetches again once the window closes. +- **On any other machine, a store that does not answer gets a bounded retry.** The store's window is not + published to other machines. Instead, such a fetch is retried with backoff out of one budget for the + whole apply: three minutes, many times the window measured. The budget is shared by every fetch in the + apply, so a store that is really down costs one bounded wait, not one per bundle. The engine's next pass + retries whatever still failed. The bound decides only whether one pass fails, never whether the machine + converges. +- **Only a store that does not answer is waited for.** That means a refused or reset connection, a cut + response, a timeout, or a gateway saying the server behind it is away. When the store does answer, with + a missing blob or the wrong bytes, the fetch fails at once as before. + +## How it is checked + +The mesh-host tests in `internal/apply/store_away_test.go` replay the 03:30 sequence. They use a pretend +runtime with the store's server and collector, and a pretend store whose address refuses connections +while the server is held still: + +- An apply on the store's machine that starts inside the window waits, and unpacks its bundle once the + window closes. +- A collection due while an apply holds the lock opens its window only after that apply ends. The test + checks that stop, the step, and start then happen in that order. +- An apply on another machine waits for a store that comes back. +- Six bundles from a store that stays down cost one bounded wait, not six. +- A store that answers 404 is asked once, and the fetch fails at once. + +Live: the nightly collection on the store's machine no longer leaves `failed … connection refused` lines +in the node-engine's apply. A window that meets an apply in flight says "an apply is in flight … the +window opens once it ends".