MIME-Version: 1.0
References: <tencent_A37630454CE614AFE640C56E33B4A0F0AE05@qq.com>
In-Reply-To: <tencent_A37630454CE614AFE640C56E33B4A0F0AE05@qq.com>
From: surya poondla <suryapoondla4@gmail.com>
Date: Tue, 19 May 2026 14:20:56 -0700
Message-ID: 
 <CAOVWO5p7CNqx-j+M37V9ncbVZ-P5TacNekX9FbmqUG1Ene59cA@mail.gmail.com>
Subject: Re: [BUG] Take a long time to reach consistent after pg_rewind
To: cca5507 <cca5507@qq.com>
Cc: pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: multipart/alternative; boundary="000000000000ea5483065232417c"
Archived-At: 
 <https://www.postgresql.org/message-id/CAOVWO5p7CNqx-j%2BM37V9ncbVZ-P5TacNekX9FbmqUG1Ene59cA%40mail.gmail.com>
Precedence: bulk

--000000000000ea5483065232417c
Content-Type: text/plain; charset="UTF-8"

Subject: Re: [BUG] Take a long time to reach consistent after pg_rewind

Hi ChangAo,

Thanks for the careful diagnosis, I reproduced the hang on macOS on the
latest postgres code (It took a lot of iterations to reproduce it)
The LSN trace matches your description and I saw the below:
    minRecoveryPoint = 0/08000028
    consistent recovery state reached at = 0/08000060

In my run the standby was stuck for ~9 s; consistency was eventually
declared at 0/08000060 because a small upstream record (most likely
a RUNNING_XACTS snapshot from bgwriter) landed at 0/08000028 and let
lastReplayedEndRecPtr leap past the bad finish line.
With the new primary stopped after pg_rewind, the wait was unbounded as
expected.

Regarding the fix: the underlying issue is that minRecoveryPoint is
implicitly expected to be the end-LSN of a real WAL record, because
lastReplayedEndRecPtr (the value it gets compared against)
can only ever take such values.  All current writers respect this
expectation except pg_rewind: pg_basebackup uses the backup-end record's
EndRecPtr, and the in-running UpdateMinRecoveryPoint path
uses buffer LSNs, both of which are record-end LSNs by construction.
pg_rewind alone uses pg_current_wal_insert_lsn(), which can return a
position just past a page header when the source is idle.
That's why I'd lean toward fixing the producer (pg_rewind).

Concretely, your original suggestion having pg_rewind use
GetXLogInsertEndRecPtr() instead of GetXLogInsertRecPtr(), restores
the invariant globally, and doesn't require future call sites that compare
against minRecoveryPoint to know about page-header adjustments.

If we still want a defense-in-depth guard in CheckRecoveryConsistency() to
handle older pg_rewind binaries running against a newer server,
the v1 patch is on the right track, but I'd suggest:
  - documenting in the helper comment why exactly SizeOfXLogShortPHD /
    SizeOfXLogLongPHD past a page boundary are the only legal
    "non-record-end" minRecoveryPoint values (i.e. who can produce
    them and under what conditions);

  - auditing the other call sites that compare against
    minRecoveryPoint to confirm none of them needs the same
    adjustment, with a comment recording the conclusion.

I can put together a TAP test under src/bin/pg_rewind/t/ that forces a WAL
switch on the source, runs pg_rewind against an
otherwise-idle primary, and asserts that the rewound node reaches
consistency without further upstream activity.
Happy to send a v2 with that test if useful.

This is a liveness bug with potentially unbounded wait on idle promoted
primaries, so it does seem worth back-patching.

Regards,
Surya Poondla

--000000000000ea5483065232417c
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr">Subject: Re: [BUG] Take a long time to reach consistent af=
ter pg_rewind<br><br>Hi ChangAo,<br><br>Thanks for the careful diagnosis, I=
 reproduced the hang on macOS on the latest postgres code (It took a lot of=
 iterations to reproduce it)<br>The LSN trace matches your description and =
I saw the below:<br>=C2=A0 =C2=A0 minRecoveryPoint =3D 0/08000028<br>=C2=A0=
 =C2=A0 consistent recovery state reached at =3D 0/08000060<br><br>In my ru=
n the standby was stuck for ~9 s; consistency was eventually declared at 0/=
08000060 because a small upstream record (most likely<br>a RUNNING_XACTS sn=
apshot from bgwriter) landed at 0/08000028 and let lastReplayedEndRecPtr le=
ap past the bad finish line. =C2=A0<div>With the new primary stopped after =
pg_rewind, the wait was unbounded as expected.<br><br></div><div>Regarding =
the fix: the underlying issue is that minRecoveryPoint is implicitly expect=
ed to be the end-LSN of a real WAL record, because lastReplayedEndRecPtr (t=
he value it gets compared against)<br>can only ever take such values.=C2=A0=
 All current writers respect this expectation except pg_rewind: pg_baseback=
up uses the backup-end record&#39;s EndRecPtr, and the in-running UpdateMin=
RecoveryPoint path<br>uses buffer LSNs, both of which are record-end LSNs b=
y construction. pg_rewind alone uses pg_current_wal_insert_lsn(), which can=
 return a position just past a page header when the source is idle. =C2=A0<=
/div><div>That&#39;s why I&#39;d lean toward fixing the producer (pg_rewind=
).</div><div><br></div><div>Concretely, your original suggestion having pg_=
rewind use GetXLogInsertEndRecPtr() instead of GetXLogInsertRecPtr(), resto=
res<br>the invariant globally, and doesn&#39;t require future call sites th=
at compare against minRecoveryPoint to know about page-header adjustments.<=
br><br>If we still want a defense-in-depth guard in CheckRecoveryConsistenc=
y() to handle older pg_rewind binaries running against a newer server,<br>t=
he v1 patch is on the right track, but I&#39;d suggest:<br>=C2=A0 - documen=
ting in the helper comment why exactly SizeOfXLogShortPHD /<br>=C2=A0 =C2=
=A0 SizeOfXLogLongPHD past a page boundary are the only legal<br>=C2=A0 =C2=
=A0 &quot;non-record-end&quot; minRecoveryPoint values (i.e. who can produc=
e<br>=C2=A0 =C2=A0 them and under what conditions);<br><br>=C2=A0 - auditin=
g the other call sites that compare against<br>=C2=A0 =C2=A0 minRecoveryPoi=
nt to confirm none of them needs the same<br>=C2=A0 =C2=A0 adjustment, with=
 a comment recording the conclusion.<br><br>I can put together a TAP test u=
nder src/bin/pg_rewind/t/ that forces a WAL switch on the source, runs pg_r=
ewind against an<br>otherwise-idle primary, and asserts that the rewound no=
de reaches consistency without further upstream activity. =C2=A0<br>Happy t=
o send a v2 with that test if useful.<br><br>This is a liveness bug with po=
tentially unbounded wait on idle promoted primaries, so it does seem worth =
back-patching.<br><br>Regards,<br>Surya Poondla</div></div>

--000000000000ea5483065232417c--