From: Antonin Houska <ah@cybertec.at>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Thomas Munro <thomas.munro@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Subject: Re: Attempt to consolidate reading of XLOG page
Date: Mon, 25 Nov 2019 10:02:00 +0100
Message-ID: <50470.1574672520@antos> (raw)
In-Reply-To: <20191122225632.GA9169@alvherre.pgsql>
References: <20191122225632.GA9169@alvherre.pgsql>
Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> On 2019-Nov-22, Antonin Houska wrote:
>
> > As I pointed out in
> >
> > https://www.postgresql.org/message-id/88183.1574261429%40antos
> >
> > seg.ws_off only replaced readOff in XLogReaderState. So we should only update
> > ws_off where readOff was updated before commit 709d003. This does happen in
> > ReadPageInternal (see HEAD) and I see no reason for the final patch to update
> > ws_off anywhere else.
>
> Oh you're right.
>
> I see no reason to leave ws_off. We can move that to XLogReaderState; I
> did that here. We also need the offset in WALReadError, though, so I
> added it there too. Conceptually it seems clearer to me this way.
>
> What do you think of the attached?
It looks good to me. Attached is just a fix of a minor problem in error
reporting that Michael pointed out earlier.
> BTW I'm not clear what errors can pread()/pg_pread() report that do not
> set errno. I think lines 1083/1084 of WALRead are spurious now.
All I can say is that the existing calls of pg_pread() do not clear errno, so
you may be right. I'd appreciate more background about the "partial read" that
Michael mentions here:
https://www.postgresql.org/message-id/20191125033048.GG37821%40paquier.xyz
--
Antonin Houska
Web: https://www.cybertec-postgresql.comAttachments:
[text/x-diff] wal_read_errno.patch (822B, ../50470.1574672520@antos/2-wal_read_errno.patch)
download | inline diff:diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.cindex 04124bc254..eda81c1df1 100644--- a/src/bin/pg_waldump/pg_waldump.c+++ b/src/bin/pg_waldump/pg_waldump.c@@ -354,9 +354,11 @@ WALDumpReadPage(XLogReaderState *state, XLogRecPtr targetPagePtr, int reqLen,
state->segcxt.ws_segsize);
if (errinfo.wre_errno != 0)
- fatal_error("could not read in file %s, offset %u, length %zu: %s",- fname, errinfo.wre_off, (Size) errinfo.wre_req,- strerror(errinfo.wre_errno));+ {+ errno = errinfo.wre_errno;+ fatal_error("could not read in file %s, offset %u, length %zu: %m",+ fname, errinfo.wre_off, (Size) errinfo.wre_req);+ }
else
fatal_error("could not read in file %s, offset %u: length: %zu",
fname, errinfo.wre_off, (Size) errinfo.wre_req);
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: ah@cybertec.at, alvherre@2ndquadrant.com, michael@paquier.xyz, thomas.munro@gmail.com, robertmhaas@gmail.com
Subject: Re: Attempt to consolidate reading of XLOG page
In-Reply-To: <50470.1574672520@antos>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox