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

From Kyotaro Horiguchi
Subject Re: pg_waldump error message fix
Date
Msg-id 20201211.174407.253153416936650938.horikyota.ntt@gmail.com
Whole thread Raw
In response to Re: pg_waldump error message fix  (Kyotaro Horiguchi <horikyota.ntt@gmail.com>)
Responses Re: pg_waldump error message fix
List pgsql-hackers
At Fri, 11 Dec 2020 17:19:33 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in 
> 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.

Wait! That's wrong.  Yeah, we can add ErrorRecPtr to point the error
record regardless of the context.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



pgsql-hackers by date:

Previous
From: Kyotaro Horiguchi
Date:
Subject: Re: pg_shmem_allocations & documentation
Next
From: Benoit Lobréau
Date:
Subject: Re: pg_shmem_allocations & documentation