Skip to content

iterateChangedRecent: skip DB entries within file range#20361

Merged
AskAlexSharov merged 3 commits intomainfrom
sudeep/icr_db_files
Apr 6, 2026
Merged

iterateChangedRecent: skip DB entries within file range#20361
AskAlexSharov merged 3 commits intomainfrom
sudeep/icr_db_files

Conversation

@sudeepdino008
Copy link
Copy Markdown
Member

Closes #20356

  • Adjust fromTxNum for the DB iterator in iterateChangedRecent to max(fromTxNum, files.EndTxNum()), matching the existing pattern in HistoryKeyTxNumRange
  • Prevents reading prunable DB entries that are already covered by segment files
  • Add TestHistory_IterateChangedRecent_SkipsFileRange: builds files, prunes DB entries within the file range, verifies HistoryRange spanning both files and DB still returns correct results

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes an inconsistency in HistoryRange when the requested range spans both frozen segment files and the DB by ensuring the DB iterator does not start inside the file-covered txNum range (which may be pruned).

Changes:

  • Adjust iterateChangedRecent to start the DB scan at max(fromTxNum, files.EndTxNum()), aligning behavior with HistoryKeyTxNumRange.
  • Add a regression test that prunes DB entries within the file range and verifies HistoryRange still returns complete results across the file/DB boundary.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
db/state/history.go Skips DB iteration for txNums already covered by frozen files by raising the DB iterator start txNum to files.EndTxNum().
db/state/history_test.go Adds a regression test that prunes DB entries inside the file-covered range and validates HistoryRange output remains correct.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread db/state/history_test.go Outdated
Comment thread db/state/history_test.go

// Prune DB entries [0, filesEnd) — these are covered by files.
rwTx, err := db2.BeginRw(ctx)
require.NoError(err)
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The write transaction from db2.BeginRw(ctx) isn’t guarded by defer rwTx.Rollback(). If any require.* fails before the commit, the tx can remain open and leak resources / affect later test cleanup. Add a rollback defer immediately after BeginRw (Commit will make it a no-op).

Suggested change
require.NoError(err)
require.NoError(err)
defer rwTx.Rollback()

Copilot uses AI. Check for mistakes.
@AskAlexSharov AskAlexSharov added this pull request to the merge queue Apr 6, 2026
Merged via the queue into main with commit a2e641c Apr 6, 2026
35 checks passed
@AskAlexSharov AskAlexSharov deleted the sudeep/icr_db_files branch April 6, 2026 14:12
github-merge-queue Bot pushed a commit that referenced this pull request Apr 11, 2026
…st (#20444)

Fixes #20355

- Skip DB entries whose step falls within the file range (`step.ToTxNum
< files.EndTxNum()`) in both the initial DB cursor push and the advance
loop — they are never loaded into the heap
- Matches the "don't query DB for data in files" pattern from #20360,
#20361, #20362
- Add `TestDomain_IteratePrefix_PrefersFilesOverDB` that simulates
partial lexicographic prune and verifies file values win

Follow-up: #20474 applies the same fix to
`DomainLatestIterFile.initCursorMDBX()` and `advanceInFiles()`

## Test plan
- [x] New test passes with fix, fails without it
- [x] `make lint` clean
- [x] `make erigon integration` builds
sudeepdino008 added a commit that referenced this pull request Apr 13, 2026
Closes #20356

- Adjust `fromTxNum` for the DB iterator in `iterateChangedRecent` to
`max(fromTxNum, files.EndTxNum())`, matching the existing pattern in
`HistoryKeyTxNumRange`
- Prevents reading prunable DB entries that are already covered by
segment files
- Add `TestHistory_IterateChangedRecent_SkipsFileRange`: builds files,
prunes DB entries within the file range, verifies `HistoryRange`
spanning both files and DB still returns correct results
sudeepdino008 added a commit that referenced this pull request Apr 13, 2026
Closes #20356

- Adjust `fromTxNum` for the DB iterator in `iterateChangedRecent` to
`max(fromTxNum, files.EndTxNum())`, matching the existing pattern in
`HistoryKeyTxNumRange`
- Prevents reading prunable DB entries that are already covered by
segment files
- Add `TestHistory_IterateChangedRecent_SkipsFileRange`: builds files,
prunes DB entries within the file range, verifies `HistoryRange`
spanning both files and DB still returns correct results
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.

iterateChangedRecent: DB query reads data within file range (prunable data)

3 participants