public inbox for [email protected]  

help / color / mirror / Atom feed

From: Heikki Linnakangas <[email protected]>
To: Álvaro Herrera <[email protected]>
Cc: Maxim Orlov <[email protected]>
Cc: Postgres hackers <[email protected]>
Subject: Re: Rework SLRU I/O errors handle
Date: Fri, 6 Mar 2026 18:22:42 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>

On 06/03/2026 17:33, Álvaro Herrera wrote:
> 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.

Agreed.

> 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.

Hmm, I thought we could just never pass a NULL pointer, but if a Write 
fails, slru.c has no context where to pull that opaque_data. Perhaps we 
should just not call the callback in that case.

I'm starting to feel that what SlruReportIOError() currently uses as the 
errdetail, could well be the main error message, and the callback could 
provide the errdetail. I.e. swap the errmsg and errdetail we have now. 
That way, we can just leave out the errdetail for Write failures. The 
current errmsg we have for Write failures is pretty bad: "ERROR: could 
not access status of transaction 0". What's currently the errdetail, 
e.g. "Could not read from file \"%s\" at offset %d: %m", is a lot more 
informative.

- Heikki

view thread (6+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Rework SLRU I/O errors handle
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox