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.