Merge pull request 'Issues 290-291: a delivery adopted while it waited never got its check; a planned maintenance window failed the applies that met it' (#164) from issues/290-291-what-waits-was-never-asked into main

2026-10-07 11:20:21 +00:00
parent f8189da489 553d81be60
commit eef2c5e082
2 changed files with 153 additions and 0 deletions
@@ -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.<repository>@<commit>.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.
@@ -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 <module>.bundle-code (…): cannot fetch http://<store>/v2/<module>/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".