Subject: Re: fix "Success" error messages
To: Michael Paquier <michael@paquier.xyz>, Shawn Debnath <sdn@amazon.com>
References: <5de61b6b-8be9-7771-0048-860328efe027@2ndquadrant.com>
 <20190618161319.GA5285@f01898859afd.ant.amazon.com>
 <20190619025155.GD17450@paquier.xyz>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
From: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Openpgp: preference=signencrypt
Autocrypt: addr=peter.eisentraut@2ndquadrant.com; prefer-encrypt=mutual;
 keydata=
 xsFNBFUUt6kBEADZd9NNTBJbHmAXXJgPGOdd6Q73l80oifIVgn2tRxP9dYkCfTlo/yLqbKTl
 JNqTcyM6XrJDnB2sMEUGYCewYZXl+9C4bBJWyNqcEI1ae9u71vEq/fknFLArJ+2wN+f7QuRx
 Q8d94ZJAiJQ5g3fNk2HVeedCQCKP5dCzU+ZLt2dbsx1PDZLjTO9gf+JswWuGFb5wY5PZVM5V
 hq66Xk7ZJzHuARacza2RT7J2fkMmQBY+kr7L98Perb0GhafnlhXB0PvUB+N3xpmggMfwuhTu
 BngS/G/r3GvemxJuFxouw8xsMqrsfdksJXA2gLXYtwRCssdPV3PO0rlD9twqDo8URfIJ5fb6
 1g/MV8aTOKjg3Q8MpJPtkwJqN4o3PvbwYdedeFnOVkkYAJdu/aX1v1BeCBDu7Hy3sF52scD3
 pQbyQSl9vUDpTLarASAxNbAV0NRJE8dp4vfW6AH1ccryGiFqw/czPK/iN0+MT8wsjAW6WW4w
 hvjm5UPA1KTwi3ZT8eT5uycuH7QpuNk7LnGGXQ6KaRALFywzkr0A0V0AvFNa+l4wstMaXaB6
 dmMXyF0/hfryqyOeu1gAdq4q0/PNC2dPgb/6uoh4eMjN6A5GXcprilhz5DulmsilY21hfSoU
 S64ODed96pKhprsOxjZiDRsN1QM5MwB+uwVWUNfsX7fIxE0BrwARAQABzSdQZXRlciBFaXNl
 bnRyYXV0IDxwZXRlckBlaXNlbnRyYXV0Lm9yZz7CwXoEEwEIACQCGwMFCwkIBwMFFQoJCAsF
 FgIDAQACHgECF4AFAlUUuKQCGQEACgkQoZ8SQ+xBJp8llw//RbGoXnF72FZTlOFsnNyxnWbF
 vViPCfdAlZl1GGK9OYCvV1dp/956HuhB0wpaGQHiBmQfOPJf/jLOPjXFYm2iEChEycrBChuE
 j4TSYemMp5BSPILYk9Uwd16kVD/+U6fQfK6H5pOhfBvbM6asMnmpY2wQuvktxHvaGlytFw84
 EzFWNZtAZfxkw28w3AhIQkKe9hJanfyDL750RcbyEr8AAHty5IErrvS7tM5vQwIeh5BF26ND
 CyKPRdZgOgMpcwFfROKBDpAt052CgWIpNdhBdfIKlkCs4/ILxmC3rT/Mb/qZiHcoMMwEu6k+
 T7vRAFZZAk62Yrr/U8/vUGKlzRZQoo86fyojCfI0ibCIc0z8rwCVY9P40G9wD9BTCZS/z8Vh
 jjpBlNnIKw2hM6hbxavKndCJjFFJD08rXrTyrLF/of18xD37vNdYooP9VATB5QjOWFRrscSZ
 4a89e+to3m7YVnd4xdXwGp47dYusoax84C8tg1gZrH38ol9dPmnKX6GZrXeS7tH9k8VeUpJ8
 HQbRThQRWAT936QpYbLConBmOtmbLmjQOGtiXPo1oK57X/7U/WBja5vWmm2q+Yxtt+3uHGVg
 DdDU4CJpXo4lX0v2n3QV5EBSW9hjdF3bNfl6GFBb1pPEhqNcMDUc7zkx/2uENdAdk/BnqMgH
 umip7b0pVfvOwU0EVRS3qQEQAJf6p0P7D+hMYUgCPkvgrcz6OJYsAYdUMUmokfqmkKwK0oJM
 ew8TNGM53ocKAgQGma5f10rwbZNPlpVPNRX5hwUmwVlcwSut0oE+JfUBH7mpysWvpF05RKIy
 q9RCoJZrAbXS7Tig56A1QD8IcM00rERZMcCOlI1+q9m4u9AOI+0LyGGSH+VvoyDHD/Jew4R2
 TmPEF8Pu7lYiNYK88Q6u0kBkc+Tz3BNlyc6fi3HxnFfp0b75HLXPSTu2SwA/0Q31wHZXwrEU
 BR9KgSn/qbgy8ydPc97i82fhiFE3cbGexfPrQSMYmDGi+dcQ0s+AMQwC3XyMC0JkHpYUuFkR
 QdlIvaIhgFUUN2Vr4xF2ADaNTNFc4DYaiPHFAgHZw+3pZWqS4uIiWgJynrEaHw4F3QWU8/JB
 iyEWDhNq4WBrRA0u914TCCAoFOOKMnRdl8eJrRQKM/5/lXbHFjjlI/LDKhLRFsuQsnJJ3331
 RjCzpHPZG5/p5KR1RHiAXa0p/DW5lXyq21hS3kjDlleoHqOxCN/cy0Jgz7qLDq1vZoCl67Sy
 RAH60c2Xu4may9SVltmot0OzWVHdrfS+85FQGp9580DaeLio7YMFKy2bfEvwQJ/c9xD2QPB+
 oSb3V8ueSznXBG5+dyfOsZFRPxceA0wA2rSWZSjxLdKxvyGTj9vsCjaDu13rABEBAAHCwV8E
 GAEIAAkFAlUUt6kCGwwACgkQoZ8SQ+xBJp+HJhAAxmSTsC5y7p9Zu+nnLpucv/h88oEx985e
 qWeXzQiwM3pfRZS+PdTH9TyzHK0KOgKoAiubk0vt+BrT91D9d0G5kVQaTctAErGBoMkTaZJT
 XVEcNphiPzysdSqvprjojMp1y7rG8AP+Xd6m0rE1LRnkRRm8HyI8vdXsuLQPITGWXDLMjaNq
 AOV3BUnzkgtjMuIYw7SCr+12ZOb5dEKvaG4kjC4kaJ/TLbuBhVANcXkC90yp0AOZLs/cB1Dj
 OAXno/RUUruJ3r27a+pFv768UAi9QWcdw58yPxreMNMLqAgsS9oTHwHfGNWcB2dJ4UW+GMLJ
 hTf9TjQ2f+N/sIx5XMQ9pYw6CAJMK29qPx6/ZayrPxf22EXf0D3zTwIEZgwfSkynFVwV25L7
 awQq60fifdMYheSxq7GbhpsixmcD+e161cCmLIt75YDt5bQ0B1yf7EmxG8n7pGZAOM6LoNsU
 fkSldba5XVwh8LuN5mK0nQBELGW6Os4TDq4REggLpfcvFC215zpuo0iCGnmE/Bns4nowqUSP
 u4tkWL+nxW4qgv8wo8HJ52jd79EJ8iqfIBoidBfy9AIUiEUApGQzWPu6yWcI8fY0rN5D+tc+
 Yw1Zt6Mm+huYJpgCRsTcfzEcahnBn521CWcWvhW1ff3KFileFMtNmnBBhMncmRKHphtTKEpN
 3jI=
