Re: Rework SLRU I/O errors handle

On 2026-Mar-06, Heikki Linnakangas wrote:

> I'm -1 on the last patch, "Expand xact SLRU IO-error to show epoch" though.
> The epoch isn't used in addressing the SLRU, the patch just expands the
> 32-bit XID into a full 64-bit XID using the current epoch. That seems
> misleading.

I agree on that.

I'm not a fan of the split. I think it all these patches should be
pushed as a single commit, and avoid introducing xact_errmsg_for_io_error
as an exposed function. I think that doesn't make a lot of sense. Each
SLRU should have a correct and appropriate error reporting callback.

The comment added in 0005 is bogus too. It mentions InvalidTransactionId,
but the problem is not that the value is 0 but rather that we get no
pointer. Also, in all other callbacks the pointer is asserted to not be
NULL, so why don't we do the same here, and avoid an error message
that's not going to help anyone? I see however that in the patch we're
passing a NULL to SlruReportIOError(), which means if you get an IO
error with any SLRU other than xact, you're going to get either a crash
on the assertion, or (on non-debug builds) a crash on dereferencing the
NULL pointer.

--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"If you want to have good ideas, you must have many ideas. Most of them
will be wrong, and what you have to learn is which ones to throw away."
(Linus Pauling)