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

On Fri, Dec 2, 2022 at 12:50 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> 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()

Thanks for reviewing.

> 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().

Yes, I removed those changes. Even if someone sees an offset of a
record within a WAL file elsewhere, they have the new utility function
(0002) pg_walfile_offset_lsn().

I'm attaching the v4 patch set for further review.

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com