Repository navigation
fix(rerank): take the buffer share lock around heap item reads - #381
Open
pavanmanishd wants to merge 1 commit into
Open
pavanmanishd wants to merge 1 commit into
pavanmanishd wants to merge 1 commit into
Conversation
pavanmanishd
force-pushed
the
fix/rerank-page-lock
branch
6 times, most recently
from
October 20, 2026 08:45
079eb7e to
5e44e90
Compare
The read-stream rerank read heap tuples from a pinned buffer without a content lock, checking only ItemIdIsNormal. A pin keeps the page from being evicted, not from being changed: VACUUM can truncate the line-pointer array and PageAddItem can reuse a slot, so a TID can outlive its line pointer. Hold BUFFER_LOCK_SHARE for the line-pointer read and the tuple header it points at, check the offset the way heap_fetch does, then release the lock before the slot store, detoast and distance. The pin keeps the tuple body valid, and visibility is unchanged by design to match the non-stream path. Cost: an interleaved A/B over the same tree with only the lock calls removed puts the rerank roughly 7% higher at LIMIT 5000 with the full pool.
pavanmanishd
force-pushed
the
fix/rerank-page-lock
branch
from
October 20, 2026 09:00
5e44e90 to
64fd225
Compare
pavanmanishd
marked this pull request as ready for review
October 20, 2026 09:36
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
1 participant
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.
Fixes #117.
The read-stream rerank read heap tuples from a pinned buffer without a content lock, checking only
ItemIdIsNormal. A pin keeps a page from being evicted, not from being changed: VACUUM can truncate the line-pointer array andPageAddItemcan reuse a slot, so a TID can outlive its line pointer.The fix holds
BUFFER_LOCK_SHAREfor the line-pointer read and the tuple header it points at, checks the offset the wayheap_fetchdoes, then releases the lock before the slot store, the detoast and the distance. The buffer stays pinned for the whole loop, so the tuple body is still valid unlocked; visibility is unchanged by design, matching the non-stream path. The lock costs roughly 7% of the query; a 24-client load cannot resolve a cost that small, so the single-query A/B is the honest number.