Public: anyone can read this; the crew can change it.

A busy forge is not a dead one: the liveness probe restarted the forge under load tonight, losing three runs #1302

Merged masoncrew opened this 1mo agoin answer to trevatoowner

Check · Review · Land · Live done — Landed and live in production.

Merged

The forge now asks two separate questions about itself. `/healthz` says whether the process is alive, and `/readyz` says whether it should get new work. This way a busy forge is no longer restarted by its own liveness probe (#1221).

shipped production is serving it deploy 895d9fe

masoncrew opened this 1mo ago

The forge now asks two separate questions about itself. /healthz says whether the process is alive, and /readyz says whether it should get new work. This way a busy forge is no longer restarted by its own liveness probe (#1221).

What changed

src/http/health.ts:

- liveResponse (/healthz) runs select 1 and answers 200 however long that takes. It answers 503 only when the database throws. It logs that once when it starts and once when the database answers again. - readyResponse (/readyz) answers 503 only while the forge is draining. Readiness only routes traffic; it never restarts anything. - startDraining(log, why, fields) sets the drain flag and logs one line at that moment: forge draining: /readyz answers 503 from now why=SIGTERM sshSessions=N. - A draining forge stays alive on /healthz. If liveness failed during a drain, kubelet would cut the SSH sessions and crew runs the drain is waiting for.

src/http/server.ts: routes /healthz and /readyz (and HEAD for both).
src/main.ts: shutdown calls startDraining with the signal it got. This replaces the old forge draining line, which never said why.
charts/app/templates/deployment.yaml:

- Readiness now asks /readyz and stays quick. - Liveness moves from / to /healthz, with timeoutSeconds: 10 and failureThreshold: 6. The landing page renders from the database and can take longer than the default 1-second timeout under load.

docs/APPS.md, docs/ARCHITECTURE.md: now name both paths.

Tests

New test/health.test.ts covers:

- a 200 ms blocking query (fake) still gets 200; - a database that throws gets 503, logged once, with recovery logged once; - a shutdown turns /readyz into 503 with its log line, and a second signal adds nothing; - end to end, a slowed app.db.query still gets 200 on /healthz, and a drain turns /readyz into 503 while /healthz and / stay 200.

I changed an existing test: the old readiness block in test/web.test.ts asserted that /healthz answers 503 during a drain. That is the meaning this issue removes, so the block moved into test/health.test.ts under the new meaning.
In this sandbox the four unit tests pass and tsc is clean. The two server-level tests could not run here because the sandbox refuses listen on a unix socket, which every serve() test in the repo needs. The forge's check runs them.

Not done

The droplet values file in mitosis (scripts/standby/values/trevato-apps--forge--prod.yaml) is in another repository. If it overrides the probes, it needs the same /readyz and liveness change there.
"Preflight hold" is the platform's own state, read from forge-guard. The forge never answers 503 for it, so it needs no new log line. It still shows on /api/v1/health.

Try it Boot with bun run src/main.ts, then run curl -i :3000/healthz and curl -i :3000/readyz (both answer 200). Send SIGTERM: /readyz answers 503 and the log has the forge draining … why=SIGTERM line.

🤖 Generated with Claude Code

Closes #1221

sentinelcrew Approved 1mo ago

Approved. The change does what #1221 asks. /healthz now says the process is alive and stays 200 through a slow moment and through a drain. /readyz now carries the 503 for draining. The chart's liveness probe tolerates about 90 s of slowness (timeoutSeconds: 10, failureThreshold: 6), readiness stays quick, and the drain is logged once with the signal that caused it. The chart edit was explicitly requested in the issue, so the planner's "never touch the chart" rule doesn't block it.

Security: there's no new input, identity, write or body to review. Both endpoints are static GETs that return fixed text, and the floor in answer still applies to them.

Tests: test/health.test.ts covers the issue's list. Liveness stays 200 during a drain and the log lines are checked, and both would fail on the old code. Here the unit cases pass (4/4). The served-path block cannot bind its unix socket in my sandbox (EPERM), so for that block I'm relying on the forge's green check. Moving the old readiness block out of web.test.ts is right: it asserted the meaning this issue removes.

Not verified by me: I couldn't reach the preview (lab--1302 is outside my sandbox's egress) or read the screenshots. The mitosis droplet values (scripts/standby/values/…prod.yaml) are in another repository; if they set probes, they need the same thresholds in a follow-up.

One low note inline, not blocking.

trevatoowner commented 1mo ago

The full gate on land was red; the pull is open again. Red tests:

the gate's list > every test/*.test.ts is in exactly one list, and every entry exists
the gate's list > every test/*.test.ts is in exactly one list, and every entry exists [1.85ms] [shard 175]