Skip to content

test(integration): give saturation followers a post-load catch-up window - #600

Merged
bdchatham merged 2 commits into
mainfrom
devin/1791339968-saturation-follower-catchup
Oct 7, 2026
Merged

bdchatham merged 2 commits into
mainfrom
devin/1791339968-saturation-follower-catchup

Conversation

@bdchatham

Copy link
Copy Markdown
Collaborator

Summary

TestNightlyBenchmark/saturation has failed in every nightly since #574 because of assertChainLive's follower-lag check, not because of a stall:

post-load nightly-dly4qli7p38p-saturation-rpc-1 at height 1983 trails the validator head 2120 by 137 blocks (> 10)

At unlimited send rate, the receipt follower (rpc-1) applies blocks at about 91% of the validators' rate (Prometheus tendermint_consensus_height over the 10m load). It falls behind for the whole load, and the gap was measured once, the moment seiload exits. Harbor Prometheus shows a saturation follower lag of 98–201 blocks in every run from Oct 2 to Oct 6. Until now this was hidden behind the release-suite vesting crashloop (#598). With that fixed, it is the only failing suite in the nightly: Chaos, GigaStoreMigration, GigaMixedRelease, Release, ChainUpgrade and WorkflowStateSync all pass. Steady-state followers stay within the threshold.

Change: after the validators-advance check, assertChainLive re-reads the validator head and every follower's height every followerCatchUpPoll (10s). It returns as soon as all followers are within followerMaxLag (10). It fails only if a follower is still behind after followerCatchUpTimeout (5m), or if the scenario ctx ends.

deadline := now + followerCatchUpTimeout
loop:
    head := validator height
    lagging := followers with head-h > followerMaxLag
    if none: return
    if past deadline: t.Errorf(each lagging) ; return
    sleep followerCatchUpPoll (or ctx done -> t.Errorf)

A follower that has really stalled still fails, because it can't close the gap. The 5m budget fits inside the scenario timeout (durationMin + 80m).

Not verified: a full nightly run on this change. In the failing run the chain was torn down right after the check, so I couldn't observe the follower catching up after load. Its block rate under load suggests it closes a ~140-block gap well within 5m once load stops. After merge, integration-harness in platform clusters/harbor/nightly/harness/cronjobs.yaml needs bumping to this merge SHA.

Validation: go vet -tags integration ./test/integration/, gofmt, and golangci-lint run --build-tags integration --new-from-merge-base=origin/main ./test/integration/... are clean.

Link to Devin session: https://app.devin.ai/sessions/046d3f7315594005af4d7040d641c18f
Open in Devin Desktop: https://app.devin.ai/desktop/session/046d3f7315594005af4d7040d641c18f?variant=devin
Requested by: @bdchatham

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor

I'll fix CI failures and address comments from users with write access that start with 'Devin'.

  • Disable automatic comment, CI, and merge conflict monitoring

@cursor

cursor Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

PR Summary

Low Risk
Integration test-only change to follower lag timing; no production or runtime behavior.

Overview
Post-load chain liveness no longer fails the instant a follower is behind the validator head after seiload finishes. assertChainLive still requires validators to advance first, then polls every 10s until every RPC follower is within 10 blocks of the head or 5 minutes elapse.

Followers that were slow under saturation load (e.g. receipt RPC trailing by ~100+ blocks at job exit) can pass once they catch up; a follower that never closes the gap within the timeout still fails with an explicit “after 5m” error. On success, validators are checked again for continued block production.

Reviewed by Cursor Bugbot for commit 8a64e6c. 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.

This change makes assertChainLive poll for up to 5 minutes, every 10s, until every follower is back within followerMaxLag of the validator head, where before it checked the gap once. Nothing blocks: a follower that has really stalled still fails, and the 5m wait fits inside the scenario timeout. The one finding is a suggestion: validator liveness is only checked before the wait (codex's lead, confirmed). The pr-600-tree merge ref was available and reviewed; no scout reading was dropped.

Non-blocking

1 finding on the changed lines, as inline comments.

1 nit, not posted on the code
  • test/integration/seiload_test.go:268 — When ctx ends partway through the loop, mustLatestHeight usually notices first. It fails its height reads and calls t.Fatalf with "endpoint unreachable", so this branch rarely runs, the real cause is misreported, and assertSeiloadRun is skipped. Checking ctx.Err() at the top of each iteration would report the actual cause.

seidroid review · decision approve · session cf42ea0d251640dcb28b7c53a3c82886 · turn resp_claude_881115f69113aeec4f3d24911e1a0427 · item 2f3eec14934d592cbaf9a3e31730bc06

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

Comment thread test/integration/seiload_test.go
…; report ctx end directly

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@bdchatham
bdchatham merged commit 84fafe6 into main Oct 7, 2026
4 checks passed
@devin-ai-integration

Copy link
Copy Markdown
Contributor

On the ctx nit: fixed in 8a64e6c. Each loop iteration now checks ctx.Err() before reading heights, so an ended scenario reports its real cause through t.Errorf instead of the "endpoint unreachable" t.Fatalf.

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