Hi,
On 2026-03-05 23:26:30 +0500, Andrey Borodin wrote:
> Interesting bug. Your analysis seems correct to me.
>
> > On 5 Mar 2026, at 22:10, Andres Freund <[email protected]> wrote:
> >
> > To be safe, this code would need to use a version of GetXLogInsertRecPtr()
> > that does use XLogBytePosToEndRecPtr() instead of XLogBytePosToRecPtr().
>
> Can't we just take Insert->CurrBytePos without XLogBytePosToEndRecPtr()?
> Is there a point in alignment before the page header?
No, that'd be a completely bogus LSN, as CurrBytePos does not include any
space for page headers, to make the the very contended spinlock'ed section in
ReserveXLogInsertLocation() cheaper:
/*
* The duration the spinlock needs to be held is minimized by minimizing
* the calculations that have to be done while holding the lock. The
* current tip of reserved WAL is kept in CurrBytePos, as a byte
position
* that only counts "usable" bytes in WAL, that is, it excludes all WAL
* page headers. The mapping between "usable" byte positions and
physical
* positions (XLogRecPtrs) can be done outside the locked region, and
* because the usable byte position doesn't include any headers,
reserving
* X bytes from WAL is almost as simple as "CurrBytePos += X".
*/
SpinLockAcquire(&Insert->insertpos_lck);
And we rely on the page LSNs to be correct for the content of newly created
permanent relations with wal_level=minimal, so you can't just return arbitrary
other values.
Greetings,
Andres Freund
