pg_walinspect: ReadNextXLogRecord's first_record argument

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Jeff Davis <pgsql(at)j-davis(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
Subject: pg_walinspect: ReadNextXLogRecord's first_record argument
Date: 2022-08-16 16:34:23
Message-ID: CA+TgmoZAOGzPUifrcZRjFZ2vbtcw3mp-mN6UgEoEcQg6bY3OVg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

I was looking at the code for pg_walinspect today and I think I may
have found a bug (or else I'm confused about how this all works, which
is also possible). ReadNextXLogRecord() takes an argument first_record
of type XLogRecPtr which is used only for error reporting purposes: if
we fail to read the next record for a reason other than end-of-WAL, we
complain that we couldn't read the WAL at the LSN specified by
first_record.

ReadNextXLogRecord() has three callers. In pg_get_wal_record_info(),
we're just reading a single record, and the LSN passed as first_record
is the LSN at which that record starts. Cool. But in the other two
callers, GetWALRecordsInfo() and GetWalStats(), we're reading multiple
records, and first_record is always passed as the LSN of the first
record. That's logical enough given the name of the argument, but the
effect of it seems to be that an error while reading any of the
records will be reported using the LSN of the first record, which does
not seem right.

By contrast, pg_rewind's extractPageMap() reports the error using
xlogreader->EndRecPtr. I think that's correct. The toplevel xlogreader
function that we're calling here is XLogReadRecord(), which in turn
calls XLogNextRecord(), which has this comment:

/*
* state->EndRecPtr is expected to have been set by the last call to
* XLogBeginRead() or XLogNextRecord(), and is the location of the
* error.
*/

Thoughts?

--
Robert Haas
EDB: http://www.enterprisedb.com

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2022-08-16 16:40:49 Re: Add find_in_log() and advance_wal() perl functions to core test framework (?)
Previous Message Bharath Rupireddy 2022-08-16 16:33:59 Re: pg_receivewal and SIGTERM