From 95cd5a72eb3adbaaabd58761115f4fb2129b8ac1 Mon Sep 17 00:00:00 2001
From: Srinath Reddy Sadipiralla <srinath2133@gmail.com>
Date: Sat, 29 Aug 2026 11:27:06 +0530
Subject: [PATCH 1/1] Fix pg_rewind file sync bypass and findLastCheckpoint
 boundary crash
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When a target cluster is cleanly shut down and a standby is promoted,
the target's WAL ends exactly at the divergence point. In this scenario
(target_wal_endrec == divergerec), pg_rewind previously assumed no
rewind was needed and exited early. While there are zero modified data 
blocks to extract, skipping the file sync phase leaves the target
missing timeline history files, non-WAL-logged configuration changes 
(e.g., postgresql.auto.conf), and a properly updated pg_control file.

Removing this check exposes another bug:
findLastCheckpoint() crashes with "invalid record length (expected 
at least 24, got 0)" when attempting to scan backward. This occurs
because divergerec points to the exact end of the target's WAL, but
the backward scanner expects a valid record header to start at that
exact LSN.

This patch fixes both issues:
1. In pg_rewind.c, the target_wal_endrec == divergerec early exit is
  removed. If timelines differ, file-level sync is strictly required.
2. In findLastCheckpoint(), if the initial backward read fails exactly
  at the forkptr due to unwritten WAL, it gracefully falls back to a
  forward scan starting from the pg_control checkpoint LSN to locate
  the last common checkpoint.

A TAP test is included to demonstrate the file-sync failure on master
using an ALTER SYSTEM command immediately after promotion.
---
 src/bin/pg_rewind/parsexlog.c             | 168 +++++++++++++++-------
 src/bin/pg_rewind/pg_rewind.c             |  24 ++--
 src/bin/pg_rewind/pg_rewind.h             |   2 +-
 src/bin/pg_rewind/t/012_equal_lsn_sync.pl |  76 ++++++++++
 4 files changed, 207 insertions(+), 63 deletions(-)
 create mode 100644 src/bin/pg_rewind/t/012_equal_lsn_sync.pl

diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c
index 023e23b063c..cb578d2a44b 100644
--- a/src/bin/pg_rewind/parsexlog.c
+++ b/src/bin/pg_rewind/parsexlog.c
@@ -167,9 +167,9 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, int tliIndex,
 void
 findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
 				   XLogRecPtr *lastchkptrec, TimeLineID *lastchkpttli,
