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
>From d1d306f80c53a59b67823ae044bf85a2718ea94e Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
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
>From 89fe3678ef9443fc56a8ebfc3657ad06f5585bf5 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
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
