P2: Operations runbook — SLOs, capacity, incidents, failover (#180)

[dkijania]

What & why

Part of the production-readiness epic (#163). Closes #180.

There was one benchmark data point but no runbook, SLOs, or documented failure-mode response.

Adds docs/runbook.md

• SLOs — starting targets for availability, p50/p99 latency, error rate.
• What to watch — the /metrics RED signals, readiness, structured logs, and suggested alert thresholds.
• Scaling & capacity — stateless horizontal scaling, Postgres as the real ceiling, and replicas × PG_MAX_CONNECTIONS vs DB max_connections math.
• Multi-host Postgres failover — documented semantics and recovery expectations (with an honest "validate for your topology" caveat).
• Common incidents — a symptom → cause → action table.
• Deploys & rollback — tied to graceful shutdown + readiness gating.

Linked from the README. References observability/config features delivered by the sibling PRs. An automated replica-failover test is noted as a follow-up (needs a multi-host DB harness), so the doc is the deliverable here.

Testing

Docs only. prettier --debug-check . clean. No application code changed.

🤖 Generated with Claude Code

[SanabriaRusso]

Great capstone for the epic — the symptom→cause→action table and the honest "validate for your topology" failover caveat are exactly what on-call wants. One correctness heads-up before merge: almost everything the runbook tells operators to observe or tune ships in sibling PRs that aren't on main yet, and the doc presents them as current with no status note. On today's main:

• /metrics + every http_* metric → don't exist (P1: Expose Prometheus metrics at /metrics (#173) #191)
• /readiness — the 503-vs-200 signal the incident table and most alerts lean on → doesn't exist; server only serves / and /healthcheck (P1: Add readiness probe distinct from liveness (#169) #187)
• PG_STATEMENT_TIMEOUT / PG_MAX_CONNECTIONS → not wired; the client is a bare postgres(connectionString) with no pool/timeout options (P0: Configure Postgres pool limits + statement_timeout (#165) #182)
• RATE_LIMIT_MAX / X-Forwarded-For handling → not present (P0: Add per-IP request rate limiting (#166) #185)
• graceful drain on SIGTERM + terminationGracePeriodSeconds + trace flush → src/index.ts currently does server.close() then an immediate process.exit(0), no drain (P1: Graceful shutdown — drain, flush traces, uncaught handlers (#170) #188)
• the docs/security.md (P0: Add security & deployment hardening guide (#168) #186) and deploy/kubernetes.yaml (P2: Reference deployment manifests — k8s + prod Compose (#179) #196) links → those files aren't on main yet, so both 404 today

Docs PRs tend to merge ahead of feature PRs, and if this one lands first the runbook misdirects on-call mid-incident (curling /readiness → 404, "confirm PG_STATEMENT_TIMEOUT is set" → no-op, relying on a drain that doesn't happen). Two clean options: gate this merge behind those PRs, or add a short status banner up top, e.g.

Status: Some capabilities below ship in in-flight PRs and are not yet on main: /metrics (#191), /readiness (#187), Postgres pool/timeout knobs (#182), rate limiting (#185), graceful shutdown (#188), config validation (#193), plus the linked deploy/ manifests (#196) and docs/security.md (#186). Sections depending on them are marked (pending).

One smaller note on Scaling: "Add read replicas and point PG_CONN at them before scaling the API further" reads as added read capacity, but the postgres client treats multi-host PG_CONN as failover to the first reachable host, not load-balanced fan-out — so extra hosts buy redundancy, not throughput. Worth a word so operators don't expect horizontal DB read scaling from it. (Your dedicated Failover section already gets this exactly right.)

Nice work overall — just want the doc to be safe to follow the day it merges.

[dkijania]

Thanks @SanabriaRusso — fixed in b6dc43a.

The "describes unshipped behavior" problem. Agreed this was the important one: a runbook that misdirects on-call mid-incident is worse than no runbook. Of your two options I took a third — scoping by version rather than by in-flight PR numbers:

Applies to 1.0.0 and later. Much of what follows — /readiness, /metrics, PG_STATEMENT_TIMEOUT / PG_MAX_CONNECTIONS, RATE_LIMIT_MAX, and the graceful drain on SIGTERM — does not exist on 0.0.x images…

Reasoning: the merge plan already lands this doc after #182/#185/#187/#188/#191, so a PR-status banner would be stale on arrival and need deleting. The version framing stays true afterwards and is arguably the more useful warning anyway — the published image today is 0.0.6, so "is this on main?" is the wrong question for an operator mid-incident; "what am I actually running?" is the right one. Same approach applied in #186 and #196.

The Scaling claim — you were right, and I can now say why. I checked the driver rather than take it on faith: postgres.js declares hostIndex inside function Connection(...) (src/connection.js:89), so it's per-connection state reset to 0 for every pooled connection. Every connection starts at host[0] and only advances when that connection's attempt fails. Failover, not fan-out — extra hosts buy redundancy, exactly as you said. Both the Scaling bullet and the failover section now say so, and point at a balancer (PgBouncer/HAProxy/managed reader endpoint) for real read capacity.

Worth flagging that this same claim was already on main's README ("the server fans queries across them") — so it wasn't just this doc. Fixed there via #186, and in deploy/README.md via #196.

One addition from the #185 fix: I split the Many 429s incident row. Now that TRUST_PROXY bounds X-Forwarded-For trust, the most likely cause of mass 429s across unrelated clients is TRUST_PROXY being unset behind a gateway — every client collapses into one bucket. That's a different diagnosis and fix from a single client exceeding the limit, so it gets its own row (and the app now warns about it at startup).

[SanabriaRusso]

Verdict: MERGEABLE ✅

Docs-only, two new/edited files, zero runtime impact — nothing here can touch mina-explorer or mina-explorer-api. I checked the runbook's factual claims against main and the sibling PRs rather than the prose.

What I checked

• Both of @SanabriaRusso's points landed. The version banner is in at head (docs/runbook.md:6-13) and the fan-out claim is gone — Scaling now says listing replicas in PG_CONN "does not spread reads across them", with the balancer recommendation. Correct per postgres.js src/connection.js:89 (hostIndex is per-Connection, reset to 0). Note main's README.md:94 still says "the server fans queries across them" — fixed in P0: Add security & deployment hardening guide (#168) #186, so land these in either order but don't drop P0: Add security & deployment hardening guide (#168) #186.
• Every identifier resolves. /readiness (P1: Add readiness probe distinct from liveness (#169) #187 src/server/readiness.ts, unconditional, 200/503), /metrics + http_requests_total / http_request_duration_seconds / http_requests_in_flight (P1: Expose Prometheus metrics at /metrics (#173) #191), PG_STATEMENT_TIMEOUT / PG_MAX_CONNECTIONS (P0: Configure Postgres pool limits + statement_timeout (#165) #182), RATE_LIMIT_MAX / TRUST_PROXY (P0: Add per-IP request rate limiting (#166) #185) — all exact-name matches, no invented knobs. BLOCK_RANGE_SIZE and CORS_ORIGIN exist on main today (src/server/server.ts:10,24). /healthcheck is correctly used as the liveness path and correctly attributed to main (src/server/server.ts:20).
• Numbers are consistent. ~800 req/s @ p99 39 ms, 12-core/32 GB matches README.md:94 verbatim. terminationGracePeriodSeconds: 30 matches P2: Reference deployment manifests — k8s + prod Compose (#179) #196's manifest and sits above P1: Graceful shutdown — drain, flush traces, uncaught handlers (#170) #188's SHUTDOWN_TIMEOUT_MS=10000 — correct ordering. "effective global limit ≈ replicas × RATE_LIMIT_MAX" is right: P0: Add per-IP request rate limiting (#166) #185's limiter is in-process (default 600/60s per instance). No capacity figure contradicts the real consumer budget (~4 req/s at limit: 500 from mina-explorer-api), and the p99 SLO is explicitly scoped to "typical event/action query" so it doesn't over-promise on the explorer's 2000-block analytics query.
• No outage-causing failover advice. The DB-blip path says pods recover automatically "without killing the (live) pods" — exactly right given readiness is DB-dependent in P1: Add readiness probe distinct from liveness (#169) #187 and liveness is not. Nothing tells an operator to restart all replicas or drain blind.
• No secrets. The only PG_CONN example is postgres://host1:5432,host2:5432/archive, credential-free. *.gcp.o1test.net isn't referenced at all.
• No contradiction with P0: Add security & deployment hardening guide (#168) #186 / P2: Reference deployment manifests — k8s + prod Compose (#179) #196 / P2: Declare 1.0.0 + versioning & schema stability policy (#178) #198. The banner's 1.0.0 boundary matches the version P2: Declare 1.0.0 + versioning & schema stability policy (#178) #198 declares, and the failover/balancer wording matches P2: Reference deployment manifests — k8s + prod Compose (#179) #196's deploy/README.md. One exception below.

Non-blocking nits

1. Rollback claim vs P2: Reference deployment manifests — k8s + prod Compose (#179) #196's own warning. "Roll back by redeploying the previous image tag — the service is stateless and carries no migrations, so rollback is safe at any time." Today's published tag is 0.0.6, and P2: Reference deployment manifests — k8s + prod Compose (#179) #196's deploy/README.md warns that on a 0.0.x image "the readiness probe 404s forever, no pod goes Ready, and the Service ends up with no endpoints at all". So the one rollback an operator is most likely to reach for at 3am is the one that black-holes the service. Suggest: Roll back within the 1.0.x line — the service is stateless and carries no migrations. Do **not** roll back to a 0.0.x image while the readiness probe targets /readiness: it 404s, no pod goes Ready, and the Service loses all endpoints.

2. /metrics is off by default. P1: Expose Prometheus metrics at /metrics (#173) #191 ships ENABLE_METRICS=false; "What to watch" reads as if /metrics is always there. One clause fixes it: "…the Prometheus /metrics endpoint (requires ENABLE_METRICS=true; off by default)". Related and worth a ping on P2: Reference deployment manifests — k8s + prod Compose (#179) #196: its Deployment sets PORT/PG_CONN/CORS_ORIGIN/SHUTDOWN_TIMEOUT_MS/TRUST_PROXY and carries prometheus.io/path: /metrics, but never sets ENABLE_METRICS, so the reference manifest scrapes a 404. That's P2: Reference deployment manifests — k8s + prod Compose (#179) #196's fix, not this PR's.

3. The practical latency ceiling is 20s, not 30s. P0: Configure Postgres pool limits + statement_timeout (#165) #182 proposes statement_timeout 30s, but both consumers abort client-side at 20s (mina-explorer src/services/api/http.ts AbortSignal.timeout; mina-explorer-api app/upstream/archive.py httpx 20s). Anything past 20s is DB work nobody is waiting for. Optional half-sentence on the "confirm PG_STATEMENT_TIMEOUT is set" row: known consumers give up at 20s, so treat that as the real budget.

4. The "Startup exits immediately / config is validated fail-fast" row depends on P1: Validate config at startup + fix boolean env parsing (#174, #74) #193, which isn't in the banner's example list (main today just does buildContext(process.env.PG_CONN) with no validation). The banner's "Much of what follows" is broad enough to cover it; adding #193-era config validation to the enumeration would be tidier.

Automated second-pass review — focus: downstream compatibility with mina-explorer / mina-explorer-api.

[SanabriaRusso]

Approving on the basis of the second-pass review comment above: no mid-to-high severity security, compatibility, or degradation issue found, and the downstream contract with mina-explorer / mina-explorer-api holds — GraphQL validation error text reaches errors[].message verbatim, the browser SPA's cross-origin access is preserved, and the real consumer query shapes (including the 2000-block analytics query and the 500-row page crawl) still pass.

Two things this approval does not mean:

• It does not close the non-blocking items in the review comment. Several are worth fixing before or shortly after merge; they are written up there with patches.
• It does not by itself mean the branch is ready to merge. main requires branches to be up to date, so this needs an update-branch (or a rebase, if the branch is conflicting) first, and a few PRs in this series have cross-PR ordering constraints called out in their review comments.

Automated second-pass review — focus: downstream compatibility with mina-explorer / mina-explorer-api.

[SanabriaRusso]

Round 1's blocker is properly fixed and every name in the runbook is real. Approving.

Verified fixed

• Rollback wording (docs/runbook.md:106-109) now names the version floor concretely: "Roll back within the 1.0.x line… Do not roll back to a 0.0.x image while readiness targets /readiness: that path 404s, no pod goes Ready, and the Service loses all endpoints." That is exactly the failure mode, stated so an on-call engineer can act on it. The scoping banner at :9-15 reinforces it.
• Every metric, log field, and endpoint exists. Checked against the sibling diffs rather than assumed: http_requests_total, http_request_duration_seconds, http_requests_in_flight are the literal name: values registered in #191 and asserted by its tests; requestId and durationMs are real fields emitted by #190's useRequestLogging; /readiness is #187; PG_STATEMENT_TIMEOUT, PG_MAX_CONNECTIONS, RATE_LIMIT_MAX, BLOCK_RANGE_SIZE and the fail-fast config validation are all real. Nothing invented — which is the bar for a runbook.
• Worth calling out: the http_requests_in_flight saturation alert at :58 is now trustworthy. Round 1 found that gauge drifted permanently negative; #191 fixed it and pinned it with a test asserting the exact exposition line http_requests_in_flight 0 after a healthcheck + preflight + POST sequence. Alerting on it would have been actively misleading a round ago.
• SLOs are consistent with the README benchmark (targets p99 < 250 ms / p50 < 50 ms against a measured p99 of 39 ms) and with #185: :63-65 correctly states the limiter is per-instance, so the effective global ceiling ≈ replicas × RATE_LIMIT_MAX. No capacity claim exceeds the shipped limit.
• TRUST_PROXY hop count at :95 (GCP external ALB = 2) matches #186 docs/security.md:98 and #185's implementation (forwarded[forwarded.length - trustProxy]). All three agree.

Non-blocking nits

1. :95 — wrong diagnosis for "Many 429s." The row blames "TRUST_PROXY unset or too low." Under #185 as shipped, unset means trustProxyConfigured === false and useRateLimit() returns a no-op plugin — rate limiting is disabled, so unset yields zero 429s, not many. Also the warning fires on the first proxied request, not at startup. Suggested replacement for the cause/action cells:

   Cause: TRUST_PROXY set too low behind a gateway, so every client shares the proxy's bucket
   Action: set TRUST_PROXY to the exact hop count (a GCP external ALB needs 2); the app logs a warning on the first proxied request when TRUST_PROXY=0 but traffic carries X-Forwarded-For. Note that leaving TRUST_PROXY unset disables rate limiting entirely — that presents as no 429s, not many.

   I have blocked #186 on the same underlying misconception (docs/security.md:57 states TRUST_PROXY defaults to 0, and :75 lists rate limiting as Default on; neither is true of what #185 ships). Fixing all three together is the efficient move — #196 deploy/README.md:46 is the third.

2. :103 contradicts the manifest it links to. It says terminationGracePeriodSeconds: 30; #196's deploy/kubernetes.yaml:131 sets 45, with a 15 s preStop sleep in front and SHUTDOWN_TIMEOUT_MS: 10000. Suggested:

   • Rolling update: a 15 s preStop sleep lets endpoint removal propagate, then the app drains in-flight requests within SHUTDOWN_TIMEOUT_MS (10 s in the reference manifest) inside terminationGracePeriodSeconds: 45.

3. State RATE_LIMIT_MAX's default in the capacity section. :96 says "adjust RATE_LIMIT_MAX" without saying from what. The shipped default is 600 per 60 s per client IP. Worth noting alongside it that a known consumer backfilling at ~4 rps already sits at ~40% of that budget from a single IP — that is exactly the sizing decision this section should be driving.

4. Worth adding to the incident section, since it is the sharpest downstream failure mode and it is not obvious: mina-explorer-api does not retry 4xx, and it maps a 429 to a circuit-breaker failure (app/upstream/graphql.py). A sustained 429 burst therefore drives that consumer's per-endpoint breaker OPEN, after which it short-circuits every call with zero I/O for the cooldown. So over-tight rate limiting on this service presents downstream as a step function — the archive tier goes dark — rather than as gradual throttling. One line under "Many 429s" would save someone a confusing incident.

5. Merge ordering: this PR's README hunk carries the old "point PG_CONN at multiple read replicas — the server fans queries across them" sentence as unchanged context, while #186, #196 and #198 each rewrite that same paragraph to the corrected failover-not-fan-out wording. Whichever lands second will conflict there; keep the corrected text.

Downstream: none, docs-only. :87 correctly encodes the consumers' 20 s client timeout as the practical latency ceiling, which is the right operational framing now that #182 lowered statement_timeout to 15 s — queries in the 15–20 s band flip from slow-but-successful to a hard error, and this is the document where an operator would look for that.