On Wed, May 21, 2025 at 12:29 PM Andres Freund <and...@anarazel.de> wrote: > > The relevant nbtree code seems more elegant if we avoid calling > > BufferGetLSNAtomic() unless and until its return value might actually > > be useful. It's quite a lot easier to understand the true purpose of > > so->currPos.lsn with this new structure. > > I'm not against that - ISTM we should do both.
Agreed. Long term, we should move all of this stuff out of index AMs, which don't have any good reason for doing their own thing. In a sense, the known bugs in GiST and SP-GiST index-only scans (the "must not drop the index page pin during index-only scans to avoid unsafe concurrent TID recycling" bugs) exist because those index AMs couldn't just opt in to some generic scheme, used by all index AMs that support amgettuple-based scans. Technically, the LSN is only needed to make kill_prior_tuple LP_DEAD bit setting safe when the page pin is dropped. But it still seems related to those GiST + SP-GiST IOS bugs, since we also need to consider when dropping the pin is categorically unsafe (only nbtree gets that aspect right currently). -- Peter Geoghegan
