11.01.2024 19:58, Yura Sokolov пишет:
Good day, hackers.
Here I am to suggest two small improvements to Point In Time Recovery.
First is ability to recover recovery-target-time with timestamp stored
in XLOG_RESTORE_POINT. Looks like historically this ability did exist
and were removed unintentionally during refactoring at commit [1]
c945af80 "Refactor checking whether we've reached the recovery target."
Second is extending XLOG_BACKUP_END record with timestamp, therefore
backup will have its own timestamp as well. It is backward compatible
change since there were no record length check before.
Both changes slightly helps in mostly idle systems, when between several
backups may happens no commits at all, so there's no timestamp to
recover to.
Attached sample patches are made in reverse order:
- XLOG_BACKUP_END then XLOG_RESTORE_POINT.
Second patch made by colleague by my idea.
Publishing for both is permitted.
If idea is accepted, patches for tests will be applied as well.
[1]
https://git.postgresql.org/gitweb/?p=postgresql.git;a=patch;h=c945af80
Good day.
Here're reordered and rebased patches with tests.
Now first patch is for XLOG_RETORE_POINT, and second one adds timestamp
to XLOG_BACKUP_END.
Btw, there's other thread by Simon Riggs with additions to
getRecordTimestamp:
https://www.postgresql.org/message-id/flat/CANbhV-F%2B8%3DY%3DcfurfD2hjoWVUvTk-Ot9BJdw2Myc%3Dst3TsZy9g%40mail.gmail.com
I didn't rush to adsorb it, because I'm not recoveryTargetUseOriginTime.
Though reaction on XLOG_END_OF_RECOVERY's timestamp is easily could be
copied from.
I believe, to react on XLOG_CHECKPOINT_ONLINE/XLOG_CHECKPOINT_SHUTDOWN
the CheckPoint.time field should be changed from `pg_time_t` to
`TimestampTz` type, since otherwise it interfere hard with
"inclusive"-ness of recovery_target_time.
-----
regards,
Yura
From 3f5f03a942abf08763e9692b00c82f3bfd2c8ed4 Mon Sep 17 00:00:00 2001
From: Yura Sokolov <y.soko...@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.fukanc...@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.soko...@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
