Skip to content

Fix buffer budget accounting to measure past ready frontier - #1364

Merged
DZakh merged 2 commits into
mainfrom
claude/queries-target-underfetch-7m5nmb
Jun 30, 2026
Merged

Fix buffer budget accounting to measure past ready frontier#1364
DZakh merged 2 commits into
mainfrom
claude/queries-target-underfetch-7m5nmb

Conversation

@DZakh

@DZakh DZakh commented Jun 30, 2026

Copy link
Copy Markdown
Member

Summary

Fix a buffer budget accounting bug where the ready prefix was being subtracted twice, causing the buffer to cap at a fraction of its target size. The budget should measure items past the ready frontier (processable items), not from the start of the buffer.

Changes

  • Moved bufferReadyCount function earlier in FetchState.res to make it available before getNextQuery, which now uses it to correctly offset the budget calculation
  • Updated budget calculation in getNextQuery to add bufferReadyCount when indexing into the buffer, ensuring the budget measures items past the ready frontier rather than from the buffer start
  • Updated test expectations to reflect the corrected behavior:
    • A partition behind the head can now fetch when budget allows, since ready items no longer consume the budget
    • Tight budgets no longer artificially cap queries when the buffer has no items past the frontier
    • Budget accounting now correctly allows the buffer to reach its target size

Implementation Details

The buffer is kept sorted by block number. The ready frontier is the highest block number where all partitions have data (no gaps). Items at or below this frontier are processable immediately.

Previously, maxQueryBlockNumber was calculated as buffer[budget + pending - 1], which treated the budget as an absolute index. This meant ready items (which should always be processable) were consuming budget slots, leaving less room for buffering ahead.

Now it's calculated as buffer[bufferReadyCount + budget + pending - 1], correctly measuring the budget as "how many items past the ready frontier to fetch" rather than "absolute position in buffer". This ensures:

  • Ready items don't consume budget
  • The buffer can grow to its intended size
  • Partitions aren't artificially starved when they have room to fetch

https://claude.ai/code/session_01N5UqJXYVuTRrYxwGygeW4S

Summary by CodeRabbit

  • Bug Fixes

    • Adjusted fetch budgeting so already-ready buffered items are counted correctly when deciding the next query.
    • Tight-budget cases now behave more consistently during partition head and merge continuation flows.
    • Improved handling of “fetch ahead” limits so they no longer undercount available work at the ready frontier.
  • Tests

    • Updated coverage to reflect the revised budget behavior in several query-planning scenarios.

The queries' target block (maxQueryBlockNumber) indexed the buffer at
`budget + chainPendingBudget - 1`. But `budget` already subtracts
totalReadyCount (the ready items in every chain's buffer), and the buffer
is then indexed from position 0 — so this chain's ready prefix is charged
twice. The target block landed bufferReadyCount items too early, capping
the buffer at a fraction of its target (≈50% in the caught-up regime) and
stalling fetching even with budget left.

Offset the index by bufferReadyCount so budget is measured past the ready
frontier. Move bufferReadyCount above getNextQuery to satisfy ordering.

Update the FetchState tests that pinned the old under-fetch behavior.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N5UqJXYVuTRrYxwGygeW4S
@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d48182bd-ba07-4805-a681-a22799c46bf5

📥 Commits

Reviewing files that changed from the base of the PR and between 2d34499 and 6a4e1f7.

📒 Files selected for processing (2)
  • packages/envio/src/FetchState.res
  • scenarios/test_codegen/test/lib_tests/FetchState_test.res

📝 Walkthrough

Walkthrough

bufferReadyCount is relocated earlier in FetchState.res so it can be used in getNextQuery, where maxQueryBlockNumber indexing now adds bufferReadyCount to the budget offset, preventing already-ready buffered items from being double-subtracted. Three test expectations are updated to match the corrected budgeting semantics.

FetchState budgeting fix

Layer / File(s) Summary
bufferReadyCount relocation and getNextQuery fix
packages/envio/src/FetchState.res
bufferReadyCount helper moved to line 587 (binary-searches buffer up to the ready frontier); old definition at line 1782 removed. getNextQuery updated to index the buffer with bufferReadyCount + budget + chainPendingBudget - 1 so the ready prefix is not subtracted twice from the fetch-ahead cap.
Test expectation updates
scenarios/test_codegen/test/lib_tests/FetchState_test.res
Three test cases updated: head-behind tight-budget now expects Ready instead of NothingToQuery; merge-continuation tight-budget now expects the same result as direct getNextQuery; wildcard scenario tightened to budget=1 with NothingToQuery still expected.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • enviodev/hyperindex#1336: Directly introduces/modifies bufferReadyCount and getNextQuery block-budgeting in FetchState.res with matching test updates.
  • enviodev/hyperindex#1338: Modifies getNextQuery budget/maxQueryBlockNumber accounting in FetchState.res with budget-sensitive test updates.
  • enviodev/hyperindex#1341: Exposes bufferReadyCount and getNextQuery readiness via ChainState, directly related to the ready-frontier accounting changed here.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: correcting buffer budget accounting relative to the ready frontier.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

@DZakh
DZakh enabled auto-merge (squash) June 30, 2026 13:46
@DZakh
DZakh merged commit 3432af3 into main Jun 30, 2026
8 checks passed
@DZakh
DZakh deleted the claude/queries-target-underfetch-7m5nmb branch June 30, 2026 13:47
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.

2 participants