Organization: 2ndQuadrant
Message-ID: <36d38542-63e9-b73c-09fe-bd5e32e06066@2ndquadrant.com>
Date: Mon, 26 Aug 2019 21:40:23 +0200
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:52.0)
 Gecko/20100101 Thunderbird/52.9.1
MIME-Version: 1.0
In-Reply-To: <20190619025155.GD17450@paquier.xyz>
Content-Type: multipart/mixed;
 boundary="------------893F683A237131F02D66269A"
Content-Language: en-US
Precedence: bulk

This is a multi-part message in MIME format.
--------------893F683A237131F02D66269A
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit

On 2019-06-19 04:51, Michael Paquier wrote:
> On Tue, Jun 18, 2019 at 09:13:19AM -0700, Shawn Debnath wrote:
>>>  		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)));
> 
> There is no point to call errcode_for_file_access() if we know that
> errno is 0.  Not a big deal, still..  The last time I looked at that,
> the best way I could think of would be to replace slru_errcause with a
> proper error string generated at error time.  Perhaps the partial
> read/write case does not justify this extra facility.  I don't know.
> At least that would be more flexible.

Here is an updated patch set that rearranges this a bit according to
your suggestions, and also fixes some similar issues in pg_checksums.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--------------893F683A237131F02D66269A
Content-Type: text/plain; charset=UTF-8; x-mac-type="0"; x-mac-creator="0";
 name="v2-0001-Better-error-messages-for-short-reads-writes-in-S.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename*0="v2-0001-Better-error-messages-for-short-reads-writes-in-S.pa";
 filename*1="tch"

