Skip to content

feat(health): serve healthz and readyz - #72

Open
bdchatham wants to merge 1 commit into
brandon2/amm-swap-scenariofrom
brandon2/health-endpoints
Open

feat(health): serve healthz and readyz#72
bdchatham wants to merge 1 commit into
brandon2/amm-swap-scenariofrom
brandon2/health-endpoints

Conversation

@bdchatham

Copy link
Copy Markdown
Contributor

Stacked on #71. Base is brandon2/amm-swap-scenario, so this branch carries the
AMM scenario, the ERC721 gas fix and these endpoints together, and the image it
builds is the one to deploy.

Why

sei-load serves /metrics and nothing else. A deployment therefore has no way
to tell a run that is still starting from one that is stuck, and a pod counts as
available the moment its container starts.

That blocks the load-generator deployment in the platform repo, whose pod spec
has a startupProbe, a readinessProbe and a livenessProbe pointing at
/readyz and /healthz. Against the current binary every one of them fails.

What the two endpoints mean

/healthz answers as soon as the HTTP server binds. It never reads the startup
sequence.

/readyz refuses until the dispatcher is running, and while it refuses it names
the phase.

Keeping them separate is the point rather than a detail. Funding, deployment and
prewarm take minutes against a cold chain. A liveness probe that reported the run
dead for that window would restart the pod before it sent a transaction, then
restart the next attempt at the same place. The run would never happen, and the
cause would read as a crash loop rather than a slow start.

Measured against the binary, polling both endpoints across a real startup:

t+3s    healthz=200   readyz=prewarming accounts [503]
t+6s    healthz=200   readyz=prewarming accounts [503]
...
t+18s   healthz=200   readyz=prewarming accounts [503]
t+21s   healthz=200   readyz=running [200]
SIGTERM -> exit 0

The phase in the body is there for the operator watching that window. A
ten-minute startup that answers only 503 says nothing about which step is slow.

One design note

The ready flag and the phase are stored as a single value, not as two atomics.
Two would leave a window where a writer has set the flag but not yet the phase,
so a reader sees the run serving while the body still names the step it left.
The status line and the body would then disagree about the same instant.

Phases

phase reported while
starting before the run reaches its first step
deploying contracts the generator deploys what the profile names
funding accounts the funder pays the account pool
prewarming accounts prewarm sends one transaction per account
running ready
shutting down after SIGTERM, through the post-summary scrape hold

shutting down drops readiness while /healthz keeps answering. The run holds
the pod open on purpose for that scrape window, and a liveness probe that failed
during it would kill the process before its final metrics were read.

Verification

Five mutations, five caught:

  • liveness made to wait for startup
  • readiness made to always pass
  • readiness made to stop naming the phase
  • NotReady made a no-op
  • the single stored value split into two atomics — a reader observed a serving
    status carrying funding accounts

The fifth is worth naming. The first version of that guard passed against the
split-atomics mutation, so its failure message claimed something it could not
detect. It was rewritten to widen the window before it was believed.

gofmt, go vet and golangci-lint run are clean. The full suite passes, and
the health package passes under -race.

🤖 Generated with Claude Code

A deployment has no way to tell a run that is starting from one that is stuck.
The process serves /metrics and nothing else, so a probe set has nothing to
gate on and a pod counts as available the moment its container starts.

/healthz answers as soon as the server binds and never reads the startup
sequence. /readyz refuses until the dispatcher is running.

Keeping those separate is the whole point. Funding, deployment and prewarm take
minutes against a cold chain. A liveness probe that reported the run dead for
that window would restart the pod before it sent a transaction, then restart the
next attempt at the same place, and the cause would read as a crash loop rather
than a slow start.

While /readyz refuses it names the phase, so a ten-minute startup shows the step
it is on. Measured against the binary: healthz held 200 through a 21 second
prewarm while readyz reported "prewarming accounts", then both answered once the
dispatcher started.

The flag and the phase are stored as one value rather than as two atomics. Two
would leave a window where a reader sees the run serving while the body still
names the step it left, so the status and the body would disagree about the same
instant.

Five mutations, five caught, including that one: split into two atomics, a
reader observed a serving status carrying "funding accounts".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@cursor

cursor Bot commented Aug 28, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Additive probe endpoints and startup-phase signaling; no changes to load generation, funding, or metrics semantics beyond when the pod is considered ready for traffic.

Overview
Adds a health package and wires /healthz and /readyz on the same HTTP server as /metrics, so deployments can use startup, readiness, and liveness probes instead of treating the pod as ready as soon as the container starts.

/healthz always returns 200 once the server is up and does not depend on startup work (avoids kubelet restarts during long contract deploy, funding, and prewarm). /readyz stays 503 until the run calls Ready(), with the response body naming the current phase (starting, deploying contracts, funding accounts, prewarming accounts, then running). Ready state and phase are stored in one atomic snapshot so status and body cannot disagree under concurrent updates.

main creates probes early, registers them on the mux, advances phases through generator build, funding, and prewarm, flips ready when sender and dispatcher are running, and on SIGTERM calls NotReady("shutting down") so traffic stops while liveness stays OK through the post-summary metrics hold.

Reviewed by Cursor Bugbot for commit 137675a. Bugbot is set up for automated code reviews on this repo. Configure here.

@seidroid seidroid Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adds a small, well-tested health package serving /healthz and /readyz on the existing metrics mux, with the ready flag and phase stored as one atomic value. The separation of liveness from readiness is correct for a run with a minutes-long startup; the only gap is that readiness is dropped on the signal path only, so a duration-bounded or error exit leaves /readyz reporting running through the shutdown/flush window.

Findings: 0 blocking | 2 non-blocking | 1 posted inline

Blockers

  • None at the file/PR level.

Non-blocking

  • [suggestion] Probes.Enter and Probes.NotReady have identical bodies; the distinction is documentation-only. Consider having NotReady delegate to Enter (or drop one) so the two cannot drift apart.
  • 1 suggestion(s)/nit(s) flagged inline on specific lines.

Comment thread main.go
// Out of service, still alive. The run holds the pod open for the
// post-summary scrape window, and /healthz keeps answering through it so
// the kubelet does not read that hold as a hang.
probes.NotReady("shutting down")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[suggestion] NotReady only runs on the signal path. When --duration expires (or a background worker fails), utils.Recv returns ctx.Err() at line 396 and returns early, so this line never executes. The run then proceeds through LogFinalStats, EmitRunSummary and the PostSummaryFlushDelay sleep (25s by default) while /readyz still answers 200 running — exactly the window readiness is meant to cover, and duration-bounded runs are the common deployment shape. Registering it once for every exit after Ready() covers all paths, e.g. defer probes.NotReady("shutting down") placed right after probes.Ready() (line 371), keeping the log line where it is.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 137675a. Configure here.

Comment thread main.go
// Out of service, still alive. The run holds the pod open for the
// post-summary scrape window, and /healthz keeps answering through it so
// the kubelet does not read that hold as a hang.
probes.NotReady("shutting down")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Readiness stuck after non-signal stop

Medium Severity

NotReady runs only when Recv gets a signal. A --duration timeout or a worker failure after Ready leaves /readyz at 200 running through the post-summary scrape hold, so the pod still looks serving after the dispatcher has already stopped.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 137675a. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant