Re: fix "Success" error messages - Mailing list pgsql-hackers

From Shawn Debnath
Subject Re: fix "Success" error messages
Date
Msg-id 20190618161319.GA5285@f01898859afd.ant.amazon.com
Whole thread Raw
In response to fix "Success" error messages  (Peter Eisentraut <peter.eisentraut@2ndquadrant.com>)
Responses Re: fix "Success" error messages
List pgsql-hackers
On Tue, Jun 18, 2019 at 02:35:02PM +0200, Peter Eisentraut wrote:
> --- a/src/backend/access/transam/slru.c
> +++ b/src/backend/access/transam/slru.c
> @@ -923,15 +923,19 @@ SlruReportIOError(SlruCtl ctl, int pageno, TransactionId xid)
>              ereport(ERROR,
>                      (errcode_for_file_access(),
>                       errmsg("could not access status of transaction %u", xid),
> -                     errdetail("Could not read from file \"%s\" at offset %u: %m.",
> -                               path, offset)));
> +                     errno == 0
> +                     ? errdetail("Short read from file \"%s\" at offset %u.", path, offset)
> +                     : errdetail("Could not read from file \"%s\" at offset %u: %m.",
> +                                 path, offset)));

Perhaps using "Encountered partial read from file ..." would be better 
suited for the error message.

>              break;
>          case SLRU_WRITE_FAILED:
>              ereport(ERROR,
>                      (errcode_for_file_access(),
>                       errmsg("could not access status of transaction %u", xid),
> -                     errdetail("Could not write to file \"%s\" at offset %u: %m.",
> -                               path, offset)));
> +                     errno == 0
> +                     ? errdetail("Short write to file \"%s\" at offset %u.", path, offset)
> +                     : errdetail("Could not write to file \"%s\" at offset %u: %m.",
> +                                 path, offset)));

Similarly, "Encountered partial write to file ..." would be better? Not 
a 100% on using "Encountered" but "partial" seems to be the right word 
to use here.

Do note that SlruPhysicalWritePage() will always set errno to ENOSPC if 
errno was 0 during a write attempt, see [0]. Not sure this is a good 
assumption to make since write(2) should return ENOSPC if the storage 
medium is out of space [1].

[0] 
https://github.com/postgres/postgres/blob/master/src/backend/access/transam/slru.c#L854
[1] https://linux.die.net/man/2/write

-- 
Shawn Debnath
Amazon Web Services (AWS)



pgsql-hackers by date:

Previous
From: Alvaro Herrera
Date:
Subject: Re: Avoiding possible future conformance headaches in JSON work
Next
From: Alvaro Herrera
Date:
Subject: Re: pgsql: Avoid spurious deadlocks when upgrading a tuple lock