Re: gistGetFakeLSN() can return incorrect LSNs

Hi,

On 2026-03-05 10:52:03 -0800, Noah Misch wrote:
> On Thu, Mar 05, 2026 at 12:10:11PM -0500, Andres Freund wrote:
> > Tomas encountered a crash with the index prefetching patchset. One of the
> > patches included therein is a generalization of the gistGetFakeLSN()
> > mechanism, which is then used by other indexes as well.  That triggered an
> > occasional, hard to locally reproduce, ERROR or PANIC in CI, about
> > 
> >   ERROR: xlog flush request 0/01BD2018 is not satisfied --- flushed only to 0/01BD2000
> 
> > To be safe, this code would need to use a version of GetXLogInsertRecPtr()
> > that does use XLogBytePosToEndRecPtr() instead of XLogBytePosToRecPtr().
> 
> I agree.  Thanks for diagnosing it.  Feel free to move forward with that
> strategy, or let me know if you'd like me to do it.

I'd appreciate if you could do it.


> > However, if I put an XLogFlush() into gistGetFakeLSN() and use
> > wal_level=minimal, it's a lot easier.
> > 
> > 
> > It looks like this was introduced in
> > 
> > commit c6b92041d38
> > Author: Noah Misch <noah@leadboat.com>
> > Date:   2020-04-04 12:25:34 -0700
> > 
> >     Skip WAL for new relfilenodes, under wal_level=minimal.
> 
> Combining that XLogFlush() with wal_level=minimal (your recipe) does make
> predicate-gist.spec fail, both at that commit and today.

Good.


I've been thinking that we really need to do more verification of LSNs in a
bunch of places.

When reading in a page outside of recovery, we should probably complain about
LSNs that are not from the past, instead of reading them into the buffer pool
and then complaining when a hint bit is set on the buffer and the buffer needs
to be flushed.

When WAL logging of hint bits is enabled and we are writing out a dirty
permanent page (leaving aside the FSM), we probably should assert that the
page has an LSN that's recent enough to be plausible.

It's too bad that PageSetLSN() doesn't have a reference to the buffer, so it's
nontrivial to see if the buffer is permanent and thus should always have a
valid LSN.  I guess we could check if the page is within (BufferBlocks,
BLCKSZ*NBlocks) and only do additional verification in that case, but brrr.

Greetings,

Andres Freund