Date: Tue, 18 Jun 2019 09:13:19 -0700
From: Shawn Debnath <sdn@amazon.com>
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
CC: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: fix "Success" error messages
Message-ID: <20190618161319.GA5285@f01898859afd.ant.amazon.com>
References: <5de61b6b-8be9-7771-0048-860328efe027@2ndquadrant.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <5de61b6b-8be9-7771-0048-860328efe027@2ndquadrant.com>
User-Agent: Mutt/1.12.0 (2019-05-25)
Precedence: Bulk
Precedence: bulk

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)