Re: Add LSN along with offset to error messages reported for WAL file read/write/validate header failures - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: Add LSN along with offset to error messages reported for WAL file read/write/validate header failures
Date
Msg-id Y4mnTnYRQZIccJHS@paquier.xyz
Whole thread Raw
In response to Re: Add LSN along with offset to error messages reported for WAL file read/write/validate header failures  (Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>)
Responses Re: Add LSN along with offset to error messages reported for WAL file read/write/validate header failures  (Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>)
List pgsql-hackers
On Thu, Nov 17, 2022 at 11:53:23AM +0530, Bharath Rupireddy wrote:
> Please do, if you feel so. Thanks for your review.

I don't really mind the addition of the LSN when operating on a given
record where we are reading a location, like in the five error paths
for the header validation or the three ones in ReadRecord()

Now this one looks confusing:
+       XLogSegNoOffsetToRecPtr(openLogSegNo, startoffset,
+                               wal_segment_size, lsn);
        ereport(PANIC,
                (errcode_for_file_access(),
                 errmsg("could not write to log file %s "
-                       "at offset %u, length %zu: %m",
-                       xlogfname, startoffset, nleft)));
+                       "at offset %u, LSN %X/%X, length %zu: %m",
+                       xlogfname, startoffset,
+                       LSN_FORMAT_ARGS(lsn), nleft)));

This does not always refer to an exact LSN of a record as we may be in
the middle of a write, so I would leave it as-is.

Similarly the addition of wre_lsn would be confusing?  The offset
looks kind of enough to me when referring to the middle of a page in
WALReadRaiseError().
--
Michael

Attachment

pgsql-hackers by date:

Previous
From: Peter Eisentraut
Date:
Subject: Re: pg_dump: Remove "blob" terminology
Next
From: sirisha chamarthi
Date:
Subject: Re: Introduce a new view for checkpointer related stats