From aebca91a47a1c935050e75f505ffc61fc7c2727f Mon Sep 17 00:00:00 2001 From: PlatformSREEngineer Date: Sat, 5 Sep 2026 20:36:31 +0000 Subject: [PATCH 1/3] fix(chart): stop the single-replica worker readiness probe from 502ing all plugin traffic (BLO-31945) The worker StatefulSet runs replicas: 1 behind a readinessProbe with timeoutSeconds: 5. A readiness probe sheds traffic to healthy peers; at one replica there are none, so three slow /healthz responses delete the only Service endpoint and every API-to-worker call 502s with "Worker tier unreachable" from worker-tier-proxy.ts. Alertmanager webhook delivery fails with it, then retries 4x into the worker that just failed its probe. Measured on paperclip-0 over 24h to 2026-09-05T20:30Z (kubelet prober_probe_*), same endpoint and window: Readiness (timeout 5s): 100.2 failed / 8559 = 1.171% Liveness (timeout 10s): 13.1 failed / 2838 = 0.462% Readiness fails at 2.53x the liveness rate on the identical /healthz, and the per-probe timeout is the only material difference. p99.9 of successful liveness probes is 8.59s, so the 5s readiness timeout sat below the endpoint's own tail latency. Raise readiness timeoutSeconds 5 -> 10 (matching liveness, above the measured tail) and periodSeconds 10 -> 15 so a probe cannot outlast its period. Liveness and startup are unchanged. Worst-case endpoint removal goes 30s -> 45s, which costs nothing at replicas: 1 where removal has no upside. Mitigation, not a cure: the durable fix is a second replica (BLO-29307 / BLO-29004) or a /healthz that cannot be blocked by the event loop. The decision to keep rather than retire the probe is recorded in values.yaml. Co-Authored-By: Paperclip --- deploy/helm/paperclip/values.yaml | 58 +++++++++++++++++++++++++++---- 1 file changed, 52 insertions(+), 6 deletions(-) diff --git a/deploy/helm/paperclip/values.yaml b/deploy/helm/paperclip/values.yaml index 5db57de58f77..d58d7c8d7fc3 100644 --- a/deploy/helm/paperclip/values.yaml +++ b/deploy/helm/paperclip/values.yaml @@ -396,10 +396,56 @@ workerProbes: timeoutSeconds: 10 failureThreshold: 6 # -- Worker readiness probe. - # Readiness gates the API-to-worker Service endpoint. Keep it tighter than - # liveness so API callers stop routing to a worker whose event loop is not - # answering promptly; during a long-but-recoverable stall the pod may stay - # alive while temporarily leaving the Service endpoints. + # + # BLO-31945 — why this is no longer tuned "tighter than liveness". + # + # The rationale here used to read: "Keep it tighter than liveness so API + # callers stop routing to a worker whose event loop is not answering + # promptly." That holds for a horizontally-scaled tier, where readiness + # sheds traffic to healthy peers. This StatefulSet runs `replicas: 1`, so + # there is no peer to shed to: removing the only endpoint does not redirect + # plugin traffic, it 502s it. Every API-to-worker call then fails in the + # `catch` at `server/src/routes/worker-tier-proxy.ts` with "Worker tier + # unreachable", which is how a `/healthz` latency blip became a total + # plugin-tier outage — Alertmanager webhook delivery included. Alertmanager + # then retries each delivery 4x into the worker that just failed its probe. + # + # Measured on `paperclip-0` over the 24h to 2026-09-05T20:30Z from the + # kubelet `prober_probe_*` metrics. Same endpoint, same pod, same window: + # + # probe timeout failed / total failure rate + # Readiness 5s 100.2 / 8559 1.171% + # Liveness 10s 13.1 / 2838 0.462% + # + # Readiness failed at 2.53x the liveness rate against the identical + # `/healthz`, and the per-probe timeout is the only material difference + # between them. The tail latency confirms the cause: p99.9 of *successful* + # liveness probes is 8.59s, i.e. `/healthz` legitimately takes longer than + # the old 5s readiness timeout. A timeout set below the endpoint's own tail + # is a false-positive generator, not a sensitivity knob. + # + # Trap when re-deriving this: `prober_probe_duration_seconds` is a + # SUCCESS-ONLY histogram — its `_count` matches + # `prober_probe_total{result="successful"}`, not the total. Timed-out probes + # are censored out of it, so the bucket distribution on its own reads as + # "almost nothing exceeds 5s" and will invert this diagnosis if used without + # the failure counter. + # + # timeoutSeconds now matches liveness (10s), above the measured tail, and + # periodSeconds moves to 15 so a probe cannot outlast its own period. + # Worst-case endpoint removal becomes 3 x 15s = 45s rather than 30s; that + # slower detection costs nothing real at `replicas: 1`, because endpoint + # removal has no upside here in the first place. + # + # Decision, recorded explicitly rather than left implicit: readiness is + # KEPT, not retired. It still gates the endpoint during rollout, and + # deleting it would make a genuine wedge hang API callers until TCP timeout + # instead of failing fast. What is retired is tuning it as though it could + # shed load. This is mitigation, not a cure: at the liveness failure rate + # the probe still drops the endpoint occasionally, and the durable fix is a + # second replica (BLO-29307 / BLO-29004) or a `/healthz` that cannot be + # blocked by the event loop. Until one of those lands, this probe must be + # tolerant rather than tight. readiness: httpGet: path: /healthz @@ -408,8 +454,8 @@ workerProbes: - name: Host value: 127.0.0.1:3100 initialDelaySeconds: 5 - periodSeconds: 10 - timeoutSeconds: 5 + periodSeconds: 15 + timeoutSeconds: 10 failureThreshold: 3 # -- Worker startup probe. startup: From a43f971e740d914eeb21d3d3cbe1d4b1612e8214 Mon Sep 17 00:00:00 2001 From: PlatformSREEngineer Date: Sat, 5 Sep 2026 20:45:05 +0000 Subject: [PATCH 2/3] test(chart): guard that worker readiness is never tighter than liveness at replicas 1 (BLO-31945) Asserts the invariant whose violation caused the outage: on a single-replica worker, readiness.timeoutSeconds must be >= liveness.timeoutSeconds, because readiness has no peer to shed to and dropping the only endpoint 502s all plugin traffic. Also asserts timeoutSeconds <= periodSeconds. Scoped to replicas === 1 so it does not block tightening readiness again once the worker runs two or more replicas (BLO-29307 / BLO-29004). Verified as a real guard, not a tautology: against master's values (timeout 5) the new test fails with the intended message; with the timeout raised to 10 all three tests in the file pass. Co-Authored-By: Paperclip --- deploy/helm/paperclip/tests/probes.test.mjs | 57 +++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/deploy/helm/paperclip/tests/probes.test.mjs b/deploy/helm/paperclip/tests/probes.test.mjs index 5ac07e5b6618..544f422a1218 100644 --- a/deploy/helm/paperclip/tests/probes.test.mjs +++ b/deploy/helm/paperclip/tests/probes.test.mjs @@ -50,3 +50,60 @@ test("API deployment keeps HTTP health probes", () => { assert.match(rendered, /path: \/healthz/); assert.doesNotMatch(rendered, /grep -qa 'server\/dist\/index\.js'/); }); + +function probeSettings(rendered, probeName) { + const match = rendered.match( + new RegExp(`^(\\s*)${probeName}:\\n((?:\\1\\s+.*\\n)+)`, "m"), + ); + assert.ok(match, `${probeName} not found in rendered template`); + const block = match[2]; + const read = (key) => { + const found = block.match(new RegExp(`\\b${key}:\\s*(\\d+)`)); + assert.ok(found, `${probeName} is missing ${key}`); + return Number(found[1]); + }; + return { + periodSeconds: read("periodSeconds"), + timeoutSeconds: read("timeoutSeconds"), + failureThreshold: read("failureThreshold"), + }; +} + +// BLO-31945. A readiness probe exists to shed traffic to healthy peers. The +// worker StatefulSet runs a single replica, so there is no peer: dropping the +// only Service endpoint does not redirect plugin traffic, it 502s it with +// "Worker tier unreachable" from worker-tier-proxy.ts, taking Alertmanager +// webhook delivery down with it. Measured on paperclip-0 over 24h to +// 2026-09-05T20:30Z, the 5s readiness timeout failed at 2.53x the rate of the +// 10s liveness timeout against the identical /healthz (1.171% vs 0.462%), +// while p99.9 of successful liveness probes was 8.59s — i.e. the readiness +// timeout had been set below the endpoint's own tail latency. +test("worker readiness probe is no tighter than liveness while the tier is single-replica", () => { + const rendered = renderTemplate("templates/statefulset.yaml"); + + const replicas = Number(rendered.match(/^\s*replicas:\s*(\d+)/m)?.[1]); + assert.ok(Number.isInteger(replicas), "could not read worker replicas"); + + const readiness = probeSettings(rendered, "readinessProbe"); + const liveness = probeSettings(rendered, "livenessProbe"); + + // Deliberately scoped to replicas === 1. Once the worker runs two or more + // replicas (BLO-29307 / BLO-29004), readiness can genuinely shed load to a + // peer and tightening it again becomes a legitimate choice — this guard + // should not block that. + if (replicas === 1) { + assert.ok( + readiness.timeoutSeconds >= liveness.timeoutSeconds, + `readiness timeoutSeconds (${readiness.timeoutSeconds}s) must not be shorter than ` + + `liveness (${liveness.timeoutSeconds}s) on a single-replica worker: with no peer to ` + + `shed to, a readiness failure removes the only endpoint and 502s all plugin traffic. ` + + `See BLO-31945.`, + ); + } + + assert.ok( + readiness.timeoutSeconds <= readiness.periodSeconds, + `readiness timeoutSeconds (${readiness.timeoutSeconds}s) must not exceed periodSeconds ` + + `(${readiness.periodSeconds}s), or one probe can outlast its own period. See BLO-31945.`, + ); +}); From 818d7861625967b9bc369936e26c9a79786babf2 Mon Sep 17 00:00:00 2001 From: PlatformSREEngineer Date: Sun, 6 Sep 2026 02:37:15 +0000 Subject: [PATCH 3/3] fix(chart): close both Ally Important findings on the readiness fix (BLO-31945) Addresses the two Important findings on PR #1673 at head a43f971e. 1. README helm-docs drift (README.md:263). The generated row still advertised periodSeconds:10/timeoutSeconds:5 and still carried the rationale this PR exists to retire -- "Keep it tighter than liveness so API callers stop routing to a worker whose event loop is not answering promptly." The operator-facing artifact was instructing the next reader to undo the fix. helm-docs is not installed here, so the three changed rows were regenerated by reproducing its transformation and validating it against the untouched workerProbes.liveness row, which regenerates byte-identical. The measurement table in the values.yaml comment was reworded to prose: helm-docs joins comment lines with spaces, so an ASCII table renders as an unreadable run-on inside a markdown cell. 2. probes.test.mjs read failureThreshold but no assertion consumed it. Reviewer measured the gap: timeoutSeconds 10->5 failed the guard, but failureThreshold 3->1 passed silently -- and that is the more damaging edit, dropping the only endpoint on any single tail-latency blip rather than requiring three consecutive failures. The floor is >= 3, not the reviewer's first suggested form (>= liveness.failureThreshold): liveness is 6 and readiness is 3, so that form would fail against the configuration it is meant to protect. Verified by mutation, not by inspection: with this commit both failureThreshold 3->1 and timeoutSeconds 10->5 fail the guard, and the unmutated chart passes 3/3. Also takes both reviewer suggestions: startup's deliberate timeout exemption is now stated (failureThreshold: 30 makes tail sensitivity costless there), and the "3 x 15s = 45s" worst case is replaced with the derivation (t=0/15/30, each timing out 10s in, so ~40s+ plus propagation). Co-Authored-By: Paperclip --- deploy/helm/paperclip/README.md | 6 ++-- deploy/helm/paperclip/tests/probes.test.mjs | 29 ++++++++++++++- deploy/helm/paperclip/values.yaml | 39 +++++++++++++++------ 3 files changed, 60 insertions(+), 14 deletions(-) diff --git a/deploy/helm/paperclip/README.md b/deploy/helm/paperclip/README.md index 03078287209a..f274437e1b57 100644 --- a/deploy/helm/paperclip/README.md +++ b/deploy/helm/paperclip/README.md @@ -258,7 +258,7 @@ kubectl -n paperclip exec paperclip-0 -- \ | serviceMonitor.scrapeAllowFromNamespaces | list | `[]` | Namespaces of the Prometheus pods that scrape this Service. Appended to the NetworkPolicy ingress `from` (when `networkPolicy.enabled`) so scrapes are not blocked. Same shape as `networkPolicy.allowFromNamespaces`. Example: `[{namespaceSelector: {matchLabels: {kubernetes.io/metadata.name: monitoring}}}]`. | | serviceMonitor.scrapeTimeout | string | `"10s"` | Per-scrape timeout. Must be `<=` `interval`. | | tolerations | list | `[]` | Tolerations. | -| workerProbes | object | `{"liveness":{"failureThreshold":6,"httpGet":{"httpHeaders":[{"name":"Host","value":"127.0.0.1:3100"}],"path":"/healthz","port":"http"},"initialDelaySeconds":30,"periodSeconds":30,"timeoutSeconds":10},"readiness":{"failureThreshold":3,"httpGet":{"httpHeaders":[{"name":"Host","value":"127.0.0.1:3100"}],"path":"/healthz","port":"http"},"initialDelaySeconds":5,"periodSeconds":10,"timeoutSeconds":5},"startup":{"failureThreshold":30,"httpGet":{"httpHeaders":[{"name":"Host","value":"127.0.0.1:3100"}],"path":"/healthz","port":"http"},"periodSeconds":10,"timeoutSeconds":5}}` | Worker StatefulSet probe configuration. The worker serves the same `/healthz` endpoint as the API tier; probing HTTP prevents Kubernetes from routing API-to-worker requests before the worker is actually listening. | +| workerProbes | object | `{"liveness":{"failureThreshold":6,"httpGet":{"httpHeaders":[{"name":"Host","value":"127.0.0.1:3100"}],"path":"/healthz","port":"http"},"initialDelaySeconds":30,"periodSeconds":30,"timeoutSeconds":10},"readiness":{"failureThreshold":3,"httpGet":{"httpHeaders":[{"name":"Host","value":"127.0.0.1:3100"}],"path":"/healthz","port":"http"},"initialDelaySeconds":5,"periodSeconds":15,"timeoutSeconds":10},"startup":{"failureThreshold":30,"httpGet":{"httpHeaders":[{"name":"Host","value":"127.0.0.1:3100"}],"path":"/healthz","port":"http"},"periodSeconds":10,"timeoutSeconds":5}}` | Worker StatefulSet probe configuration. The worker serves the same `/healthz` endpoint as the API tier; probing HTTP prevents Kubernetes from routing API-to-worker requests before the worker is actually listening. | | workerProbes.liveness | object | `{"failureThreshold":6,"httpGet":{"httpHeaders":[{"name":"Host","value":"127.0.0.1:3100"}],"path":"/healthz","port":"http"},"initialDelaySeconds":30,"periodSeconds":30,"timeoutSeconds":10}` | Worker liveness probe. BLO-19722 — why this is deliberately laxer than `probes.liveness` above. What `/healthz` actually measures: nothing declares this route. No handler in `server/src` matches `/healthz` (the only one in the repo belongs to the MCP gateway), so the request falls through to the SPA catch-all at `server/src/app.ts:777-787`, which synchronously returns 200 + static HTML. It therefore probes exactly one thing — "is the event loop able to service a request right now" — and says nothing about database or heartbeat health. At the previous `timeoutSeconds: 5 / periodSeconds: 30 / failureThreshold: 3` a ~90s event-loop stall was enough for kubelet to kill the pod. That is not theoretical: on 2026-07-31 `Liveness probe failed: context deadline exceeded` on `paperclip-0` was followed ~5s later by every active `agent-opencode-*` pod dying with it. A busy heartbeat pass is not a wedged process, but this probe could not tell them apart. The asymmetry with the API tier is the point. An API replica is stateless and horizontally scaled, so killing one early is cheap. The worker is a singleton supervising every in-flight agent run, so a false kill orphans all of them — the exact failure this issue exists to stop. We therefore buy stall tolerance at the cost of slower detection of a true wedge: 6 × 30s ≈ 180s before a kill, versus ~90s previously. Not fixed here: making `/healthz` un-blockable. Node runs our request handling on one thread, so any in-process endpoint is blocked by whatever blocks the loop; a genuinely independent health responder needs a worker thread (or a sidecar) and is a larger change than this issue should carry. Widening with the rationale recorded is the other branch this issue allows. Latent landmine while we are here: because `/healthz` resolves via the SPA catch-all, a build serving no `ui-dist` (`app.ts:788-790` warns and skips the catch-all) would 404 every probe and CrashLoop the worker for a reason that has nothing to do with its health. | -| workerProbes.readiness | object | `{"failureThreshold":3,"httpGet":{"httpHeaders":[{"name":"Host","value":"127.0.0.1:3100"}],"path":"/healthz","port":"http"},"initialDelaySeconds":5,"periodSeconds":10,"timeoutSeconds":5}` | Worker readiness probe. Readiness gates the API-to-worker Service endpoint. Keep it tighter than liveness so API callers stop routing to a worker whose event loop is not answering promptly; during a long-but-recoverable stall the pod may stay alive while temporarily leaving the Service endpoints. | -| workerProbes.startup | object | `{"failureThreshold":30,"httpGet":{"httpHeaders":[{"name":"Host","value":"127.0.0.1:3100"}],"path":"/healthz","port":"http"},"periodSeconds":10,"timeoutSeconds":5}` | Worker startup probe. | +| workerProbes.readiness | object | `{"failureThreshold":3,"httpGet":{"httpHeaders":[{"name":"Host","value":"127.0.0.1:3100"}],"path":"/healthz","port":"http"},"initialDelaySeconds":5,"periodSeconds":15,"timeoutSeconds":10}` | Worker readiness probe. BLO-31945 — why this is no longer tuned "tighter than liveness". The rationale here used to read: "Keep it tighter than liveness so API callers stop routing to a worker whose event loop is not answering promptly." That holds for a horizontally-scaled tier, where readiness sheds traffic to healthy peers. This StatefulSet runs `replicas: 1`, so there is no peer to shed to: removing the only endpoint does not redirect plugin traffic, it 502s it. Every API-to-worker call then fails in the `catch` at `server/src/routes/worker-tier-proxy.ts` with "Worker tier unreachable", which is how a `/healthz` latency blip became a total plugin-tier outage — Alertmanager webhook delivery included. Alertmanager then retries each delivery 4x into the worker that just failed its probe. Measured on `paperclip-0` over the 24h to 2026-09-05T20:30Z from the kubelet `prober_probe_*` metrics — same endpoint, same pod, same window, with the per-probe timeout as the only material difference: readiness at a 5s timeout failed 100.2 of 8559 probes (1.171%), while liveness at a 10s timeout failed 13.1 of 2838 (0.462%). Readiness failed at 2.53x the liveness rate against the identical `/healthz`. The tail latency confirms the cause: p99.9 of *successful* liveness probes is 8.59s, i.e. `/healthz` legitimately takes longer than the old 5s readiness timeout. A timeout set below the endpoint's own tail is a false-positive generator, not a sensitivity knob. Trap when re-deriving this: `prober_probe_duration_seconds` is a SUCCESS-ONLY histogram — its `_count` matches `prober_probe_total{result="successful"}`, not the total. Timed-out probes are censored out of it, so the bucket distribution on its own reads as "almost nothing exceeds 5s" and will invert this diagnosis if used without the failure counter. timeoutSeconds now matches liveness (10s), above the measured tail, and periodSeconds moves to 15 so a probe cannot outlast its own period. Worst-case endpoint removal, stated derivably rather than as `3 x period`: probes start at t=0/15/30 and each times out 10s in, so the third consecutive failure is only observed at t=40s, and endpoint propagation adds more on top. That is ~40s+ versus ~20s+ under the old 10s/5s settings. The slower detection costs nothing real at `replicas: 1`, because endpoint removal has no upside here in the first place. Decision, recorded explicitly rather than left implicit: readiness is KEPT, not retired. It still gates the endpoint during rollout, and deleting it would make a genuine wedge hang API callers until TCP timeout instead of failing fast. What is retired is tuning it as though it could shed load. This is mitigation, not a cure: at the liveness failure rate the probe still drops the endpoint occasionally, and the durable fix is a second replica (BLO-29307 / BLO-29004) or a `/healthz` that cannot be blocked by the event loop. Until one of those lands, this probe must be tolerant rather than tight. `failureThreshold: 3` is load-bearing, not incidental, and it is the knob a future reader will reach for once told not to touch the timeout. At the 0.462% per-probe failure rate this endpoint actually exhibits, requiring three *consecutive* failures puts an endpoint drop at roughly one per several years; `failureThreshold: 1` would drop the only endpoint on any single tail-latency blip, i.e. tens of times a day, reopening exactly the outage this change closes. `tests/probes.test.mjs` enforces both this floor and the timeout relationship while the tier is single-replica. | +| workerProbes.startup | object | `{"failureThreshold":30,"httpGet":{"httpHeaders":[{"name":"Host","value":"127.0.0.1:3100"}],"path":"/healthz","port":"http"},"periodSeconds":10,"timeoutSeconds":5}` | Worker startup probe. Deliberately left at `timeoutSeconds: 5`, below the 8.59s `/healthz` tail measured above, and that exemption is intentional rather than an oversight: `failureThreshold: 30` at a 10s period gives a ~300s budget, so an individual timed-out probe is absorbed instead of being fatal. The readiness defect was that three false timeouts were *sufficient* to remove the endpoint; here they are not close to sufficient, so tail sensitivity carries no cost. Re-tighten `failureThreshold` here and that stops being true. | diff --git a/deploy/helm/paperclip/tests/probes.test.mjs b/deploy/helm/paperclip/tests/probes.test.mjs index 544f422a1218..301e0c3bf2a6 100644 --- a/deploy/helm/paperclip/tests/probes.test.mjs +++ b/deploy/helm/paperclip/tests/probes.test.mjs @@ -51,6 +51,10 @@ test("API deployment keeps HTTP health probes", () => { assert.doesNotMatch(rendered, /grep -qa 'server\/dist\/index\.js'/); }); +// Smallest readiness failureThreshold that still requires *consecutive* +// failures before the only Service endpoint is removed. See BLO-31945. +const MIN_READINESS_FAILURE_THRESHOLD = 3; + function probeSettings(rendered, probeName) { const match = rendered.match( new RegExp(`^(\\s*)${probeName}:\\n((?:\\1\\s+.*\\n)+)`, "m"), @@ -78,7 +82,13 @@ function probeSettings(rendered, probeName) { // 10s liveness timeout against the identical /healthz (1.171% vs 0.462%), // while p99.9 of successful liveness probes was 8.59s — i.e. the readiness // timeout had been set below the endpoint's own tail latency. -test("worker readiness probe is no tighter than liveness while the tier is single-replica", () => { +// +// Three separate knobs can each reopen that outage, so all three are asserted: +// the timeout (how easily one probe reports failure), failureThreshold (how +// many failures are needed to act on it), and the timeout/period relationship. +// Guarding only the timeout leaves failureThreshold as the next lever a reader +// reaches for once told the timeout is off limits. +test("worker readiness probe cannot be re-tightened into the BLO-31945 outage", () => { const rendered = renderTemplate("templates/statefulset.yaml"); const replicas = Number(rendered.match(/^\s*replicas:\s*(\d+)/m)?.[1]); @@ -99,6 +109,23 @@ test("worker readiness probe is no tighter than liveness while the tier is singl `shed to, a readiness failure removes the only endpoint and 502s all plugin traffic. ` + `See BLO-31945.`, ); + + // The floor is 3, not `>= liveness.failureThreshold`. Liveness is 6 and + // readiness is 3, so keying off liveness would fail against the very + // configuration this guard is meant to protect. 3 is the smallest value + // that still requires *consecutive* failures, which is the property that + // matters: at the 0.462% per-probe failure rate this endpoint exhibits, + // needing three in a row makes an endpoint drop rare, while + // failureThreshold: 1 drops the only endpoint on any single tail-latency + // blip and reopens the outage — with every other assertion here still + // green. + assert.ok( + readiness.failureThreshold >= MIN_READINESS_FAILURE_THRESHOLD, + `readiness failureThreshold (${readiness.failureThreshold}) must be at least ` + + `${MIN_READINESS_FAILURE_THRESHOLD} on a single-replica worker: a lower threshold lets ` + + `an isolated slow /healthz remove the only Service endpoint, which 502s all plugin ` + + `traffic rather than shedding it. See BLO-31945.`, + ); } assert.ok( diff --git a/deploy/helm/paperclip/values.yaml b/deploy/helm/paperclip/values.yaml index d58d7c8d7fc3..f2bc74cbcc86 100644 --- a/deploy/helm/paperclip/values.yaml +++ b/deploy/helm/paperclip/values.yaml @@ -411,15 +411,13 @@ workerProbes: # then retries each delivery 4x into the worker that just failed its probe. # # Measured on `paperclip-0` over the 24h to 2026-09-05T20:30Z from the - # kubelet `prober_probe_*` metrics. Same endpoint, same pod, same window: - # - # probe timeout failed / total failure rate - # Readiness 5s 100.2 / 8559 1.171% - # Liveness 10s 13.1 / 2838 0.462% + # kubelet `prober_probe_*` metrics — same endpoint, same pod, same window, + # with the per-probe timeout as the only material difference: readiness at a + # 5s timeout failed 100.2 of 8559 probes (1.171%), while liveness at a 10s + # timeout failed 13.1 of 2838 (0.462%). # # Readiness failed at 2.53x the liveness rate against the identical - # `/healthz`, and the per-probe timeout is the only material difference - # between them. The tail latency confirms the cause: p99.9 of *successful* + # `/healthz`. The tail latency confirms the cause: p99.9 of *successful* # liveness probes is 8.59s, i.e. `/healthz` legitimately takes longer than # the old 5s readiness timeout. A timeout set below the endpoint's own tail # is a false-positive generator, not a sensitivity knob. @@ -433,9 +431,12 @@ workerProbes: # # timeoutSeconds now matches liveness (10s), above the measured tail, and # periodSeconds moves to 15 so a probe cannot outlast its own period. - # Worst-case endpoint removal becomes 3 x 15s = 45s rather than 30s; that - # slower detection costs nothing real at `replicas: 1`, because endpoint - # removal has no upside here in the first place. + # Worst-case endpoint removal, stated derivably rather than as `3 x period`: + # probes start at t=0/15/30 and each times out 10s in, so the third + # consecutive failure is only observed at t=40s, and endpoint propagation + # adds more on top. That is ~40s+ versus ~20s+ under the old 10s/5s + # settings. The slower detection costs nothing real at `replicas: 1`, + # because endpoint removal has no upside here in the first place. # # Decision, recorded explicitly rather than left implicit: readiness is # KEPT, not retired. It still gates the endpoint during rollout, and @@ -446,6 +447,15 @@ workerProbes: # second replica (BLO-29307 / BLO-29004) or a `/healthz` that cannot be # blocked by the event loop. Until one of those lands, this probe must be # tolerant rather than tight. + # + # `failureThreshold: 3` is load-bearing, not incidental, and it is the knob + # a future reader will reach for once told not to touch the timeout. At the + # 0.462% per-probe failure rate this endpoint actually exhibits, requiring + # three *consecutive* failures puts an endpoint drop at roughly one per + # several years; `failureThreshold: 1` would drop the only endpoint on any + # single tail-latency blip, i.e. tens of times a day, reopening exactly the + # outage this change closes. `tests/probes.test.mjs` enforces both this + # floor and the timeout relationship while the tier is single-replica. readiness: httpGet: path: /healthz @@ -458,6 +468,15 @@ workerProbes: timeoutSeconds: 10 failureThreshold: 3 # -- Worker startup probe. + # + # Deliberately left at `timeoutSeconds: 5`, below the 8.59s `/healthz` tail + # measured above, and that exemption is intentional rather than an + # oversight: `failureThreshold: 30` at a 10s period gives a ~300s budget, so + # an individual timed-out probe is absorbed instead of being fatal. The + # readiness defect was that three false timeouts were *sufficient* to remove + # the endpoint; here they are not close to sufficient, so tail sensitivity + # carries no cost. Re-tighten `failureThreshold` here and that stops being + # true. startup: httpGet: path: /healthz