Suggest two small improvements for PITR.

From 3f5f03a942abf08763e9692b00c82f3bfd2c8ed4 Mon Sep 17 00:00:00 2001
From: Yura Sokolov <y.sokolov@postgrespro.ru>
Date: Tue, 18 Jun 2024 12:07:41 +0300
Subject: [PATCH v2 1/2] Restore ability to react on timestamp in
 XLOG_RESTORE_POINT

Looks like, reaction on XLOG_RESTORE_POINT's timestamp as a
recovery_target_time were lost during refactoring in
c945af80 "Refactor checking whether we've reached the recovery target."

Co-authored-by: Sergey Fukanchik <s.fukanchik@postgrespro.ru>
---
 src/backend/access/transam/xlogrecovery.c   | 43 +++++++++++++++++++++
 src/test/recovery/t/003_recovery_targets.pl | 19 ++++++++-
 2 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index b45b8331720..e1ba4452113 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -2783,6 +2783,49 @@ recoveryStopsAfter(XLogReaderState *record)
 		return true;
 	}
 
+	if (recoveryTarget == RECOVERY_TARGET_TIME &&
+		rmid == RM_XLOG_ID && getRecordTimestamp(record, &recordXtime))
+	{
+		bool		stops_here = false;
+
+		/*
+		 * Use same conditions as in recoveryStopsBefore for transaction
+		 * records to not override transactions time handling.
+		 */
+		if (recoveryTargetInclusive)
+			stops_here = recordXtime > recoveryTargetTime;
+		else
+			stops_here = recordXtime >= recoveryTargetTime;
+
+		if (stops_here)
+		{
+			recoveryStopAfter = true;
+			recoveryStopXid = InvalidTransactionId;
+			recoveryStopTime = recordXtime;
+			recoveryStopLSN = InvalidXLogRecPtr;
+			recoveryStopName[0] = '\0';
+
+			if (info == XLOG_RESTORE_POINT)
+			{
+				xl_restore_point *recordRestorePointData;
+
+				recordRestorePointData = (xl_restore_point *) XLogRecGetData(record);
+
+				ereport(LOG,
+						(errmsg("recovery stopping at restore point \"%.*s\", time %s",
+								MAXFNAMELEN, recordRestorePointData->rp_name,
+								timestamptz_to_str(recoveryStopTime))));
+			}
+			else
+			{
+				ereport(LOG,
+						(errmsg("recovery stopping at %s, time %s",
+								xlog_identify(info),
+								timestamptz_to_str(recoveryStopTime))));
+			}
+		}
+	}
+
 	if (rmid != RM_XACT_ID)
 		return false;
 
diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl
index 58241a68f0a..d5345c1bf1e 100644
--- a/src/test/recovery/t/003_recovery_targets.pl
+++ b/src/test/recovery/t/003_recovery_targets.pl
@@ -22,6 +22,7 @@ sub test_recovery_standby
 	my $recovery_params = shift;
 	my $num_rows = shift;
 	my $until_lsn = shift;
