Implement cold-start chunk probing for faster query range heuristic - #1337
Conversation
A single partition could not saturate its concurrency budget because each scheduling round only ever produced 2 tail chunks. Emit up to 3 chunks per range, and when no queries are in flight ahead of the range, size the first two at 0.9x the history range so their responses return quickly and refresh the chunking heuristic before committing to full-size chunks. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V2oe4wzvNM4P2RZEVFhDww
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthrough
ChangesAdaptive chunk probe heuristic
Sequence Diagram(s)sequenceDiagram
participant Client
participant pushQueriesForRange
participant queryLoop
participant BlockRange
Client->>pushQueriesForRange: invoke with chunkRange, maxBlock
pushQueriesForRange->>pushQueriesForRange: probeSize = 0.9 * chunkRange
pushQueriesForRange->>queryLoop: emit probe chunk 1
queryLoop->>BlockRange: create query with probe size
BlockRange-->>queryLoop: chunk 1 query
queryLoop->>queryLoop: emit probe chunk 2
queryLoop->>BlockRange: create query with probe size
BlockRange-->>queryLoop: chunk 2 query
queryLoop->>queryLoop: loop: emit full-size chunks
loop Full-size chunk iteration
queryLoop->>BlockRange: check if next chunk fits within maxBlock
alt fits
queryLoop->>BlockRange: create full-size chunk query
BlockRange-->>queryLoop: chunk query
else exceeds maxBlock
queryLoop->>queryLoop: break loop
end
end
queryLoop-->>pushQueriesForRange: all chunk queries emitted
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Adjust the chunk-boundary, growth/shrink, out-of-order, and partition-merge assertions to the new cold-start probe sizing (two 0.9x chunks then full 1.8x) and up-to-3 chunks per range. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V2oe4wzvNM4P2RZEVFhDww
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scenarios/test_codegen/test/rollback/Rollback_test.res (1)
2503-2532:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTrigger the reorg on the post-118 call instead of the first pending call.
Line 2505 intentionally leaves
chunk2(110-112) in flight, so the laterresolveGetItemsOrThrow(..., ~resolveAt=#first)can resolve that earlier pending call while injectingprevRangeLastBlock=118. That pairs an impossible previous-block value with the 110-112 query and makes the rollback path depend on mock queue ordering. Resolve the specific pending call whose range starts after 118, or resolvechunk2first and update the expected gap.🤖 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/rollback/Rollback_test.res` around lines 2503 - 2532, The issue is that sourceMock.resolveGetItemsOrThrow with ~resolveAt=`#first` is resolving the first pending call in the queue (chunk2 with range 110-112) but pairing it with an impossible prevRangeLastBlock of 118. Instead of using ~resolveAt=`#first` to resolve the first pending call, find and resolve the specific pending call whose fromBlock range starts after 118 (similar to how the continuationCall was found earlier in the test by matching call.payload["fromBlock"] == 116), or alternatively resolve chunk2 explicitly first and update the test expectations accordingly. This ensures the reorg checkpoint is paired with the correct query based on block ranges rather than mock queue ordering.
🤖 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.
Inline comments:
In `@scenarios/test_codegen/test/rollback/Rollback_test.res`:
- Around line 2395-2402: The test uses Array.some to verify the gap-fill query
exists for the 116-118 range in partition "0", but Array.some only checks for
presence of at least one matching query and cannot detect duplicates. Replace
the Array.some call that checks for the fromBlock == 116 && toBlock == Some(118)
condition with a filter operation that counts exact matches, then verify the
length equals 1 to ensure the gap-fill query is unique and not duplicated across
partition "0".
---
Outside diff comments:
In `@scenarios/test_codegen/test/rollback/Rollback_test.res`:
- Around line 2503-2532: The issue is that sourceMock.resolveGetItemsOrThrow
with ~resolveAt=`#first` is resolving the first pending call in the queue (chunk2
with range 110-112) but pairing it with an impossible prevRangeLastBlock of 118.
Instead of using ~resolveAt=`#first` to resolve the first pending call, find and
resolve the specific pending call whose fromBlock range starts after 118
(similar to how the continuationCall was found earlier in the test by matching
call.payload["fromBlock"] == 116), or alternatively resolve chunk2 explicitly
first and update the test expectations accordingly. This ensures the reorg
checkpoint is paired with the correct query based on block ranges rather than
mock queue ordering.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 139035c0-39e1-49bd-9dba-72b549ee08f2
📒 Files selected for processing (4)
packages/envio/src/FetchState.resscenarios/test_codegen/test/E2E_test.resscenarios/test_codegen/test/lib_tests/FetchState_test.resscenarios/test_codegen/test/rollback/Rollback_test.res
- Assert the gap-fill query is unique (exactly one 116-118 in partition 0) instead of merely present, restoring the no-duplicate-queries guard. - Trigger the reorg on the post-118 tail query so prevRangeLastBlock=118 is paired with its real parent block rather than an unrelated in-flight call. - Drop an unused chunk binding. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V2oe4wzvNM4P2RZEVFhDww
Emit two 0.9-size probe chunks followed by three full-size chunks on every range, dropping the hasEarlierPending conditional. This keeps a single partition's in-flight depth closer to what the pre-refactor twice-triggered fetch produced, while still leading with fast probes that refresh the chunking heuristic. Update unit, E2E, and rollback test expectations for the new chunk counts and boundaries. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V2oe4wzvNM4P2RZEVFhDww
Summary
Implements a cold-start probing strategy for chunked queries to quickly refresh the query range heuristic before committing to full-size chunks. When no earlier queries are in flight, the first two chunks use a smaller 0.9x probe size instead of the full 1.8x size, allowing their responses to return faster and inform better chunking decisions.
Key Changes
FetchState.res: Modified
pushQueriesForRangeto accept ahasEarlierPendingparameter that controls chunk sizing:hasEarlierPending=false(cold start), first two chunks use 0.9x probe sizehasEarlierPendingbased on whether pending queries existTest updates: Updated test expectations across multiple test files to reflect the new chunking behavior:
Implementation Details
The change introduces a heuristic optimization: when starting fresh (no in-flight queries ahead), smaller probe chunks allow responses to arrive quickly, enabling the system to measure actual query performance and adjust chunking strategy before committing to larger chunks. This is particularly beneficial for cold starts where the query range heuristic may be inaccurate.
The
hasEarlierPendingflag is computed at call sites:hasEarlierPending=pqIdx.contents > 0hasEarlierPending=p.mutPendingQueries->Array.length > 0https://claude.ai/code/session_01V2oe4wzvNM4P2RZEVFhDww
Summary by CodeRabbit
Bug Fixes
Tests