Skip to content

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

Merged
bdchatham merged 5 commits into
mainfrom
brandon2/health-endpoints
Aug 29, 2026
Merged

feat(health): serve healthz and readyz#72
bdchatham merged 5 commits into
mainfrom
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

bdchatham and others added 3 commits August 27, 2026 20:25
The scenario declared 22460 gas for a mint. Measured against the deployed
binding, a mint to a receiver holding none of the token needs 69319, and one to
a receiver that already holds some needs 51757. Every mint the scenario sent
landed in a block with a failed status, having burned the whole limit, and
trackReceipts defaults to false so the run reported each one as sent.

22460 is ERC20Noop's constant, copied. PLT-1091 covers the two scenarios that
still carry it.

The limit is now 75000, and the test pins it against the measurement rather than
against itself. Broke the constant back to 22460 and to 200000 on purpose; the
test caught both.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A DeFi profile had no contract to drive. This adds a constant-product pair with
the storage and gas shape of a UniswapV2 swap: both reserves, the caller's
balance in each token, and an event.

The contract never reverts on bookkeeping, which is the choice StorageRWv1
already makes. The balances wrap rather than check, because nothing reads them
back and a load generator that fails on its own accounting stops measuring the
chain. A short caller is not credited: crediting exactly what is then debited
returns the slot to zero, and a zero to non-zero storage write costs four times
one that changes a slot already holding a value. Under the default mix, which
draws one direction, that write would land on every swap rather than the first.

The reserves sit between a floor and a ceiling. Without the ceiling the input
side grows without bound and the output halves every 100000 swaps, so a long run
prices nothing like its start. The ceiling is also what keeps one oversized call
from ending the pair: a swap of 1e49 leaves the input reserve at 1e49, and the
contract has no owner and no reset. Measured, the next ordinary swap instead
resets that side to the floor and pays out in full.

The gas limit is 85000, read from eth_estimateGas rather than from a receipt.
GasUsed is the post-refund charge and a transaction carries the pre-refund peak;
sizing from a receipt put an earlier draft 20% under what its own swap needed.
An account's first swap needs 79988 and every later one needs 45177, so a run in
steady state declares about 44% more gas than it spends. PLT-1093 carries the
prewarm change that would close that. PLT-1092 carries the chain-parameter
exposure, which is the whole package rather than this constant.

Every guard here was broken on purpose before it was believed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
Operational probe endpoints on the metrics port with no auth or transaction-path changes; incorrect wiring could affect pod scheduling but behavior is heavily tested.

Overview
Adds a health package that exposes /healthz and /readyz on the existing metrics HTTP server so Kubernetes startup, readiness, and liveness probes can succeed without treating long cold-chain startup as a crash.

/healthz always returns 200 while the process is listening and does not consult startup state. /readyz returns 503 with a plain-text phase (starting, deploying contracts, funding accounts, prewarming accounts) until the run calls Ready(), then 200 with running. NotReady("shutting down") is deferred when the load test is up so traffic stops during the post-summary scrape hold while liveness stays OK.

State is a single atomic pointer pairing the ready flag and phase so probe responses cannot disagree. main.go creates probes before the metrics server starts, registers them on the mux, advances phases through generator deploy, funding, and prewarm, marks ready when the dispatcher starts, and drops readiness on every exit path.

Tests lock in the split between liveness and readiness, phase bodies, shutdown behavior, and concurrent consistency (including race-sensitive storage).

Reviewed by Cursor Bugbot for commit d555a26. 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 Outdated
// 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 Outdated
bdchatham and others added 2 commits August 28, 2026 20:26
#71 merged squashed, so its content reached main under a new commit and this
branch's history no longer shares it. Without this merge the pull request diffs
the AMM scenario again alongside the health endpoints.
NotReady ran on the signal path alone. A run whose duration expired, or whose
background worker failed, returned early and never reached it, so readiness
stayed true through the whole shutdown: the final statistics, the run summary,
and the post-summary hold that keeps the pod open for a last scrape. That hold
defaults to 25 seconds, and it is exactly the window readiness exists to cover.

Deferring it right after Ready covers every path out by construction rather than
by remembering to call it at each return.

Measured against the same duration-bounded run, before and after. Before,
/readyz answered "running" for all fourteen seconds, through the deadline at five
and the ten-second hold after it. After, it flips to "shutting down" at the
deadline. /healthz answers 200 throughout in both, which is what keeps the
kubelet from reading a deliberate hold as a hang.

The guard here is structural rather than a test: a defer at the top of the scope
covers every return, and this package has no harness that drives the run's
lifecycle. The measurement above is what stands in for one.

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

Copy link
Copy Markdown
Contributor Author

Correct, and confirmed by running it rather than by reading. Fixed in d555a26.

NotReady ran on the signal path alone, so a duration-bounded run returned early at utils.Recv and never reached it. Readiness then stayed true through the final statistics, the run summary, and the post-summary hold — which defaults to 25 seconds and is exactly the window readiness exists to cover.

Took your suggested shape: defer probes.NotReady("shutting down") immediately after Ready(). That covers every return by construction rather than by remembering to call it at each one.

Measured on the same duration-bounded run, before and after:

                 t+1..4      t+5 (deadline)    t+6..14 (scrape hold)
  pre-fix        running     running           running        <- never drops
  post-fix       running     shutting down     shutting down

/healthz answered 200 throughout in both, which is the half that has to keep working so the kubelet does not read a deliberate hold as a hang.

One thing worth stating plainly: the guard here is structural, not a test. A defer at the top of the scope covers every return path by construction, and this package has no harness that drives the run's lifecycle — main_test.go holds two unit tests and nothing that starts a run. The before/after measurement above is what stands in for one. If that trade is wrong I would rather add the harness than leave a claim resting on a comment.

@bdchatham
bdchatham merged commit 0a2df89 into main Aug 29, 2026
9 checks passed
@bdchatham
bdchatham deleted the brandon2/health-endpoints branch August 29, 2026 03:35
bdchatham added a commit that referenced this pull request Aug 29, 2026
#71 and #72 both merged squashed, so their content reached main under new
commits and this branch's history no longer shares it. Git therefore saw the
scenario files as added on both sides.

Resolved toward this branch for AMM.go, AMM_test.go and ERC721.go, which carry
the same contracts with their hard-coded gas constants replaced by the measured
path — main holds the earlier form. Removed ERC721_test.go, which pinned a
constant this branch deletes and could not compile against it.

Took main's deferred NotReady in main.go. That fix landed in #72 after this
branch was cut, and it covers every exit rather than the signal path alone.
bdchatham added a commit that referenced this pull request Aug 29, 2026
The conflict resolution took main's deferred call but git had already
auto-merged this branch's inline one from a region that did not conflict, so
the signal path called NotReady twice. Harmless, and the opposite of what #72
did: it replaced the inline call precisely because it covered only that path.
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