From 26d179c3f09212f1e01fc0c19f9f4a2764c6a742 Mon Sep 17 00:00:00 2001 From: Bretton Date: Sat, 8 Aug 2026 02:33:59 -0700 Subject: [PATCH] fix(e2e): make the tier poll cadence fit inside the limiter budget MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit contractPollInterval 250ms → 600ms so 100/min buckets outlast the 45s contractBudget on EVERY wait (75 polls < 100): four contracts measured 23.9-24.7s against the 25s cliff on the last gate run, and the posts lane legitimately runs long now that it carries four collections. The premise that justified 250ms — healthy waits never approach the budget — was measured false twice (task 4's comment contract, task 5's cluster). Per-wait comment-contract override folded into the default. Co-Authored-By: Claude Fable 5 --- tests/e2e/comment_contract_test.go | 16 +++------- tests/e2e/contracts_test.go | 51 ++++++++++++++++-------------- 2 files changed, 33 insertions(+), 34 deletions(-) diff --git a/tests/e2e/comment_contract_test.go b/tests/e2e/comment_contract_test.go index bfa39e0..c19806c 100644 --- a/tests/e2e/comment_contract_test.go +++ b/tests/e2e/comment_contract_test.go @@ -273,23 +273,17 @@ func indexedPost(t *testing.T, p *pipeline, community provisionedCommunity, auth postRecord(community.DID, authorDID, title, "a post to hang comments on")) uri := postURI(community.DID, rkey) - // 600ms, not the default 250ms: this wait legitimately runs LONG. Under a - // full-suite `make ci` the posts consumer is draining every parallel - // contract's records at once, and this parent-post index was measured at - // 23.97s on a QUIET stack — right at the ~25s cliff where 250ms polling - // exhausts the global 100/minute bucket and the wait dies as a 429 with - // 20 seconds of budget still unspent (observed 5 consecutive CI runs, - // 2026-08-08). At 600ms the full 45s contractBudget fits inside the - // bucket (75 polls < 100), so the wait fails on the budget or not at all — - // which is what contractPollInterval's own doc says a 429 here should - // mean. Discovery latency on a healthy fast run costs ~350ms extra. + // This wait was the first to hit the 250ms-era limiter cliff (measured + // 23.97s healthy latency; five consecutive gate deaths) and carried its + // own 600ms override until task 5 made that the tier default — see + // contractPollInterval's HISTORY note. p.Await(t, "the post these comments hang off to be indexed", func() (bool, error) { view, err := p.Post(context.Background(), uri) if err != nil { return false, err } return !view.NotFound, nil - }, testkit.WithPollInterval(600*time.Millisecond)) + }) return strongRef{URI: uri, CID: record.CID} } diff --git a/tests/e2e/contracts_test.go b/tests/e2e/contracts_test.go index 8df55d7..5c0a0ae 100644 --- a/tests/e2e/contracts_test.go +++ b/tests/e2e/contracts_test.go @@ -326,29 +326,34 @@ const contractHoldWindow = 5 * time.Second // contractPollInterval is how often a T2 wait re-asks the serving endpoint. // -// Slower than testkit's 100ms default ON PURPOSE, and the reason is the rate -// limiter described in the package doc. Every poll is a request against a -// 100-per-minute budget, so the interval and contractBudget are a pair: -// -// 45s budget ÷ 250ms = 180 polls if a wait runs its FULL length -// -// which is over the 100 a bucket allows. That is deliberate rather than -// overlooked, because of what the two cases cost: -// -// - A wait that SUCCEEDS costs one or two polls. The pipeline delivers in -// well under a second on this stack, and WaitFor probes before it sleeps, -// so a healthy contract never approaches the budget. This is every poll the -// tier issues on a green run. -// - A wait that FAILS was going to fail anyway. Past roughly 25 seconds it -// starts collecting 429s instead of "not yet" — so Await translates that -// status into a message saying so, rather than letting a rate limit -// masquerade as a broken endpoint. -// -// Buying the difference would mean either a 1s interval (adding half a second -// to every wait in the tier for the benefit of runs that are already red) or -// raising the AppView's limit in .env.ci — which would delete the one signal -// that a polling storm is happening at all. Neither trade is worth it. -const contractPollInterval = 250 * time.Millisecond +// Slower than testkit's 100ms default ON PURPOSE, and the interval and +// contractBudget are a pair against the 100-per-minute limiter described in +// the package doc: +// +// 45s budget ÷ 600ms = 75 polls if a wait runs its FULL length +// +// which fits inside the 100 a bucket allows, so a wait always fails on the +// BUDGET (a real timeout with consumer health attached), never on the +// limiter. That invariant is what the 429-explainer in Await promises, and +// it was not always true here. +// +// HISTORY — this was 250ms, on the stated premise that "the pipeline +// delivers in well under a second, so a healthy contract never approaches +// the budget," making the limiter cliff (100 polls ≈ 25s) unreachable for +// green runs. The premise was measured false twice as the suite grew: +// task 4 found the comment contract's parent-post wait at 23.97s healthy +// latency under full-suite load (five consecutive cliff deaths), and task 5 +// measured FOUR contracts clustered at 23.9-24.7s against the 25s wall — +// one scheduling hiccup from red on every gate run. Under `make ci` the +// posts lane legitimately runs tens of seconds behind (it carries four +// collections, and inline dead-letter retries block it by design), so long +// waits are healthy, not hopeless. 600ms buys the full budget for every +// wait at a cost of ~350ms average extra discovery latency on fast runs. +// +// Do NOT "fix" a marginal wait by raising the AppView's limit in .env.ci — +// that would delete the one signal that a genuine polling storm is +// happening at all. +const contractPollInterval = 600 * time.Millisecond // contractHoldPollInterval is how often a Holds re-asks. It is deliberately // four times slower than contractPollInterval, and the reason is arithmetic the -- 2.51.2