pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Michael Paquier <michael@paquier.xyz>
To: pgsql-committers@lists.postgresql.org
Subject: pgsql: Fix header check for continuation records where standbys could b
Date: Mon, 20 Jan 2025 00:31:07 +0000
Message-ID: <E1tZfh5-002T78-BD@gemulon.postgresql.org> (raw)

Fix header check for continuation records where standbys could be stuck

XLogPageRead() checks immediately for an invalid WAL record header on a
standby, to be able to handle the case of continuation records that need
to be read across two different sources.  As written, the check was too
generic, applying to any target LSN.  Based on an analysis by Kyotaro
Horiguchi, what really matters is to make sure that the page header is
checked when attempting to read a LSN at the boundary of a segment, to
handle the case of a continuation record that spawns across multiple
pages when dealing with multiple segments, as WAL receivers are spawned
they request WAL from the beginning of a segment.  This fix has been
proposed by Kyotaro Horiguchi.

This could cause standbys to loop infinitely when dealing with a
continuation record during a timeline jump, in the case where the
contents of the record in the follow-up page are invalid.

Some regression tests are added to check such scenarios, able to
reproduce the original problem.  In the test, the contents of a
continuation record are overwritten with junk zeros on its follow-up
page, and replayed on standbys.  This is inspired by 039_end_of_wal.pl,
and is enough to show how standbys should react on promotion by not
being stuck.  Without the fix, the test would fail with a timeout.  The
test to reproduce the problem has been written by Alexander Kukushkin.

The original check has been introduced in 066871980183, for a similar
problem.

Author: Kyotaro Horiguchi, Alexander Kukushkin
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/CAFh8B=mozC+e1wGJq0H=0O65goZju+6ab5AU7DEWCSUA2OtwDg@mail.gmail.com
Backpatch-through: 13

Branch
------
REL_16_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/2c2e1d4f42c00784b46023ba123bd5cd38d914fb

Modified Files
--------------
src/backend/access/transam/xlogrecovery.c       |  13 +-
src/test/recovery/meson.build                   |   1 +
src/test/recovery/t/043_no_contrecord_switch.pl | 153 ++++++++++++++++++++++++
3 files changed, 161 insertions(+), 6 deletions(-)



view thread (6+ messages)  latest in thread

Message-ID: <E1tZfh5-002T78-BD@gemulon.postgresql.org>
Permalink:  ../E1tZfh5-002T78-BD@gemulon.postgresql.org/
Also on:    postgresql.org/message-id/E1tZfh5-002T78-BD@gemulon.postgresql.org

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-committers@postgresql.org
  Cc: michael@paquier.xyz, pgsql-committers@lists.postgresql.org
  Subject: Re: pgsql: Fix header check for continuation records where standbys could b
  In-Reply-To: <E1tZfh5-002T78-BD@gemulon.postgresql.org>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox