On 12/10/20, 9:23 PM, "Michael Paquier" <michael@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.
Nathan