Re: BufFileRead() error signalling

Thomas Munro <thomas.munro@gmail.com>

From: Thomas Munro <thomas.munro@gmail.com>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Michael Paquier <michael@paquier.xyz>, Robert Haas <robertmhaas@gmail.com>, Ibrar Ahmed <ibrar.ahmad@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2020-06-09T00:21:53Z
Lists: pgsql-hackers

Attachments

On Tue, Jun 9, 2020 at 2:49 AM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> I think using our standard "exception" mechanism makes sense.  As for
> additional context, I think usefulness of the error messages would be
> improved by showing the file path (because then user knows which
> filesystem/tablespace was full, for example), but IMO any additional
> context on top of that is of marginal additional benefit.  If we really
> cared, we could have errcontext() callbacks in the sites of interest,
> but that would be a separate patch and perhaps not backpatchable.

Cool.  It does show the path, so that'll tell you which file system is
full or broken.

I thought a bit about the ENOSPC thing, and took that change out.
Since commit 1173344e we handle write() returning a positive number
less than the full size by predicting that a follow-up call to write()
would surely return ENOSPC, without the hassle of trying to write
more, or having a separate error message sans %m.  But
BufFileDumpBuffer() does try again, and only raises an error if the
system call returns < 0 (well, it says <= 0, but 0 is impossible
according to POSIX, at least assuming you didn't try to write zero
bytes, and we already exclude that).  So ENOSPC-prediction is
unnecessary here.

> The wording we use is "could not seek TO block N".  (Or used to use,
> before switching to pread/pwrite in most places, it seems).

Fixed in a couple of places.

Commits

  1. Make BufFileWrite() void.

  2. Fix buffile.c error handling.

  3. Adjust WAL code so that checkpoints truncate the xlog at the previous