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@paquier.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