From d1d306f80c53a59b67823ae044bf85a2718ea94e Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Mon, 26 Aug 2019 21:37:10 +0200
Subject: [PATCH v2 1/2] Better error messages for short reads/writes in SLRU

This avoids getting a

    Could not read from file ...: Success.

for a short read or write (since errno is not set in that case).
Instead, report a more specific error messages.
---
 src/backend/access/transam/slru.c | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index 0fbcb4e6fe..e38f9199dd 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -920,18 +920,29 @@ SlruReportIOError(SlruCtl ctl, int pageno, TransactionId xid)
 							   path, offset)));
 			break;
 		case SLRU_READ_FAILED:
-			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)));
+			if (errno)
+				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)));
+			else
+				ereport(ERROR,
+						(errmsg("could not access status of transaction %u", xid),
+						 errdetail("Could not read from file \"%s\" at offset %u: read too few bytes.", path, offset)));
 			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)));
+			if (errno)
+				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)));
+			else
+				ereport(ERROR,
+						(errmsg("could not access status of transaction %u", xid),
+						 errdetail("Could not write to file \"%s\" at offset %u: wrote too few bytes.",
+								   path, offset)));
 			break;
 		case SLRU_FSYNC_FAILED:
 			ereport(data_sync_elevel(ERROR),

base-commit: acb96eb7d294a003a9392cdd445630ef137d9918
-- 
2.22.0


--------------893F683A237131F02D66269A
Content-Type: text/plain; charset=UTF-8; x-mac-type="0"; x-mac-creator="0";
 name="v2-0002-pg_checksums-Handle-read-and-write-returns-correc.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename*0="v2-0002-pg_checksums-Handle-read-and-write-returns-correc.pa";
 filename*1="tch"

From 89fe3678ef9443fc56a8ebfc3657ad06f5585bf5 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Mon, 26 Aug 2019 21:37:50 +0200
Subject: [PATCH v2 2/2] pg_checksums: Handle read and write returns correctly

The read() return was not checking for errors, the write() return was
not checking for short writes.
---
 src/bin/pg_checksums/pg_checksums.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/src/bin/pg_checksums/pg_checksums.c b/src/bin/pg_checksums/pg_checksums.c
index 8c00ec9a3b..40619be0fa 100644
--- a/src/bin/pg_checksums/pg_checksums.c
+++ b/src/bin/pg_checksums/pg_checksums.c
@@ -196,7 +196,13 @@ scan_file(const char *fn, BlockNumber segmentno)
 
 		if (r == 0)
 			break;
-		if (r != BLCKSZ)
+		else if (r < 0)
+		{
+			pg_log_error("could not read block %u in file \"%s\": %m",
+						 blockno, fn);
+			exit(1);
+		}
+		else if (r != BLCKSZ)
 		{
 			pg_log_error("could not read block %u in file \"%s\": read %d of %d",
 						 blockno, fn, r, BLCKSZ);
@@ -222,6 +228,8 @@ scan_file(const char *fn, BlockNumber segmentno)
 		}
 		else if (mode == PG_MODE_ENABLE)
 		{
+			int		w;
+
 			/* Set checksum in page header */
 			header->pd_checksum = csum;
 
@@ -233,10 +241,15 @@ scan_file(const char *fn, BlockNumber segmentno)
 			}
 
 			/* Write block with checksum */
-			if (write(f, buf.data, BLCKSZ) != BLCKSZ)
+			w = write(f, buf.data, BLCKSZ);
+			if (w != BLCKSZ)
 			{
-				pg_log_error("could not write block %u in file \"%s\": %m",
-							 blockno, fn);
+				if (w < 0)
+					pg_log_error("could not write block %u in file \"%s\": %m",
+								 blockno, fn);
+				else
+					pg_log_error("could not write block %u in file \"%s\": wrote %d of %d",
+								 blockno, fn, w, BLCKSZ);
 				exit(1);
 			}
 		}
-- 
2.22.0


--------------893F683A237131F02D66269A--