Feedback-ID: id4a34324:Fastmail
Date: Thu, 5 Mar 2026 14:27:04 -0500
From: Andres Freund <andres@anarazel.de>
To: Andrey Borodin <x4mmm@yandex-team.ru>
Cc: pgsql-hackers@postgresql.org, Noah Misch <noah@leadboat.com>,
	Tomas Vondra <tv@fuzzy.cz>, Peter Geoghegan <pg@bowt.ie>
Subject: Re: gistGetFakeLSN() can return incorrect LSNs
Message-ID: <hiubiovh2sxfyd6ok2lthdsy4pw66sodmldmwvd2tqfmxuxajz@q7uha6sx6zpb>
References: <vf4hbwrotvhbgcnknrqmfbqlu75oyjkmausvy66ic7x7vuhafx@e4rvwavtjswo>
 <65418A7A-9139-486A-9F67-B2F83E148E83@yandex-team.ru>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <65418A7A-9139-486A-9F67-B2F83E148E83@yandex-team.ru>
Archived-At: 
 <https://www.postgresql.org/message-id/hiubiovh2sxfyd6ok2lthdsy4pw66sodmldmwvd2tqfmxuxajz%40q7uha6sx6zpb>
Precedence: bulk

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 <andres@anarazel.de> 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