On 2019-12-11 12:40, Leif Gunnar Erlandsen wrote:
Adding patch written for 13dev from git
"Michael Paquier" <mich...@paquier.xyz> skrev 1. desember 2019 kl. 03:08:
On Fri, Nov 22, 2019 at 11:26:59AM +0000, Leif Gunnar Erlandsen wrote:
No it does not. It works well to demonstrate its purpose though.
And it might be to stop with FATAL would be more correct.
This is still under active discussion. Please note that the latest
patch does not apply, so a rebase would be nice to have. I have moved
the patch to next CF, waiting on author.
I reworked your patch a bit. I changed the outcome to be an error, as
was discussed. I also added tests and documentation. Please take a look.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 0850de3922854ecdd4249f98ea854afd6ecc9ae2 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Tue, 14 Jan 2020 21:01:26 +0100
Subject: [PATCH v5] Fail if recovery target is not reached
Before, if a recovery target is configured, but the archive ended
before the target was reached, recovery would end and the server would
promote without further notice. That was deemed to be pretty wrong.
With this change, if the recovery target is not reached, it is a fatal
error.
Discussion:
https://www.postgresql.org/message-id/flat/993736dd3f1713ec1f63fc3b65383...@lako.no
---
doc/src/sgml/config.sgml | 5 +++
src/backend/access/transam/xlog.c | 19 +++++++++
src/test/perl/PostgresNode.pm | 33 +++++++++++++--
src/test/recovery/t/003_recovery_targets.pl | 45 ++++++++++++++++++++-
4 files changed, 98 insertions(+), 4 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 5d45b6f7cb..385ccc7196 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -3571,6 +3571,11 @@ <title>Recovery Target</title>
If <xref linkend="guc-hot-standby"/> is not enabled, a setting of
<literal>pause</literal> will act the same as
<literal>shutdown</literal>.
</para>
+ <para>
+ In any case, if a recovery target is configured but the archive
+ recovery ends before the target is reached, the server will shut down
+ with a fatal error.
+ </para>
</listitem>
</varlistentry>
diff --git a/src/backend/access/transam/xlog.c
b/src/backend/access/transam/xlog.c
index 7f4f784c0e..43a78e365d 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -7303,6 +7303,15 @@ StartupXLOG(void)
break;
}
}
+ else if (recoveryTarget != RECOVERY_TARGET_UNSET)
+ {
+ /*
+ * A recovery target was set but we got here
without the
+ * target being reached.
+ */
+ ereport(FATAL,
+ (errmsg("recovery ended before
configured recovery target was reached")));
+ }
/* Allow resource managers to do any required cleanup.
*/
for (rmid = 0; rmid <= RM_MAX_ID; rmid++)
@@ -7324,6 +7333,16 @@ StartupXLOG(void)
}
else
{
+ if (recoveryTarget != RECOVERY_TARGET_UNSET)
+ {
+ /*
+ * A recovery target was set but we got here
without the
+ * target being reached.
+ */
+ ereport(FATAL,
+ (errmsg("recovery ended before
configured recovery target was reached")));
+ }
+
/* there are no WAL records following the checkpoint */
ereport(LOG,
(errmsg("redo is not required")));
diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index 2e0cf4a2f3..be44e8784f 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -653,6 +653,9 @@ Restoring WAL segments from archives using restore_command
can be enabled
by passing the keyword parameter has_restoring => 1. This is disabled by
default.
+Is has_restoring is used, standby mode is used by default. To use
+recovery mode instead, pass the keyword parameter standby => 0.
+
The backup is copied, leaving the original unmodified. pg_hba.conf is
unconditionally set to enable replication connections.
@@ -669,6 +672,7 @@ sub init_from_backup
$params{has_streaming} = 0 unless defined $params{has_streaming};
$params{has_restoring} = 0 unless defined $params{has_restoring};
+ $params{standby} = 1 unless defined $params{standby};
print
"# Initializing node \"$node_name\" from backup \"$backup_name\" of
node \"$root_name\"\n";
@@ -699,7 +703,7 @@ port = $port
"unix_socket_directories = '$host'");
}
$self->enable_streaming($root_node) if $params{has_streaming};
- $self->enable_restoring($root_node) if $params{has_restoring};
+ $self->enable_restoring($root_node, $params{standby}) if
$params{has_restoring};
return;
}
@@ -939,7 +943,7 @@ primary_conninfo='$root_connstr'
# Internal routine to enable archive recovery command on a standby node
sub enable_restoring
{
- my ($self, $root_node) = @_;
+ my ($self, $root_node, $standby) = @_;
my $path = TestLib::perl2host($root_node->archive_dir);
my $name = $self->name;
@@ -961,7 +965,30 @@ sub enable_restoring
'postgresql.conf', qq(
restore_command = '$copy_command'
));
- $self->set_standby_mode();
+ if ($standby)
+ {
+ $self->set_standby_mode();
+ }
+ else
+ {
+ $self->set_recovery_mode();
+ }
+ return;
+}
+
+=pod
+
+=item $node->set_recovery_mode()
+
+Place recovery.signal file.
+
+=cut
+
+sub set_recovery_mode
+{
+ my ($self) = @_;
+
+ $self->append_conf('recovery.signal', '');
return;
}
diff --git a/src/test/recovery/t/003_recovery_targets.pl
b/src/test/recovery/t/003_recovery_targets.pl
index d8fbd50011..5e2cf217a6 100644
--- a/src/test/recovery/t/003_recovery_targets.pl
+++ b/src/test/recovery/t/003_recovery_targets.pl
@@ -3,7 +3,8 @@
use warnings;
use PostgresNode;
use TestLib;
-use Test::More tests => 8;
+use Test::More tests => 10;
+use Time::HiRes qw(usleep);
# Create and test a standby from given backup, with a certain recovery target.
# Choose $until_lsn later than the transaction commit that causes the row
@@ -145,3 +146,45 @@ sub test_recovery_standby
my $logfile = slurp_file($node_standby->logfile());
ok($logfile =~ qr/multiple recovery targets specified/,
'multiple conflicting settings');
+
+# Check behavior when recovery ends before target is reached
+
+$node_standby = get_new_node('standby_8');
+$node_standby->init_from_backup($node_master, 'my_backup',
+ has_restoring
=> 1, standby => 0);
+$node_standby->append_conf('postgresql.conf',
+ "recovery_target_name =
'does_not_exist'");
+
+run_log(['pg_ctl', '-D', $node_standby->data_dir,
+ '-l', $node_standby->logfile, 'start']);
+
+# wait up to 10 seconds for postgres to terminate
+foreach my $i (0..100)
+{
+ last if ! -f $node_standby->data_dir . '/postmaster.pid';
+ usleep(100_000);
+}
+$logfile = slurp_file($node_standby->logfile());
+ok($logfile =~ qr/FATAL: recovery ended before configured recovery target was
reached/,
+ 'recovery end before target reached is a fatal error');
+
+# also test the case of no redo needed (same result but different code path)
+my $node2 = get_new_node('node2');
+$node2->init(allows_streaming => 1);
+$node2->append_conf(
+ 'postgresql.conf', "recovery_target_name = 'does_not_exist'
+restore_command = 'false'
+hot_standby = off");
+$node2->append_conf('recovery.signal', '');
+
+run_log(['pg_ctl', '-D', $node2->data_dir,
+ '-l', $node2->logfile, 'start']);
+
+foreach my $i (0..100)
+{
+ last if ! -f $node2->data_dir . '/postmaster.pid';
+ usleep(100_000);
+}
+$logfile = slurp_file($node2->logfile());
+ok($logfile =~ qr/FATAL: recovery ended before configured recovery target was
reached/,
+ 'recovery end before target reached is a fatal error (no redo
required)');
--
2.24.1