-				   XLogRecPtr *lastchkptredo, const char *restoreCommand)
+				   XLogRecPtr *lastchkptredo, const char *restoreCommand, 
+                   XLogRecPtr cntrlfilechkptrec)
 {
-	/* Walk backwards, starting from the given record */
 	XLogRecord *record;
 	XLogRecPtr	searchptr;
 	XLogReaderState *xlogreader;
@@ -177,6 +177,7 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
 	XLogPageReadPrivate private;
 	XLogSegNo	current_segno = 0;
 	TimeLineID	current_tli = 0;
+	bool		fallback_to_forward = false;
 
 	/*
 	 * The given fork pointer points to the end of the last common record,
@@ -200,66 +201,137 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, int tliIndex,
 	if (xlogreader == NULL)
 		pg_fatal("out of memory while allocating a WAL reading processor");
 
+	/* 
+     * Attempt the standard backward scan first. 
+     */
 	searchptr = forkptr;
-	for (;;)
-	{
-		uint8		info;
+	XLogBeginRead(xlogreader, searchptr);
+	record = XLogReadRecord(xlogreader, &errormsg);
 
-		XLogBeginRead(xlogreader, searchptr);
-		record = XLogReadRecord(xlogreader, &errormsg);
+	if (record == NULL)
+	{
+		/*
+		 * If we fail to read exactly at the forkptr, we assume the target crashed 
+		 * exactly at a record boundary and the WAL is padded with zeroes. 
+		 * Instead of crashing pg_rewind, we fallback to a forward scan.
+		 */
+		fallback_to_forward = true;
+	}
 
-		if (record == NULL)
+	if (!fallback_to_forward)
+	{
+		/* We successfully read the first record; proceed with backward scan */
+		for (;;)
 		{
-			if (errormsg)
-				pg_fatal("could not find previous WAL record at %X/%08X: %s",
-						 LSN_FORMAT_ARGS(searchptr),
-						 errormsg);
-			else
-				pg_fatal("could not find previous WAL record at %X/%08X",
-						 LSN_FORMAT_ARGS(searchptr));
-		}
+			uint8		info;
 
-		/* Detect if a new WAL file has been opened */
-		if (xlogreader->seg.ws_tli != current_tli ||
-			xlogreader->seg.ws_segno != current_segno)
-		{
-			char		xlogfname[MAXFNAMELEN];
+			if (record == NULL)
+			{
+				/* A failure mid-scan means real corruption, so we error out */
+				if (errormsg)
+					pg_fatal("could not find previous WAL record at %X/%08X: %s",
+							 LSN_FORMAT_ARGS(searchptr), errormsg);
+				else
+					pg_fatal("could not find previous WAL record at %X/%08X",
+							 LSN_FORMAT_ARGS(searchptr));
+			}
 
-			snprintf(xlogfname, MAXFNAMELEN, XLOGDIR "/");
+			/* Detect if a new WAL file has been opened */
+			if (xlogreader->seg.ws_tli != current_tli ||
+				xlogreader->seg.ws_segno != current_segno)
+			{
+				char		xlogfname[MAXFNAMELEN];
+
+				snprintf(xlogfname, MAXFNAMELEN, XLOGDIR "/");
+				current_tli = xlogreader->seg.ws_tli;
+				current_segno = xlogreader->seg.ws_segno;
+				XLogFileName(xlogfname + sizeof(XLOGDIR),
+							 current_tli, current_segno, WalSegSz);
+				keepwal_add_entry(xlogfname);
+			}
 
-			/* update current values */
-			current_tli = xlogreader->seg.ws_tli;
-			current_segno = xlogreader->seg.ws_segno;
+			/* Check if it is a valid checkpoint record. */
+			info = XLogRecGetInfo(xlogreader) & ~XLR_INFO_MASK;
+			if (searchptr < forkptr &&
+				XLogRecGetRmid(xlogreader) == RM_XLOG_ID &&
+				(info == XLOG_CHECKPOINT_SHUTDOWN ||
+				 info == XLOG_CHECKPOINT_ONLINE))
+			{
+				CheckPoint	checkPoint;
 
-			XLogFileName(xlogfname + sizeof(XLOGDIR),
-						 current_tli, current_segno, WalSegSz);
+				memcpy(&checkPoint, XLogRecGetData(xlogreader), sizeof(CheckPoint));
+				*lastchkptrec = searchptr;
+				*lastchkpttli = checkPoint.ThisTimeLineID;
+				*lastchkptredo = checkPoint.redo;
+				break;
+			}
 
-			/* Track this filename as one to not remove */
-			keepwal_add_entry(xlogfname);
+			/* Walk backwards to previous record. */
+			searchptr = record->xl_prev;
+			XLogBeginRead(xlogreader, searchptr);
+			record = XLogReadRecord(xlogreader, &errormsg);
 		}
+	}
+	else
+	{
+		/* 
+         * Fallback. Scan forward from the control file's last checkpoint. 
+         */
+		searchptr = cntrlfilechkptrec;
+		XLogBeginRead(xlogreader, searchptr);
 
-		/*
-		 * Check if it is a checkpoint record. This checkpoint record needs to
-		 * be the latest checkpoint before WAL forked and not the checkpoint
-		 * where the primary has been stopped to be rewound.
-		 */
-		info = XLogRecGetInfo(xlogreader) & ~XLR_INFO_MASK;
-		if (searchptr < forkptr &&
-			XLogRecGetRmid(xlogreader) == RM_XLOG_ID &&
-			(info == XLOG_CHECKPOINT_SHUTDOWN ||
-			 info == XLOG_CHECKPOINT_ONLINE))
+		for (;;)
 		{
-			CheckPoint	checkPoint;
+			uint8		info;
 
-			memcpy(&checkPoint, XLogRecGetData(xlogreader), sizeof(CheckPoint));
-			*lastchkptrec = searchptr;
-			*lastchkpttli = checkPoint.ThisTimeLineID;
-			*lastchkptredo = checkPoint.redo;
-			break;
-		}
+			record = XLogReadRecord(xlogreader, &errormsg);
+
+			if (record == NULL)
+			{
+				if (errormsg)
+					pg_fatal("could not read WAL record during forward scan at %X/%08X: %s",
+							 LSN_FORMAT_ARGS(searchptr), errormsg);
+				else
+					pg_fatal("could not read WAL record during forward scan at %X/%08X",
+							 LSN_FORMAT_ARGS(searchptr));
+			}
+
+			/* Update searchptr to the start of the record we just read */
+			searchptr = xlogreader->ReadRecPtr;
+
+			/* Detect if a new WAL file has been opened */
+			if (xlogreader->seg.ws_tli != current_tli ||
+				xlogreader->seg.ws_segno != current_segno)
+			{
+				char		xlogfname[MAXFNAMELEN];
+
+				snprintf(xlogfname, MAXFNAMELEN, XLOGDIR "/");
+				current_tli = xlogreader->seg.ws_tli;
+				current_segno = xlogreader->seg.ws_segno;
+				XLogFileName(xlogfname + sizeof(XLOGDIR),
+							 current_tli, current_segno, WalSegSz);
+				keepwal_add_entry(xlogfname);
+			}
 
-		/* Walk backwards to previous record. */
-		searchptr = record->xl_prev;
+			/* Check if it is a checkpoint record. Update pointers iteratively. */
+			info = XLogRecGetInfo(xlogreader) & ~XLR_INFO_MASK;
+			if (searchptr < forkptr &&
+				XLogRecGetRmid(xlogreader) == RM_XLOG_ID &&
+				(info == XLOG_CHECKPOINT_SHUTDOWN ||
+				 info == XLOG_CHECKPOINT_ONLINE))
+			{
+				CheckPoint	checkPoint;
+
+				memcpy(&checkPoint, XLogRecGetData(xlogreader), sizeof(CheckPoint));
+				*lastchkptrec = searchptr;
+				*lastchkpttli = checkPoint.ThisTimeLineID;
+				*lastchkptredo = checkPoint.redo;
+			}
+
+			/* If we've reached or passed the divergence point, we are done */
+			if (xlogreader->EndRecPtr >= forkptr)
+				break;
+		}
 	}
 
 	XLogReaderFree(xlogreader);
diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index 2e86fd158d0..fdeced703cb 100644
--- a/src/bin/pg_rewind/pg_rewind.c
+++ b/src/bin/pg_rewind/pg_rewind.c
@@ -439,22 +439,18 @@ main(int argc, char **argv)
 			target_wal_endrec = chkptendrec;
 		}
 
+		pg_log_info("target WAL ends at %X/%08X", LSN_FORMAT_ARGS(target_wal_endrec));
+
 		/*
-		 * Check for the possibility that the target is in fact a direct
-		 * ancestor of the source. In that case, there is no divergent history
-		 * in the target that needs rewinding.
+		 * The target WAL cannot end before the divergence point.
+		 * If target_wal_endrec == divergerec, the target wrote no WAL past the 
+		 * fork point. However, because the timelines diverged, we must still 
+		 * synchronize non-WAL files and fetch the new timeline history.
 		 */
-		if (target_wal_endrec > divergerec)
-		{
-			rewind_needed = true;
-		}
-		else
-		{
-			/* the last common checkpoint record must be part of target WAL */
-			Assert(target_wal_endrec == divergerec);
+		if (target_wal_endrec < divergerec)
+			pg_fatal("target WAL ends before the divergence point");
 
-			rewind_needed = false;
-		}
+		rewind_needed = true;
 	}
 
 	if (!rewind_needed)
@@ -471,7 +467,7 @@ main(int argc, char **argv)
 	keepwal_init();
 
 	findLastCheckpoint(datadir_target, divergerec, lastcommontliIndex,
-					   &chkptrec, &chkpttli, &chkptredo, restore_command);
+					   &chkptrec, &chkpttli, &chkptredo, restore_command, ControlFile_target.checkPoint);
 	pg_log_info("rewinding from last common checkpoint at %X/%08X on timeline %u",
 				LSN_FORMAT_ARGS(chkptrec), chkpttli);
 
diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h
index 9a981f7f246..bc3f700a76e 100644
--- a/src/bin/pg_rewind/pg_rewind.h
+++ b/src/bin/pg_rewind/pg_rewind.h
@@ -39,7 +39,7 @@ extern void findLastCheckpoint(const char *datadir, XLogRecPtr forkptr,
 							   int tliIndex,
 							   XLogRecPtr *lastchkptrec, TimeLineID *lastchkpttli,
 							   XLogRecPtr *lastchkptredo,
-							   const char *restoreCommand);
+							   const char *restoreCommand, XLogRecPtr cntrlfilechkptrec);
 extern XLogRecPtr readOneRecord(const char *datadir, XLogRecPtr ptr,
 								int tliIndex, const char *restoreCommand);
 
diff --git a/src/bin/pg_rewind/t/012_equal_lsn_sync.pl b/src/bin/pg_rewind/t/012_equal_lsn_sync.pl
new file mode 100644
index 00000000000..906f6ffdb4a
--- /dev/null
+++ b/src/bin/pg_rewind/t/012_equal_lsn_sync.pl
@@ -0,0 +1,76 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+# Initialize and start the primary node (Node A)
+my $node_a = PostgreSQL::Test::Cluster->new('node_a');
+$node_a->init(allows_streaming => 1);
+$node_a->start;
+
+# Initialize the standby node (Node B) from a backup of Node A
+my $node_b = PostgreSQL::Test::Cluster->new('node_b');
+$node_a->backup('my_backup');
+$node_b->init_from_backup($node_a, 'my_backup', has_streaming => 1);
+$node_b->start;
+
+# Wait for the standby to catch up to ensure WAL is identical
+$node_a->wait_for_catchup($node_b, 'replay', $node_a->lsn('insert'));
+
+# stop NODE A cleanly. 
+# This is the critical step to trigger the bug. A clean shutdown writes a 
+# shutdown checkpoint. Node A's WAL now ends exactly at this LSN.
+$node_a->stop;
+
+# Promote Node B. 
+# It writes an end-of-recovery record at the exact LSN where Node A stopped.
+# divergerec will now perfectly equal target_wal_endrec.
+$node_b->promote;
+
+# Make a non-WAL-logged change on the new primary (Node B).
+# ALTER SYSTEM modifies postgresql.auto.conf but generates no WAL.
+$node_b->safe_psql('postgres', "ALTER SYSTEM SET work_mem = '50MB';");
+
+# Run pg_rewind using run_command to capture all output
+my ($stdout, $stderr) = run_command(
+	[
+		'pg_rewind', '--debug',
+		'--source-server' => $node_b->connstr,
+		'--target-pgdata' => $node_a->data_dir,
+		'--no-sync'
+	]
+);
+
+my ($divergerec) = $stderr =~ /servers diverged at WAL location ([A-F0-9X\/]+) on timeline/;
+my ($target_wal_end) = $stderr =~ /target WAL ends at ([A-F0-9X\/]+)/;
+
+ok(defined $divergerec, "Found divergence LSN in logs: $divergerec");
+ok(defined $target_wal_end, "Found target WAL end LSN in logs: $target_wal_end");
+
+# It asserts that the test successfully triggered the exact boundary condition.
+is(
+	$target_wal_end, 
+	$divergerec, 
+	'Target WAL ends exactly at the divergence point (0 WAL changes)'
+);
+
+# If pg_rewind wrongly assumes it can skip the sync, it prints this exact line.
+# We want to ensure it DOES NOT print this line.
+unlike(
+	$stderr,
+	qr/no rewind required/,
+	'pg_rewind correctly recognized that differing timelines require a rewind'
+);
+
+# Verify the file sync actually occurred
+my $auto_conf_target = slurp_file($node_a->data_dir . '/postgresql.auto.conf');
+like(
+	$auto_conf_target,
+	qr/work_mem = '50MB'/,
+	'postgresql.auto.conf was synchronized from the source even WAL LSNs matched'
+);
+
+done_testing();
\ No newline at end of file
-- 
2.43.0

