Re: strange perf regression with data checksums

On Wed, May 21, 2025 at 12:29 PM Andres Freund <andres@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