Remove budget-based query capping; propose against natural ceiling - #1380
Conversation
…ync/SVM maxNumLogs getNextQuery indexed the shared buffer at a budget-derived position to cap how far each query could range — with 100+ partitions this forced many tiny queries per tick even though checkAndFetch's cross-chain admission loop already gates aggregate volume by estResponseSize. Dropping the ceiling lets each chain propose against its natural ceiling (head/endBlock/mergeBlock), producing fewer, larger requests without changing what gets admitted. The remaining risk was calculateEstResponseSize's flat defaultEstResponseSize (10,000) for a partition with no query history, now paired with an unbounded range instead of one the old ceiling had already truncated. HyperSync/SVM already expose max_num_logs/max_num_instructions (wired through the Rust napi layer, unused until now) as a server-enforced soft cap — pass the query's own estResponseSize through as maxNumItems so a wrong estimate truncates the response instead of overshooting the buffer, on every query rather than just first-fetches. RPC keeps its own independent AIMD interval and never relied on this ceiling; Fuel and Simulate ignore the new parameter. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TPtu2dmeaji6DNdLfUHLuf
… to 5,000 Halves both the indexer-wide buffer pool default (CrossChainState.calculateTargetBufferSize) and the flat estimate used for a partition with no query history yet (FetchState.defaultEstResponseSize), scaling down memory pressure now that getNextQuery no longer caps a query's range against the buffer. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TPtu2dmeaji6DNdLfUHLuf
"Max" overstated it as a hard ceiling; HyperSync/SVM only treat it as a soft target they try not to overshoot by much. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TPtu2dmeaji6DNdLfUHLuf
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR removes explicit shared-budget parameters from FetchState/ChainState/CrossChainState query-generation flows, shifting query sizing to use knownHeight and lowering default response-size/buffer values. It introduces an itemsTarget parameter across Source implementations, with HyperSync-backed sources enforcing it server-side, and updates related tests. ChangesBudgetless query generation and itemsTarget capping
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CrossChainState
participant ChainState
participant FetchState
participant SourceManager
participant Source
CrossChainState->>ChainState: getNextQuery(cs)
ChainState->>FetchState: getNextQuery(fetchState)
FetchState-->>ChainState: candidate query bounded by endBlock/knownHeight
ChainState-->>CrossChainState: candidate query
CrossChainState->>CrossChainState: pool/admit candidates against shared budget
CrossChainState->>SourceManager: dispatch admitted query
SourceManager->>SourceManager: itemsTarget = ceil(query.estResponseSize)
SourceManager->>Source: getItemsOrThrow(itemsTarget)
Source-->>SourceManager: items capped by itemsTarget
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scenarios/test_codegen/test/helpers/MockIndexer.res (1)
730-741: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider capturing
itemsTargetin the call payload for future test assertions.Currently discarded via
as _. SinceitemsTargetis now a meaningful soft-cap signal computed upstream inSourceManager.executeQuery, capturing it (similar tofromBlock/toBlock) would let future tests assert on the computed value without further mock changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scenarios/test_codegen/test/helpers/MockIndexer.res` around lines 730 - 741, The MockIndexer.getItemsOrThrow test helper is discarding itemsTarget, which prevents future assertions on the soft-cap value coming from SourceManager.executeQuery. Update the mock payload handling in getItemsOrThrow to capture itemsTarget instead of ignoring it, following the same pattern used for fromBlock and toBlock, so tests can inspect the computed value without changing the mock again.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@scenarios/test_codegen/test/helpers/MockIndexer.res`:
- Around line 730-741: The MockIndexer.getItemsOrThrow test helper is discarding
itemsTarget, which prevents future assertions on the soft-cap value coming from
SourceManager.executeQuery. Update the mock payload handling in getItemsOrThrow
to capture itemsTarget instead of ignoring it, following the same pattern used
for fromBlock and toBlock, so tests can inspect the computed value without
changing the mock again.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b31e0034-566f-41f1-809e-322754c2a43d
📒 Files selected for processing (24)
packages/envio/src/ChainState.respackages/envio/src/ChainState.resipackages/envio/src/CrossChainState.respackages/envio/src/FetchState.respackages/envio/src/sources/HyperFuelSource.respackages/envio/src/sources/HyperSync.respackages/envio/src/sources/HyperSync.resipackages/envio/src/sources/HyperSyncSource.respackages/envio/src/sources/RpcSource.respackages/envio/src/sources/SimulateSource.respackages/envio/src/sources/Source.respackages/envio/src/sources/SourceManager.respackages/envio/src/sources/Svm.respackages/envio/src/sources/SvmHyperSyncSource.resscenarios/test_codegen/test/ClientAddressFilter_test.resscenarios/test_codegen/test/EventBlockFilter_test.resscenarios/test_codegen/test/HyperSync_test.resscenarios/test_codegen/test/RateLimit_test.resscenarios/test_codegen/test/RpcSource_test.resscenarios/test_codegen/test/SourceBlockHashes_test.resscenarios/test_codegen/test/SvmHyperSyncSource_test.resscenarios/test_codegen/test/helpers/MockIndexer.resscenarios/test_codegen/test/lib_tests/FetchState_test.resscenarios/test_codegen/test/lib_tests/SourceManager_test.res
calculateDefaultEstResponseSize(~partitionsCount) replaces the flat defaultEstResponseSize constant: 20_000 / partitionsCount, clamped to [2_000, 10_000]. With many partitions sharing the same buffer pool, assuming every zero-history partition's first query is "big" starves the rest of that tick's admission — so the per-partition default shrinks as partition count grows, letting more first-queries admit concurrently while still capping each one's actual response server-side via itemsTarget. SourceManager's own confirmed-zero-density safety net now falls back to FetchState.minEstResponseSize (the 2_000 floor) instead of the old flat constant. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TPtu2dmeaji6DNdLfUHLuf
Surfaces each admitted query's estResponseSize (the itemsTarget derived from it downstream) alongside the existing fromBlock/targetBlock, rounded for readable log output. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TPtu2dmeaji6DNdLfUHLuf
Summary
Removes the per-chain budget mechanism from query proposal, simplifying
FetchState.getNextQueryto propose queries against their natural ceiling (head block, endBlock, or mergeBlock) without buffer-size constraints. Admission against the shared buffer budget now happens exclusively inCrossChainState, which pools candidate queries from all chains and admits them in priority order until the budget is consumed.Key Changes
FetchState.getNextQuery: Removed
~budgetand~chainPendingBudgetparameters; no longer caps queries based on buffer fill. Queries are now proposed unconstrained by the shared buffer state.Query sizing: Reduced
defaultEstResponseSizefrom 10,000 to 5,000 items to better reflect typical partition response sizes and improve admission accuracy.Buffer admission: Moved all budget enforcement to
CrossChainState.checkAndFetch, which now:ChainState.getNextQuery(simplified signature)itemsTargetpassed to sources (HyperSync enforces server-side; RPC/Fuel/Simulate ignore it)Default buffer target: Reduced from 100,000 to 50,000 items in
CrossChainState.calculateTargetBufferSize.Source interface: Added
~itemsTarget: intparameter toSource.t.getItemsOrThrowto communicate the soft cap from the admission loop to backends that support it (HyperSync viamaxNumLogs).Test updates: Removed budget-related test cases and assertions; updated expected
estResponseSizevalues throughout to reflect the new default of 5,000.Implementation Details
maxQueryBlockNumbercalculation that previously capped queries based on buffer position is removed entirely;knownHeightis used directly where needed.SourceManager.executeQuerynow ceils theestResponseSizeestimate before passing it asitemsTargetto avoid rounding sparse partitions down to 0.CrossChainState.https://claude.ai/code/session_01TPtu2dmeaji6DNdLfUHLuf
Summary by CodeRabbit
New Features
Bug Fixes