fix(coordinator): totals cache misses no longer queue behind the refresher - #1158
Open
brandon-eigenlabs wants to merge 1 commit into
Open
brandon-eigenlabs wants to merge 1 commit into
brandon-eigenlabs wants to merge 1 commit into
Conversation
A /v1/network/totals request that missed the read cache first waited, unbounded, for any compute already in flight for its window, then blocked on the shared queryMu behind the refresher's other windows, then ran its own 10 s attempt. During the September 2026 outage that produced 20-40 s client latencies in multiples of the store timeout for what was always a 503, and every external poll added another scan of provider_earnings while the refresher was already failing. Bound the request-path wait on an in-flight compute to coldFillWait (2 s) while the refreshers keep waiting for the flight, and have the request path TryLock the query bound: if the refresher holds it the compute returns errComputeBusy, which the cache layer treats as "serve what is cached" without logging or counting a refresh failure. Requests therefore answer from the cache or 503 within 2 s and never start an aggregate concurrently with the refresher. The 5 minute safety TTL is unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
brandon-eigenlabs
requested a deployment
to
benchmarks
September 21, 2026 01:49 — with
GitHub Actions
Waiting
|
@brandon-eigenlabs is attempting to deploy a commit to the EigenLabs Team on Vercel. A member of the Team first needs to authorize it. |
This branch is waiting to be deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A
/v1/network/totalsrequest that missed the read cache waited, unbounded, for any compute already in flight for its window, then blocked on the sharedqueryMubehind the refresher's other windows, then ran its own 10 s attempt. During the September outage that produced 20–40 s client latencies (multiples of the store timeout) for what was always going to be a 503, and every external poll added anotherprovider_earningsscan while the refresher was already failing. This bounds the request-path wait on an in-flight compute to 2 s and makes the request pathTryLockthe query bound: if the refresher holds it, the request answers from the cache or 503s at once and never starts an aggregate. The 5-minute safety TTL and the refresher's own behavior are unchanged.Linked issue
Closes #1155
Test plan
cd coordinator && gofmt -l ./api— clean;go build ./...— ok;go vet ./api/— cleancd coordinator && go test ./api/ -run 'Totals|Cache|Stats|Summary' -count=1— passcd coordinator && go test $(go list ./... | grep -v /internal/api) -count=1— the pre-push set, passmake docs-stamp FILES=docs/reference/api-contracts.md;make docs-impact-check BASE=origin/master— coverage OK;make docs-check— 311 files OKTests (
coordinator/api/cache_concurrency_test.go):TestTotalsColdRequestsDoNotQueueBehindBackgroundRefreshreplacesTestTotalsColdWindowsSerializeWithBackgroundRefresh, which encoded the old contract (cold requests wait behind the refresher and all return 200). With the refresher's first window held open: a direct request-path compute returnserrComputeBusy; cold requests for all four windows return 503 withincoldFillWait+ 1 s; the store saw exactly one call (requests started no aggregate). After the refresher finishes all windows the same requests are 200 cache hits; peak query concurrency stays 1; a failing compute still releases the bound. Fails without the fix (requests block on the held query and return 200 only after it releases).TestGetCachedEntryBoundsWaitOnInflightCompute— a request finding an open flight returns(nil, false)after ~coldFillWaitwithout computing; a refresher finding the same flight keeps waiting and returns once it closes.Components touched
Protocol / interface changes
No new status codes or shapes. The observable change is latency: a miss during an in-flight refresh now resolves within 2 s (cache body or the existing 503 envelope) instead of after the refresher's and its own store timeouts.
docs/reference/api-contracts.mdstates this on the/v1/network/totalsrow and in the cache-behavior paragraph./v1/statssharescomputeCachedEntry, so its miss path gets the same bounded wait (it has no separate query lock; unchanged otherwise).Documentation impact
docs/reference/api-contracts.md(/v1/network/totalsrow; cache-behavior paragraph namescoldFillWait), stamp refreshed;CHANGELOG.mdbullet under Unreleased.Notes for reviewers
Before / after — observable behavior
sequenceDiagram participant C as client (cache miss) participant H as handleNetworkTotals participant F as in-flight refresh (window A) participant Q as queryMu (held by refresher) participant DB as provider_earnings Note over C,DB: BEFORE C->>H: GET ?window=B H->>Q: Lock() … waits for refresher's window (≤10 s) Q-->>H: acquired H->>DB: own aggregate (≤10 s) DB--xH: timeout H-->>C: 503 after 20–40 s; one extra scan addedsequenceDiagram participant C as client (cache miss) participant H as handleNetworkTotals participant F as in-flight refresh participant Q as queryMu (held by refresher) Note over C,Q: AFTER C->>H: GET ?window=A (flight open) H->>F: wait ≤ coldFillWait (2 s) H-->>C: cached body, or 503, within 2 s C->>H: GET ?window=B (no flight) H->>Q: TryLock() Q-->>H: busy → errComputeBusy (not logged, not counted) H-->>C: cached body, or 503, immediatelyCode flow
flowchart TD R[request miss] --> G["getCachedEntry (refresh=false)"] G -->|flight open| W["select: flight done / coldFillWait"] --> C1[readCache.Get → body or 503] G -->|no flight| K["computeNetworkTotals(window, waitForQuery=false)"] K -->|"TryLock ok"| DB[aggregate → Set → 200] K -->|"TryLock busy"| B[errComputeBusy] --> C1 BG[refresher] --> G2["refreshCachedEntry (refresh=true)"] -->|flight open| W2["<-wait (unbounded, as before)"] G2 -->|no flight| K2["computeNetworkTotals(window, true) → Lock()"]Design choices
errComputeBusyis not a refresh failure.computeCachedEntryreturns the cached value for it without thecache refresh failedlog or thecache.refresh_failedcounter, so request traffic cannot inflate the monitor observability: Datadog monitor on cache.refresh_failed as code #1157 adds.updated_at. That is a product decision about how old a number the dashboard may show as current; this PR only removes the queueing and the request-driven load, and keeps the documented contract.coldFillWaitis a packagevaronly so the tests can shorten it; it is not configurable.Limitations
/v1/network/totalsmisses resolving in ≤2 s (curl -w %{time_total}) instead of 20–40 s, and no request-drivenNetworkTotalsqueries inpg_stat_activitywhile the refresher is running.v0.3.6/v0.4.0with zero items).🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.