At Fri, 11 Dec 2020 19:27:31 +0000, "Bossart, Nathan" <bossa...@amazon.com> wrote in > On 12/10/20, 9:23 PM, "Michael Paquier" <mich...@paquier.xyz> wrote: > > 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 > > cause DecodeXLogRecord() to report an error with the end of the > > current record but it makes more sense to point to ReadRecPtr in this > > 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. > > I looked through all the calls to report_invalid_record() in > xlogreader.c and noticed that all but a few in > XLogReaderValidatePageHeader() already report an LSN. Of the calls in > XLogReaderValidatePageHeader() that don't report an LSN, it looks like > most still report a position, and the remaining ones are for "WAL file > is from different database system...," which IIUC generally happens on > the first page of the segment. > > Perhaps we could simply omit the LSN in the pg_waldump message.
Yeah, I had the same feeling. At least, the two LSNs in the message under discussion are simply redundant. So +1 to just remove the LSN at the caller site. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
