Repository navigation
[release/3.4] fix(history): reset page counter on recsplit collision retry in buildVI - #19697
Merged
Merged
Conversation
The counter 'i' used to track page offsets in paged history files was declared outside the retry loop. When a recsplit collision occurred and the loop retried, 'i' retained its value from the previous iteration, causing incorrect page offset tracking in the .vi index. Fix: Move 'i := 0' inside the retry loop so it's reset on each attempt.
AskAlexSharov
approved these changes
Mar 6, 2026
Giulio2002
pushed a commit
that referenced
this pull request
Mar 6, 2026
UnitTest for #19697 - maybe can simplify in future (by passing external recsplit cfg)
sudeepdino008
added a commit
that referenced
this pull request
Mar 9, 2026
The variable 'keyPos' used to track key offsets in accessor files was declared outside the retry loop. When a recsplit collision occurred and the loop retried, 'keyPos' retained its value from the previous iteration, causing incorrect key offset tracking in the index. Fix: Move keyPos initialization inside the retry loop so it's reset on each attempt. Similar to the fix in history.go (PR #19697).
sudeepdino008
added a commit
that referenced
this pull request
Mar 9, 2026
…ain.go Same bug pattern as PR #19697 (history.go buildVI page counter). When buildHashMapAccessor encounters a recsplit collision and retries, keyPos and valPos must be reset to 0. Previously they were declared outside the retry loop, causing incorrect offsets in the .kvi index on retry. Also cherry-picks test infrastructure from release/3.4 (PR #19698): - ForceCollisionOnce() method in recsplit for testing - _testBuildVIHook in history.go - TestHistoryBuildVI_PageCounterResetOnCollisionRetry Adds TestDomain_KeyPosResetOnCollisionRetry to verify the fix.
sudeepdino008
pushed a commit
that referenced
this pull request
Mar 9, 2026
UnitTest for #19697 - maybe can simplify in future (by passing external recsplit cfg)
AskAlexSharov
pushed a commit
that referenced
this pull request
Mar 10, 2026
…ain.go (#19752) ## Summary Same bug pattern as PR #19697 (history.go buildVI page counter). When `buildHashMapAccessor` encounters a recsplit collision and retries, `keyPos` and `valPos` must be reset to 0. Previously they were declared outside the retry loop, causing incorrect offsets in the .kvi index on retry. ## Changes - **db/state/domain.go**: Move `keyPos` and `valPos` declarations inside the retry loop - **db/state/domain_test.go**: Add `TestDomain_KeyPosResetOnCollisionRetry` to verify the fix - Cherry-pick test infrastructure from release/3.4 (PR #19698): - `ForceCollisionOnce()` method in recsplit for testing - `_testBuildVIHook` in history.go - `TestHistoryBuildVI_PageCounterResetOnCollisionRetry` ## Related - PR #19697: Fixed same bug in history.go buildVI - PR #19698: Test infrastructure for collision retry testing (release/3.4)
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.
Cherry-pick of #19695 to release/3.4
The counter 'i' used to track page offsets in paged history files was declared outside the retry loop. When a recsplit collision occurred and the loop retried, 'i' retained its value from the previous iteration, causing incorrect page offset tracking in the .vi index.
Fix: Move
i := 0inside the retry loop so it's reset on each attempt.