Date: Fri, 3 Apr 2026 13:10:08 +0800
From: Adam Lee <adam8157@gmail.com>
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: pgsql-hackers@lists.postgresql.org,
	Michael Paquier <michael@paquier.xyz>
Subject: Re: [PATCH] Fix minRecoveryPoint not advanced past checkpoint in
 CreateRestartPoint
Message-ID: <ac9LsF7bL6UW75VM@MAC-CVW1VHW5R6>
References: <aczc9cxkr7SEHXV5@MAC-CVW1VHW5R6>
 <038d97bc-fbe4-4d99-b7a5-e468ef361123@iki.fi>
 <acz_NsifcL_IzMjn@MAC-CVW1VHW5R6>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="rlqg/1ddaRuGTcZI"
Content-Disposition: inline
In-Reply-To: <acz_NsifcL_IzMjn@MAC-CVW1VHW5R6>
Archived-At: 
 <https://www.postgresql.org/message-id/ac9LsF7bL6UW75VM%40MAC-CVW1VHW5R6>
Precedence: bulk


--rlqg/1ddaRuGTcZI
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

PATCH v2 removed the variable lastCheckPointEndPtr and refined the comments.

Thanks for reviewing.

-- 
Adam

--rlqg/1ddaRuGTcZI
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename=0001-Fix-minRecoveryPoint-not-advanced-past-checkpoint-in.patch

From f639b7c0bc76ab06ff5a05c26aceb1764955a849 Mon Sep 17 00:00:00 2001
From: Adam Lee <adam8157@gmail.com>
Date: Tue, 31 Mar 2026 18:43:53 +0800
Subject: [PATCH v2] Fix minRecoveryPoint not advanced past checkpoint in
 CreateRestartPoint

When recovery_target_action=shutdown triggers, the checkpointer performs
a shutdown restartpoint via CreateRestartPoint. If a new CHECKPOINT
record was replayed shortly before the recovery target, the restartpoint
advances minRecoveryPoint to the end of that CHECKPOINT record. And the
following replay doesn't advance minRecoveryPoint, it's assumed that
flushing the buffers will do that as a side-effect.

But no-op records replayed after the CHECKPOINT (such as RESTORE_POINT) do
not dirty any pages, so the minRecoveryPoint is not updated as expected.
As a result, minRecoveryPoint in pg_control ends up behind the actual
replay position. This does not cause a recovery correctness issue,
however the inaccurate pg_controldata "Minimum recovery ending location"
prevents users or tools from using this value to verify that recovery
has reached a specific restore point.

Fix by reading the current replay position from shared memory and
advancing minRecoveryPoint to match it. Since the replay position is
always at least as far as the checkpoint end, this also subsumes the
previous lastCheckPointEndPtr update.

Reproducer:
  CHECKPOINT; SELECT pg_create_restore_point('test_rp');
  -- recover with recovery_target_name + recovery_target_action=shutdown
  -- pg_controldata shows minRecoveryPoint 104 bytes behind
---
 src/backend/access/transam/xlog.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 2c1c6f88b74..ff9e373c4fa 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -7721,7 +7721,6 @@ bool
 CreateRestartPoint(int flags)
 {
 	XLogRecPtr	lastCheckPointRecPtr;
-	XLogRecPtr	lastCheckPointEndPtr;
 	CheckPoint	lastCheckPoint;
 	XLogRecPtr	PriorRedoPtr;
 	XLogRecPtr	receivePtr;
@@ -7737,7 +7736,6 @@ CreateRestartPoint(int flags)
 	/* Get a local copy of the last safe checkpoint record. */
 	SpinLockAcquire(&XLogCtl->info_lck);
 	lastCheckPointRecPtr = XLogCtl->lastCheckPointRecPtr;
-	lastCheckPointEndPtr = XLogCtl->lastCheckPointEndPtr;
 	lastCheckPoint = XLogCtl->lastCheckPoint;
 	SpinLockRelease(&XLogCtl->info_lck);
 
@@ -7864,15 +7862,32 @@ CreateRestartPoint(int flags)
 		 */
 		if (ControlFile->state == DB_IN_ARCHIVE_RECOVERY)
 		{
-			if (ControlFile->minRecoveryPoint < lastCheckPointEndPtr)
+			/*
+			 * Advance minRecoveryPoint to at least the current replay
+			 * position.  Normally this happens as a side effect of
+			 * flushing dirty buffers, but during a shutdown restartpoint
+			 * there may be records between the checkpoint and the
+			 * recovery target that didn't dirty any buffers (e.g. a
+			 * RESTORE_POINT record).  Without this, a shutdown triggered
+			 * by recovery_target_action leaves minRecoveryPoint behind
+			 * the actual replay position.
+			 */
 			{
-				ControlFile->minRecoveryPoint = lastCheckPointEndPtr;
-				ControlFile->minRecoveryPointTLI = lastCheckPoint.ThisTimeLineID;
+				XLogRecPtr	replayPtr;
+				TimeLineID	replayTLI;
 
-				/* update local copy */
-				LocalMinRecoveryPoint = ControlFile->minRecoveryPoint;
-				LocalMinRecoveryPointTLI = ControlFile->minRecoveryPointTLI;
+				replayPtr = GetCurrentReplayRecPtr(&replayTLI);
+				if (ControlFile->minRecoveryPoint < replayPtr)
+				{
+					ControlFile->minRecoveryPoint = replayPtr;
+					ControlFile->minRecoveryPointTLI = replayTLI;
+				}
 			}
+
+			/* update local copy */
+			LocalMinRecoveryPoint = ControlFile->minRecoveryPoint;
+			LocalMinRecoveryPointTLI = ControlFile->minRecoveryPointTLI;
+
 			if (flags & CHECKPOINT_IS_SHUTDOWN)
 				ControlFile->state = DB_SHUTDOWNED_IN_RECOVERY;
 		}
-- 
2.47.3


--rlqg/1ddaRuGTcZI--