+	my $expect_in_log = shift;
 
 	my $node_standby = PostgreSQL::Test::Cluster->new($node_name);
 	$node_standby->init_from_backup($node_primary, 'my_backup',
@@ -45,6 +46,16 @@ sub test_recovery_standby
 	  $node_standby->safe_psql('postgres', "SELECT count(*) FROM tab_int");
 	is($result, qq($num_rows), "check standby content for $test_name");
 
+	if (defined $expect_in_log)
+	{
+		local $/;
+		open(my $FH, '<', $node_standby->logfile())
+			or die "Can't open standby nodes's log";
+		my $lines = <$FH>;
+		close($FH);
+		like($lines, qr/${expect_in_log}/i, "expecting \'$expect_in_log\' in log file for $test_name");
+	}
+
 	# Stop standby node
 	$node_standby->teardown_node;
 
@@ -93,8 +104,9 @@ $node_primary->safe_psql('postgres',
 my $recovery_name = "my_target";
 my $lsn4 =
   $node_primary->safe_psql('postgres', "SELECT pg_current_wal_lsn();");
-$node_primary->safe_psql('postgres',
-	"SELECT pg_create_restore_point('$recovery_name');");
+$ret = $node_primary->safe_psql('postgres',
+	"SELECT now(), pg_create_restore_point('$recovery_name');");
+my $recovery_restore_point_time = (split /\|/, $ret)[0];
 
 # And now for a recovery target LSN
 $node_primary->safe_psql('postgres',
@@ -121,6 +133,9 @@ test_recovery_standby('time', 'standby_3', $node_primary, \@recovery_params,
 @recovery_params = ("recovery_target_name = '$recovery_name'");
 test_recovery_standby('name', 'standby_4', $node_primary, \@recovery_params,
 	"4000", $lsn4);
+@recovery_params = ("recovery_target_time = '$recovery_restore_point_time'");
+test_recovery_standby('rp_time', 'standby_4_time', $node_primary, \@recovery_params,
+	"4000", $lsn4, "recovery stopping at restore point \"$recovery_name\"");
 @recovery_params = ("recovery_target_lsn = '$recovery_lsn'");
 test_recovery_standby('LSN', 'standby_5', $node_primary, \@recovery_params,
 	"5000", $lsn5);
-- 
2.43.0

From 5358def958af3ca0ed45a008dafafe5aaff290a0 Mon Sep 17 00:00:00 2001
From: Yura Sokolov <y.sokolov@postgrespro.ru>
Date: Tue, 18 Jun 2024 14:02:14 +0300
Subject: [PATCH v2 2/2] Add timestamp to XLOG_BACKUP_END.

It will allow to have exact recovery time for backup, so user may
use it as recovery_target_time.
---
 src/backend/access/rmgrdesc/xlogdesc.c      |  8 ++++---
 src/backend/access/transam/xlog.c           |  6 ++++--
 src/backend/access/transam/xlogrecovery.c   | 11 ++++++++++
 src/include/access/xlog_internal.h          |  7 +++++++
 src/test/recovery/t/003_recovery_targets.pl | 23 ++++++++++++++++++---
 src/tools/pgindent/typedefs.list            |  1 +
 6 files changed, 48 insertions(+), 8 deletions(-)

diff --git a/src/backend/access/rmgrdesc/xlogdesc.c b/src/backend/access/rmgrdesc/xlogdesc.c
index e455400716d..17702e73341 100644
--- a/src/backend/access/rmgrdesc/xlogdesc.c
+++ b/src/backend/access/rmgrdesc/xlogdesc.c
@@ -86,10 +86,12 @@ xlog_desc(StringInfo buf, XLogReaderState *record)
 	}
 	else if (info == XLOG_BACKUP_END)
 	{
-		XLogRecPtr	startpoint;
+		xl_backup_end xlrec;
 
-		memcpy(&startpoint, rec, sizeof(XLogRecPtr));
-		appendStringInfo(buf, "%X/%X", LSN_FORMAT_ARGS(startpoint));
+		memcpy(&xlrec, rec, sizeof(xl_backup_end));
+		appendStringInfo(buf, "%X/%X; time %s",
+						 LSN_FORMAT_ARGS(xlrec.startpoint),
+						 timestamptz_to_str(xlrec.end_time));
 	}
 	else if (info == XLOG_PARAMETER_CHANGE)
 	{
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 330e058c5f2..c82bb1a208b 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -9162,14 +9162,16 @@ do_pg_backup_stop(BackupState *state, bool waitforarchive)
 	}
 	else
 	{
+		xl_backup_end xlrec;
 		char	   *history_file;
 
 		/*
 		 * Write the backup-end xlog record
 		 */
+		xlrec.startpoint = state->startpoint;
+		xlrec.end_time = GetCurrentTimestamp();
 		XLogBeginInsert();
-		XLogRegisterData((char *) (&state->startpoint),
-						 sizeof(state->startpoint));
+		XLogRegisterData((char *) (&xlrec), sizeof(xlrec));
 		state->stoppoint = XLogInsert(RM_XLOG_ID, XLOG_BACKUP_END);
 
 		/*
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index e1ba4452113..96a41c7a449 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -2434,6 +2434,11 @@ getRecordTimestamp(XLogReaderState *record, TimestampTz *recordXtime)
 		*recordXtime = ((xl_restore_point *) XLogRecGetData(record))->rp_time;
 		return true;
 	}
+	if (rmid == RM_XLOG_ID && info == XLOG_BACKUP_END)
+	{
+		*recordXtime = ((xl_backup_end *) XLogRecGetData(record))->end_time;
+		return true;
+	}
 	if (rmid == RM_XACT_ID && (xact_info == XLOG_XACT_COMMIT ||
 							   xact_info == XLOG_XACT_COMMIT_PREPARED))
 	{
@@ -2816,6 +2821,12 @@ recoveryStopsAfter(XLogReaderState *record)
 								MAXFNAMELEN, recordRestorePointData->rp_name,
 								timestamptz_to_str(recoveryStopTime))));
 			}
+			else if (info == XLOG_BACKUP_END)
+			{
+				ereport(LOG,
+						(errmsg("recovery stopping at backup end, time %s",
+								timestamptz_to_str(recoveryStopTime))));
+			}
 			else
 			{
 				ereport(LOG,
diff --git a/src/include/access/xlog_internal.h b/src/include/access/xlog_internal.h
index 5161b72f281..18b03115273 100644
--- a/src/include/access/xlog_internal.h
+++ b/src/include/access/xlog_internal.h
@@ -289,6 +289,13 @@ typedef struct xl_restore_point
 	char		rp_name[MAXFNAMELEN];
 } xl_restore_point;
 
+/* backup end record */
+typedef struct xl_backup_end
+{
+	XLogRecPtr	startpoint;
+	TimestampTz end_time;
+} xl_backup_end;
+
 /* Overwrite of prior contrecord */
 typedef struct xl_overwrite_contrecord
 {
diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl
index d5345c1bf1e..3e2b161b503 100644
--- a/src/test/recovery/t/003_recovery_targets.pl
+++ b/src/test/recovery/t/003_recovery_targets.pl
@@ -114,9 +114,23 @@ $node_primary->safe_psql('postgres',
 my $lsn5 = my $recovery_lsn =
   $node_primary->safe_psql('postgres', "SELECT pg_current_wal_lsn()");
 
+# Try recovery target time for backup
 $node_primary->safe_psql('postgres',
 	"INSERT INTO tab_int VALUES (generate_series(5001,6000))");
 
+my $main_h = $node_primary->background_psql('postgres');
+$main_h->query_safe("SELECT pg_backup_start('test', true)");
+print("AFTER backup_start\n");
+my $lsn6 =
+	$node_primary->safe_psql('postgres', "SELECT pg_current_wal_lsn()");
+my $out = $main_h->query_safe("SELECT now(), pg_backup_stop(false);");
+print("AFTER backup_stop\n");
+my ($recovery_backup_time) = $out =~ /(20\d\d-\d\d-\d\d [\d:.+-]+)/;
+$main_h->quit();
+
+$node_primary->safe_psql('postgres',
+	"INSERT INTO tab_int VALUES (generate_series(6001,7000))");
+
 # Force archiving of WAL file
 $node_primary->safe_psql('postgres', "SELECT pg_switch_wal()");
 
@@ -139,6 +153,9 @@ test_recovery_standby('rp_time', 'standby_4_time', $node_primary, \@recovery_par
 @recovery_params = ("recovery_target_lsn = '$recovery_lsn'");
 test_recovery_standby('LSN', 'standby_5', $node_primary, \@recovery_params,
 	"5000", $lsn5);
+@recovery_params = ("recovery_target_time = '$recovery_backup_time'");
+test_recovery_standby('be_time', 'standby_6', $node_primary, \@recovery_params,
+	"6000", $lsn6, "recovery stopping at backup end");
 
 # Multiple targets
 #
@@ -151,9 +168,9 @@ test_recovery_standby('LSN', 'standby_5', $node_primary, \@recovery_params,
 	"recovery_target_name = ''",
 	"recovery_target_time = '$recovery_time'");
 test_recovery_standby('multiple overriding settings',
-	'standby_6', $node_primary, \@recovery_params, "3000", $lsn3);
+	'standby_7', $node_primary, \@recovery_params, "3000", $lsn3);
 
-my $node_standby = PostgreSQL::Test::Cluster->new('standby_7');
+my $node_standby = PostgreSQL::Test::Cluster->new('standby_8');
 $node_standby->init_from_backup($node_primary, 'my_backup',
 	has_restoring => 1);
 $node_standby->append_conf(
@@ -173,7 +190,7 @@ ok($logfile =~ qr/multiple recovery targets specified/,
 
 # Check behavior when recovery ends before target is reached
 
-$node_standby = PostgreSQL::Test::Cluster->new('standby_8');
+$node_standby = PostgreSQL::Test::Cluster->new('standby_9');
 $node_standby->init_from_backup(
 	$node_primary, 'my_backup',
 	has_restoring => 1,
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 61ad417cde6..fd043656702 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -4032,6 +4032,7 @@ worktable
 wrap
 ws_file_info
 ws_options
+xl_backup_end
 xl_brin_createidx
 xl_brin_desummarize
 xl_brin_insert
-- 
2.43.0