Horton

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

Thomas Munro <thomas.munro@gmail.com>

View thread
From: Thomas Munro <thomas.munro@gmail.com>
To: Alexander Lakhin <exclusion@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>, 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-19T22:51:12Z
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

Attachments

On Wed, Sep 20, 2023 at 1:00 AM Alexander Lakhin <exclusion@gmail.com> wrote:
> I've tested this patch version on Debian 9 and it works fine now.

Progress...

> But as to wal_log_hints, I see that v7 patches work for REL_12_STABLE and
> REL_13_STABLE, but not for other branches, though v6 patches definitely
> failed on REL_12_STABLE and REL_13_STABLE too. So the behavior improved,
> but not for all versions, for some reason (I haven't looked deeper yet).

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

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.

> Also I've noticed a few minor discrepancies in the patches.
> A commit message in patch.15 contains the sentence "We should fix ...",
> not "We could fix ...", as in commit messages for other versions.
> Also, patch.12 contains a duplicated "Reviewed-by" string.
> And may be "xlp_prev" is a typo?

Fixed.