At Fri, 11 Dec 2020 17:19:33 +0900 (JST), Kyotaro Horiguchi <horikyota....@gmail.com> wrote in > At Fri, 11 Dec 2020 14:21:57 +0900, Michael Paquier <mich...@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
