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: Thomas Munro <thomas.munro@gmail.com>
Cc: Alexander Lakhin <exclusion@gmail.com>, Sergei Kornilov <sk@zsrv.org>, Noah Misch <noah@leadboat.com>, Tom Lane <tgl@sss.pgh.pa.us>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, pgsql-bugs@lists.postgresql.org, Fujii Masao <masao.fujii@oss.nttdata.com>
Date: 2023-09-20T00:51:39Z
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 Wed, Sep 20, 2023 at 10:51:12AM +1200, Thomas Munro wrote:
> Ahh, yeah, there was a second problem screwing up the LSN accounting.
> I was trying to write correct xl_prev links, but also emitting
> optional padding records to get into the right position (depending on
> initial conditions varying between branches etc), and I forgot about
> the extra COMMIT records that emit_message() generates.  Which, I
> guess, comes back to Michael's observation that this would all be a
> bit easier if we had a way to emit and flush a single record...

Unfortunately this part is not going to be backpatched based on the
current consensus, so this is goint to need an alternate and/or fluffy
solution to force the flushes at given points:
https://www.postgresql.org/message-id/20230816003333.7hn2rx5m2l7una3d@awork3.anarazel.de

The test could be largely simplified on HEAD once the flush option is
available.  I'm just waiting a bit more on the other thread, keeping
an eye on the temperature, but it looks like nobody would complain to
make that optional, with the default being what we did previously, at
least..

> The solution in this version is to call get_insert_len() instead of
> using the result of emit_message() for the values returned by the
> advance_XXX() functions.  The result of emit_message() is actually the
> LSN of the following COMMIT record so can't be used directly for
> building xl_prev chains.

And by doing so the test paths would enter once again in the inner
loops generating the records so as we'll only get out once we know
that we are at the border wanted, enough to stabilize the tests.
Smart.
--
Michael