Horton

Re: Infinite loop in XLogPageRead() on standby

Michael Paquier <michael@paquier.xyz>

View thread
From: Michael Paquier <michael@paquier.xyz>
To: Alexander Kukushkin <cyberdemn@gmail.com>
Cc: Ants Aasma <ants.aasma@cybertec.at>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, pgsql-hackers@postgresql.org, thomas.munro@gmail.com
Date: 2025-01-15T04:45:05Z
Lists: pgsql-hackers

Commits

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

  1. Fix header check for continuation records where standbys could be stuck

  2. Move routines to manipulate WAL into PostgreSQL::Test::Cluster

  3. Prevent archive recovery from scanning non-existent WAL files.

  4. Fix scenario where streaming standby gets stuck at a continuation record.

Attachments

On Wed, Dec 25, 2024 at 12:00:59PM +0900, Michael Paquier wrote:
> All of them refer to an infinite loop reachable in the startup process
> when we read an incorrect incomplete record just after a failover or
> when a WAL receiver restarts.  Not sure which way is best in order to
> fix all of them in one shot, I am still considering what the possible
> options at hand are here.

And please find attached a set of two patches addressing the issue.
I've reviewed again the thread and after pondering quite a bit about
it I am on board with the point of making the early header check in
the page read callback less generic by restricting it to a WAL segment
boundary, as that's the original case we care about, to map also with 
the fact that WAL receivers are spawned with a LSN starting at the
beginning of segment, as suggested by Horiguchi-san.  As far as I can
see, I've misunderstood a bit things last week about the backpatch
point, sorry about that.

The new regression test is something I really want to keep around,
to be able to emulate the infinite loop, but I got annoyed with the
amount of duplication between the new test and the existing
039_end_of_wal.pl as there share the same ideas.  I have refactored
039_end_of_wal.pl and moved its routines to generate WAL messages,
to advance WAL and to write WAL into Cluster.pm, then reused the
refactored result in the new test.  I've also detected a race
condition in the new test, where we should wait for the logs of the
two standbys to report their "invalid magic number" failures.  The
test could fail if the replay is slow when scanning their logs.  There
is wait_for_log() for this job, available down to v13.

Barring objections, I'd like to get the main issue in 0002 fixed at
the beginning of next week.  I am also planning to do the refactoring
bits tomorrow or so as these are rather straight-forward.  The names
of the new routines in Cluster.pm are inherited from the existing
recovery test.  Perhaps they could use a better name, but 0001 does
not look that bad to me either.
--
Michael