Horton

Re: BUG #17928: Standby fails to decode WAL on termination of primary

Michael Paquier <michael@paquier.xyz>

View thread
From: Michael Paquier <michael@paquier.xyz>
To: Noah Misch <noah@leadboat.com>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>, thomas.munro@gmail.com, exclusion@gmail.com, pgsql-bugs@lists.postgresql.org
Date: 2023-08-11T06:08:26Z
Lists: pgsql-bugs

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Correct assertion and comments about XLogRecordMaxSize.

  2. Fix edge-case for xl_tot_len broken by bae868ca.

  3. Don't use Perl pack('Q') in 039_end_of_wal.pl.

  4. Don't trust unvalidated xl_tot_len.

  5. Make recovery report error message when invalid page header is found.

  6. Add more protections in WAL record APIs against overflows

On Thu, Aug 10, 2023 at 07:58:08PM -0700, Noah Misch wrote:
> On Thu, Aug 10, 2023 at 04:45:25PM +0900, Michael Paquier wrote:
>> Good idea to pollute the data with recycled segments.  Using a minimal
>> WAL segment size whould help here as well in keeping a test cheap, and
>> two segments should be enough.  The alignment calculations and the
>> header size can be known, but the standby records are an issue for the
>> predictability of the test when it comes to adjust the length of the
>> logical message depending on the 8k WAL page, no?
> 
> Could be.  I expect there would be challenges translating that outline into a
> real test, but I don't know if that will be a major one.  The test doesn't
> need to be 100% deterministic.  If it fails 25% of the time and is not the
> slowest test in the recovery suite, I'd find that good enough.

FWIW, I'm having a pretty hard time to get something close enough to a
page border in a reliable way.  Perhaps using a larger series of
records and select only one would be more reliable..  Need to test
that a bit more.

>> FWIW, I came back to this thread while tweaking the error reporting of
>> xlogreader.c for the sake of this thread and this proposal is an
>> improvement to be able to make a distinction between an OOM and an
>> incorrect record:
>> https://www.postgresql.org/message-id/ZMh/WV+CuknqePQQ@paquier.xyz
>> 
>> Anyway, agreed that it's an improvement to remove this check out of
>> allocate_recordbuf().  Noah, are you planning to work more on that?
> 
> I can push xl_tot_len-validate-v1.patch, particularly given the testing result
> you reported today.  I'm content for my part to stop there.

Okay, fine by me.  That's going to help with what I am doing in the
other thread as I'd need to make a better difference between the OOM
and the invalid cases for the allocation path.

You are planning for a backpatch to take care of the inconsistency,
right?  The report has been on 15~ where the prefetching was
introduced.  I'd be OK to just do that and not mess up with the stable
branches more than necessary (aka ~14) if nobody complains, especially
REL_11_STABLE planned to be EOL'd in the next minor cycle.
--
Michael