Horton
Menu

Thread

  1. [BUG] Take a long time to reach consistent after pg_rewind

    cca5507 <cca5507@qq.com> — 2026-04-10T09:57:39Z

    Hi,

Steps to reproduce (PG19):

1) start two nodes, node1 (primary), node2 (standby), both with the following configuration:

```
archive_mode = on
archive_command = '/bin/true'
archive_timeout = 10
checkpoint_timeout = '60min'
wal_keep_size = 1024
logging_collector = on
```

2) promote node2

3) stop node1

4) make sure the pg_current_wal_insert_lsn() of node2 is at the begin of a wal
segment (end with 000028), if not, do a checkpoint and recheck. (archive_timeout
will switch the wal)

5) execute pg_rewind with node1

6) start node1

7) now node1 can't reach consistent until node2 write some wal

Logs of node1:

```
2026-04-10 16:16:07.802 CST [45623] LOG:  starting backup recovery with redo LSN 0/02000028, checkpoint LSN 0/02000088, on timeline ID 1
2026-04-10 16:16:07.802 CST [45623] LOG:  entering standby mode
2026-04-10 16:16:07.803 CST [45623] LOG:  redo starts at 0/02000028
2026-04-10 16:16:07.803 CST [45623] LOG:  completed backup recovery with redo LSN 0/02000028 and end LSN 0/02000130
2026-04-10 16:16:07.806 CST [45624] LOG:  started streaming WAL from primary at 0/04000000 on timeline 2
2026-04-10 16:19:13.083 CST [47039] FATAL:  the database system is not yet accepting connections
2026-04-10 16:19:13.083 CST [47039] DETAIL:  Consistent recovery state has not been yet reached.
2026-04-10 16:20:16.413 CST [45623] LOG:  consistent recovery state reached at 0/04000048
2026-04-10 16:20:16.413 CST [45616] LOG:  database system is ready to accept read-only connections
```

Root cause:

The min recovery point of node1 is at 0/04000028, but node2 doesn't have any wal after that and may keep idle for
a long time.

Possible fix:

The pg_rewind use pg_current_wal_insert_lsn() to set the min recovery point, which calls
GetXLogInsertRecPtr() and returns the latest wal insert pointer. Maybe we should use
GetXLogInsertEndRecPtr() which returns the latest wal record end pointer.

Thoughts?

--
Regards,
ChangAo Chen

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

    cca5507 <cca5507@qq.com> — 2026-04-13T13:14:19Z

    > Possible fix:
> 
> The pg_rewind use pg_current_wal_insert_lsn() to set the min recovery point, which calls
> GetXLogInsertRecPtr() and returns the latest wal insert pointer. Maybe we should use
> GetXLogInsertEndRecPtr() which returns the latest wal record end pointer.
> 
> Thoughts?

Another solution:

If minRecoveryPoint is just after a xlog page header, we can move it to the begin of
the page. It's safe because we just skip the xlog page header. Do I miss something?

Attach a patch done like this.

--
Regards,
ChangAo Chen

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

    surya poondla <suryapoondla4@gmail.com> — 2026-05-19T21:20:56Z

    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

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

    cca5507 <cca5507@qq.com> — 2026-06-11T03:03:17Z

    Hi Surya,

Thanks for the review and sorry for the late reply.

I get a new idea to fix this bug from:

https://www.postgresql.org/message-id/05DEB465-3C99-417E-B7FA-275A28068D90%40yandex-team.ru

Please see the v2 patch for details.

--
Regards,
ChangAo Chen

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

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2026-06-11T06:35:32Z

    Hello.

At Thu, 11 Jun 2026 11:03:17 +0800, "cca5507" <cca5507@qq.com> wrote in 
> I get a new idea to fix this bug from:
> 
> https://www.postgresql.org/message-id/05DEB465-3C99-417E-B7FA-275A28068D90%40yandex-team.ru
> 
> Please see the v2 patch for details.

Thanks for the patch.

However, I am not sure that using the flush LSN is the right
direction.

As I understand it, the root of the problem is that
get_current_wal_insert_lsn() returns the next WAL insertion position,
while minRecoveryPoint is expected to point just past the end of the
last required record. At positions immediately following a WAL page or
segment header, those locations can be logically equivalent but
numerically different.

