Re: pg_waldump error message fix - Mailing list pgsql-hackers

From Kyotaro Horiguchi
Subject Re: pg_waldump error message fix
Date
Msg-id 20201211.171933.1331902748959798238.horikyota.ntt@gmail.com
Whole thread Raw
In response to Re: pg_waldump error message fix  (Michael Paquier <michael@paquier.xyz>)
Responses Re: pg_waldump error message fix
List pgsql-hackers
At Fri, 11 Dec 2020 14:21:57 +0900, Michael Paquier <michael@paquier.xyz> wrote in 
> On Fri, Dec 11, 2020 at 01:30:16PM +0900, Kyotaro Horiguchi wrote:
> > currRecPtr is a private member of the struct (see the definition of
> > the struct XLogReaderState).  We should instead use EndRecPtr outside
> > xlog reader.
> 
> Please note that this is documented in xlogreader.h.  Hmm.  I can see
> your point here, still I think that what both of you are suggesting
> is not completely correct.  For example, switching to EndRecPtr would

EndRecPtr is defined as it points to the LSN to start reading the next
record.  It donsn't move if XLogReadRecord failed to read the
record. I think this is documented in a comment somewhere.  It can
point to the beginning of a page so "error in WAL record at <page
start>" is a kind of bogus but that is not the point here.

> cause DecodeXLogRecord() to report an error with the end of the
> current record but it makes more sense to point to ReadRecPtr in this

DecodeXLogRecord() handles a record alread successflly read. So
ReadRecPtr is pointing to the beginning of the given record at the
timex.  pg_waldump:main() and ReadRecrod (or the context of
DecodeXLogRecord()) are in different contexts.  The place in question
in pg_waldump seems to be a result of a thinko that it can use
ReadRecPtr regardless of whether XLogReadRecrod successfully read a
record or not.

> case.  On the other hand, it would make sense to report the beginning 
> of the position we are working on when validating the header if an
> error happens at this point.  So it seems to me that EndRecPtr or
> ReadRecPtr are not completely correct, and that we may need an extra
> LSN position to tell on which LSN we are working on instead that gets
> updated before the validation part, because we update ReadRecPtr and
> EndRecPtr only after this initial validation is done.

So we cannot use the ErrorRecPtr since pg_waldump:main() shoud show
the LSN XLogReadRecord() found a invalid record and DecodeXLogRecord()
should show the LSN XLogReadRecord() found a valid record.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



pgsql-hackers by date:

Previous
From: Michael Paquier
Date:
Subject: Re: Fail Fast In CTAS/CMV If Relation Already Exists To Avoid Unnecessary Rewrite, Planning Costs
Next
From: Kyotaro Horiguchi
Date:
Subject: Re: pg_shmem_allocations & documentation