I think pg_rewind is probably using the insert LSN because it wants to
choose a conservative position as far ahead as possible.  It might be
possible to use the flush LSN if the copying logic is carefully
arranged, but I would prefer to keep using the insert LSN if we can.

I also think it is unfortunate to require client-side tools to be
aware of the exact "end of last record + 1" representation. If both
representations are logically equivalent at those positions, I would
rather have the recovery side accept and normalize them than require
every tool to produce exactly the expected form.

So I wonder whether we should instead normalize minRecoveryPoint when
reading it from the control file, so that these equivalent
representations are treated as the same position. That would also make
the recovery side a bit more robust.

For example, something like this:

diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index 73b78a83fa7..660c681795d 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -955,6 +955,20 @@ InitWalRecovery(ControlFileData *ControlFile, bool *wasShutdown_ptr,
 	{
 		minRecoveryPoint = ControlFile->minRecoveryPoint;
 		minRecoveryPointTLI = ControlFile->minRecoveryPointTLI;
+
+		/*
+		 * minRecoveryPoint in the control file is expected to point to the
+		 * location immediately following the end of the target
+		 * record. However, some tools may record a location immediately after
+		 * a WAL segment or page header when the target position is at the
+		 * beginning of a segment or page. Normalize such values here before
+		 * further processing.
+		 */
+		if (XLogSegmentOffset(minRecoveryPoint, wal_segment_size)
+			== SizeOfXLogLongPHD)
+			minRecoveryPoint -= SizeOfXLogLongPHD;
+		else if (minRecoveryPoint % XLOG_BLCKSZ == SizeOfXLogShortPHD)
+			minRecoveryPoint -= SizeOfXLogShortPHD;
 	}
 	else
 	{


Thoughts?

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

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

    cca5507 <cca5507@qq.com> — 2026-06-11T07:50:19Z

    Hi,

> As I understand it, the root of the problem is that
> get_current_wal_insert_lsn() returns the next WAL insertion position,
> while minRecoveryPoint is expected to point just past the end of the
> last required record. At positions immediately following a WAL page or
> segment header, those locations can be logically equivalent but
> numerically different.

Yes.

> I think pg_rewind is probably using the insert LSN because it wants to
> choose a conservative position as far ahead as possible.  It might be
> possible to use the flush LSN if the copying logic is carefully
> arranged, but I would prefer to keep using the insert LSN if we can.

The insert LSN is not crash safe, is this really make sense to use it? For
example, the primary has insert LSN 1000, flush LSN 500, the standby
sets minRecoveryPoint to 1000, and then the primary crash and restart.
The primary now only has LSN 500, but the standby cannot reach
consistent until LSN 1000. This doesn’t make sense to me.

> I also think it is unfortunate to require client-side tools to be
> aware of the exact "end of last record + 1" representation. If both
> representations are logically equivalent at those positions, I would
> rather have the recovery side accept and normalize them than require
> every tool to produce exactly the expected form.

Sounds reasonable.

> So I wonder whether we should instead normalize minRecoveryPoint when
> reading it from the control file, so that these equivalent
> representations are treated as the same position. That would also make
> the recovery side a bit more robust.
> 
> For example, something like this:
> 
> diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
> index 73b78a83fa7..660c681795d 100644
> --- a/src/backend/access/transam/xlogrecovery.c
> +++ b/src/backend/access/transam/xlogrecovery.c
> @@ -955,6 +955,20 @@ InitWalRecovery(ControlFileData *ControlFile, bool *wasShutdown_ptr,
>   {
>   minRecoveryPoint = ControlFile->minRecoveryPoint;
>   minRecoveryPointTLI = ControlFile->minRecoveryPointTLI;
> +
> + /*
> +  * minRecoveryPoint in the control file is expected to point to the
> +  * location immediately following the end of the target
> +  * record. However, some tools may record a location immediately after
> +  * a WAL segment or page header when the target position is at the
> +  * beginning of a segment or page. Normalize such values here before
> +  * further processing.
> +  */
> + if (XLogSegmentOffset(minRecoveryPoint, wal_segment_size)
> + == SizeOfXLogLongPHD)
> + minRecoveryPoint -= SizeOfXLogLongPHD;
> + else if (minRecoveryPoint % XLOG_BLCKSZ == SizeOfXLogShortPHD)
> + minRecoveryPoint -= SizeOfXLogShortPHD;
>   }
>   else
>   {

I had thought about this before. To update minRecoveryPoint in place, I think we
should make sure that it won't cause any side effects. That means we need to
check every places we use minRecoveryPoint. That's why the v1 patch introduces
GetEffectiveMinRecoveryPoint() rather than updates it in place.

--
Regards,
ChangAo Chen

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

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2026-06-12T08:33:30Z

    Hello.

At Thu, 11 Jun 2026 15:50:19 +0800, "cca5507" <cca5507@qq.com> wrote in 
> > I think pg_rewind is probably using the insert LSN because it wants to
> > choose a conservative position as far ahead as possible.  It might be
> > possible to use the flush LSN if the copying logic is carefully
> > arranged, but I would prefer to keep using the insert LSN if we can.
> 
> The insert LSN is not crash safe, is this really make sense to use it? For
> example, the primary has insert LSN 1000, flush LSN 500, the standby
> sets minRecoveryPoint to 1000, and then the primary crash and restart.
> The primary now only has LSN 500, but the standby cannot reach
> consistent until LSN 1000. This doesn’t make sense to me.

My understanding had been that the state produced by pg_rewind only
needed to be valid with respect to the source server at the time
pg_rewind was run.

If the expectation is that the rewound standby must also remain usable
after the source server subsequently goes through crash recovery, then
I agree that using the insert LSN becomes harder to justify.

> I had thought about this before. To update minRecoveryPoint in place, I think we
> should make sure that it won't cause any side effects. That means we need to
> check every places we use minRecoveryPoint. That's why the v1 patch introduces
> GetEffectiveMinRecoveryPoint() rather than updates it in place.

The reason I suggested that approach in my earlier email was simply
that, as far as I could tell, that was the only place that needed to
interpret a minRecoveryPoint value. That said, since minRecoveryPoint
is expected to point to the end of the last required record, I think
normalizing the value earlier would also be reasonable.

Anyway, if we decide to use the flush LSN instead, then none of this
should be necessary.

Regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

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

    surya poondla <suryapoondla4@gmail.com> — 2026-06-15T23:22:06Z

    Hi,

Thanks ChangAo for v2, and Kyotaro for working through the alternatives. I
think v2 is on the right track.

For me the Flush LSN looks correct for the below reasons:
1. The standby can reach it. XLOG_SWITCH (driven by archive_timeout) calls
XLogFlush(EndPos) with EndPos = segment boundary. xlogreader treats SWITCH
specially and advances NextRecPtr to end-of-segment
 (XLogDecodeNextRecord()), so lastReplayedEndRecPtr lands on the segment
boundary and the consistency check fires. With v2, minRecoveryPoint =
segment boundary, which is exactly reachable.

This restores the implicit producer-side invariant: pg_basebackup's
XLOG_BACKUP_END EndRecPtr, UpdateMinRecoveryPoint() via
GetCurrentReplayRecPtr(), and the standby-source path in pg_rewind all
yield a value  record-driven progress can match.

Caveat: XLogBackgroundFlush rounds WriteRqst.Write down to a page boundary,
and the asyncXactLSN bump only re-targets when we've already flushed past
it. On a busy primary the flush LSN can therefore sit mid-record, but
that's fine because live insertions advance the standby past whatever
endrec we pin. Not the scenario in this bug, just worth noting that no
producer-side or recovery-side fix eliminates every theoretical liveness
corner.

2. GetXLogInsertRecPtr() reads CurrBytePos from shared memory not durable.
If the source crashes after pg_rewind, it recovers to its flush LSN,
leaving the target's minRecoveryPoint pinned to a higher
insert LSN which is unreachable forever. flush LSN sits at the segment
boundary, which is on disk and recovery cannot regress past it. So this
approach is crash-safe

3. perform_rewind() does: copy files -> fetch source control file ->
get_current_wal_flush_lsn.
Any WAL-logged page on source's disk has page-LSN <= source's flush-LSN at
the time it was written (FlushBuffer enforces XLogFlush before eviction;
hint-bit pages with checksums use XLogSaveBufferForHint + PageSetLSN).
Flush LSN is monotonic, so the value dominates every copied page-LSN.
FSM/VM pages are exempt by design (freespace.c uses MarkBufferDirtyHint
without WAL) but aren't gated by minRecoveryPoint. Unlogged tables are out
of scope. Torn pages are closed by the existing full_page_writes=on
precondition.

Review comments
1. Commit message understates the fix. It only describes the page-header
symptom. The crash-safety property is the stronger argument and the one
that resolves the back-and-forth on which LSN to use. Suggest adding:
a. "Using the flush LSN is also crash-safe with respect to the source: the
insert LSN lives only in shared memory and can be lost on a source crash,
leaving the standby's minRecoveryPoint ahead of any LSN the source can
subsequently reach."
2. Code comment should explain why flush LSN is sufficient. The current "We
must replay to the last WAL flush location" doesn't say why. Suggest:
a. "Use the source's flush LSN as the target's minRecoveryPoint: every
WAL-logged page we copied has page-LSN <= source's flush LSN at copy time
(WAL-before-data), and flush LSN is monotonic. We avoid the insert LSN
because it can sit one page-header past a record's end at segment
boundaries (where no record will end), and it is not durable, a source
crash can leave flush LSN behind an insert LSN we already pinned."
3. Worth a comment in rewind_source.h that the callback must only be
invoked against a non-standby source, pg_current_wal_flush_lsn() errors out
under recovery.
4. No regression test. We can add a regression test under
src/bin/pg_rewind/t/.

Otherwise +1 on the v2 direction.

Regards,
Surya Poondla

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

    cca5507 <cca5507@qq.com> — 2026-06-16T04:46:18Z

    Hi,

Thanks for the comments!

> 1. Commit message understates the fix. It only describes the page-header symptom. The crash-safety property is the stronger argument and the one that resolves the > back-and-forth on which LSN to use. Suggest adding:
> a. "Using the flush LSN is also crash-safe with respect to the source: the insert LSN lives only in shared memory and can be lost on a source crash, leaving the standby's minRecoveryPoint ahead of any LSN the source can subsequently reach."
> 2. Code comment should explain why flush LSN is sufficient. The current "We must replay to the last WAL flush location" doesn't say why. Suggest:
> a. "Use the source's flush LSN as the target's minRecoveryPoint: every WAL-logged page we copied has page-LSN <= source's flush LSN at copy time (WAL-before-data), and flush LSN is monotonic. We avoid the insert LSN because it can sit one page-header past a record's end at segment boundaries (where no record will end), and it is not durable, a source crash can leave flush LSN behind an insert LSN we already pinned."
> 3. Worth a comment in rewind_source.h that the callback must only be invoked against a non-standby source, pg_current_wal_flush_lsn() errors out under recovery.

Fixed.

> 4. No regression test. We can add a regression test under src/bin/pg_rewind/t/.

Currently I don't have a good idea about the test, I will work on it later. Any help is welcome!

--
Regards,
ChangAo Chen

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

    surya poondla <suryapoondla4@gmail.com> — 2026-06-29T18:53:27Z

    Hi ChangAo,

Thanks for the v3, the commit message, in-line comment, and the
rewind_source.h note all look good

On the test front: I don't think a hang-detection test can be made
reliable. The bug requires the source's insert LSN to be exactly
segment_boundary + SizeOfXLogLongPHD with no further WAL activity, but
bgwriter's periodic LogStandbySnapshot emits a RUNNING_XACTS which can
advance the insert LSN
nondeterministically between pg_switch_wal() and the rewind. In my
reproduction bgwriter ended the hang after ~9s; that's the kind of timing
we don't want in CI.

The deterministic alternative is to parse pg_controldata on the target
after pg_rewind and assert minRecoveryPoint does not land
at "boundary + SizeOfXLogLongPHD". That's a direct check on the patched
behavior independent of source idleness or replay
timing. It doesn't exercise the integration property that the rewound node
reaches consistency without further upstream WAL.
So I am not sure if this testcase is a complete one in our scenario.


Regards,
Surya Poondla