Allow users to choose what happens when recovery target is not reached

From 10a103d543145d110f654fbeaba926b2157f6600 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Fri, 12 Nov 2021 09:50:09 +0000
Subject: [PATCH v1] Allow users to choose what happens when recovery target is
 not reached

Currently, the server shuts down with a FATAL error (added by commit
dc78866) when the recovery target isn't reached. This can cause a
server availability problem, especially in case of disaster
recovery (geo restores) where the primary was down and the user is
doing a PITR on a server lying in another region where it had missed
to receive few of the last WAL files required to reach the recovery
target. In this case, users might want the server to be available
rather than a no server. With the commit [1], there's no way to achieve
what users wanted.

This patch adds a new GUC so that users can choose either to emit
a warning and promote or shutdown with FATAL error (as default)
when recovery target isn't reached. In reality, users can choose
to shutdown with FATAL error, if strict consistency is the necessity,
otherwise they can choose to get promoted, if availability is
preferred.
---
 doc/src/sgml/config.sgml                      | 28 ++++++++++-
 src/backend/access/transam/xlog.c             | 30 ++++++++++-
 src/backend/utils/misc/guc.c                  | 11 ++++
 src/backend/utils/misc/postgresql.conf.sample |  3 +-
 src/include/access/xlog.h                     |  1 +
 src/include/access/xlog_internal.h            |  9 ++++
 src/test/recovery/t/003_recovery_targets.pl   | 50 ++++++++++++++++++-
 7 files changed, 126 insertions(+), 6 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 3f806740d5..8641f759ec 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -4017,8 +4017,32 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"'  # Windows
        </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.
+        recovery ends before the target is reached, the server, depending on
+        <xref linkend="guc-recovery-end-before-target-action"/> parameter, will
+        either shutdown with a fatal error or emit a warning and promote.
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry id="guc-recovery-end-before-target-action"
+                   xreflabel="recovery_end_before_target_action">
+      <term><varname>recovery_end_before_target_action</varname> (<type>enum</type>)
+      <indexterm>
+        <primary><varname>recovery_end_before_target_action</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        Specifies what action the server should take when the recovery target is
+        not reached. The default is <literal>shutdown</literal>, which will stop
+        the server. <literal>promote</literal> means the recovery process will
+        finish and the server will start to accept connections. This parameter
+        has no effect if no recovery target is set.
+       </para>
+       <para>
+        The intended use of the <literal>promote</literal> setting is to allow
+        user to have the server available at least.
+       </para>
        </para>
       </listitem>
      </varlistentry>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index e073121a7e..d3ebf48715 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -184,6 +184,12 @@ const struct config_enum_entry recovery_target_action_options[] = {
 	{NULL, 0, false}
 };
 
+const struct config_enum_entry recovery_end_before_target_action_options[] = {
+	{"shutdown", RECOVERY_END_BEFORE_TARGET_ACTION_SHUTDOWN, false},
+	{"promote", RECOVERY_END_BEFORE_TARGET_ACTION_PROMOTE, false},
+	{NULL, 0, false}
+};
+
 /*
  * Statistics for current checkpoint are collected in this global struct.
  * Because only the checkpointer or a stand-alone backend can perform
@@ -273,6 +279,7 @@ char	   *archiveCleanupCommand = NULL;
 RecoveryTargetType recoveryTarget = RECOVERY_TARGET_UNSET;
 bool		recoveryTargetInclusive = true;
 int			recoveryTargetAction = RECOVERY_TARGET_ACTION_PAUSE;
+int			recoveryEndBeforeTargetAction = RECOVERY_END_BEFORE_TARGET_ACTION_SHUTDOWN;
 TransactionId recoveryTargetXid;
 char	   *recovery_target_time_string;
 static TimestampTz recoveryTargetTime;
@@ -7852,8 +7859,27 @@ StartupXLOG(void)
 		if (ArchiveRecoveryRequested &&
 			recoveryTarget != RECOVERY_TARGET_UNSET &&
 			!reachedRecoveryTarget)
-			ereport(FATAL,
-					(errmsg("recovery ended before configured recovery target was reached")));
+		{
+			switch (recoveryEndBeforeTargetAction)
+			{
+				case RECOVERY_END_BEFORE_TARGET_ACTION_SHUTDOWN:
+					ereport(FATAL,
+							(errmsg("recovery ended before configured recovery target was reached")));
+					break;
+				case RECOVERY_END_BEFORE_TARGET_ACTION_PROMOTE:
+					/*
+					 * Do not shutdown the server but issue a warning and
+					 * promote so that the user can have it available at least.
+					 * Note that it is the behaviour chosen by the user and the
+					 * server is not guaranteed to be in a consistent state
+					 * though.
+					 */
+					ereport(WARNING,
+							(errmsg("recovery ended before configured recovery target was reached, but promoting the server as parameter \"%s\" is set to \"%s\"",
+							"recovery_end_before_target_action", "promote")));
+					break;
+			}
+		}
 	}
 
 	/*
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index e91d5a3cfd..f239e9a5a6 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -563,6 +563,7 @@ static const struct config_enum_entry wal_compression_options[] = {
 extern const struct config_enum_entry wal_level_options[];
 extern const struct config_enum_entry archive_mode_options[];
 extern const struct config_enum_entry recovery_target_action_options[];
+extern const struct config_enum_entry recovery_end_before_target_action_options[];
 extern const struct config_enum_entry sync_method_options[];
 extern const struct config_enum_entry dynamic_shared_memory_options[];
 
@@ -4833,6 +4834,16 @@ static struct config_enum ConfigureNamesEnum[] =
 		NULL, NULL, NULL
 	},
 
+	{
+		{"recovery_end_before_target_action", PGC_POSTMASTER, WAL_RECOVERY_TARGET,
+			gettext_noop("Sets the action to perform when the recovery target is not reached."),
+			NULL
+		},
+		&recoveryEndBeforeTargetAction,
+		RECOVERY_END_BEFORE_TARGET_ACTION_SHUTDOWN, recovery_end_before_target_action_options,
+		NULL, NULL, NULL
+	},
+
 	{
 		{"trace_recovery_messages", PGC_SIGHUP, DEVELOPER_OPTIONS,
 			gettext_noop("Enables logging of recovery-related debugging information."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 1cbc9feeb6..2697180803 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -286,7 +286,8 @@
 				# (change requires restart)
 #recovery_target_action = 'pause'	# 'pause', 'promote', 'shutdown'
 				# (change requires restart)
-
+#recovery_end_before_target_action = 'shutdown'	# 'promote', 'shutdown'
+				# (change requires restart)
 
 #------------------------------------------------------------------------------
 # REPLICATION
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 898df2ee03..75d3500068 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -83,6 +83,7 @@ extern char *recoveryEndCommand;
 extern char *archiveCleanupCommand;
 extern bool recoveryTargetInclusive;
 extern int	recoveryTargetAction;
+extern int	recoveryEndBeforeTargetAction;
 extern int	recovery_min_apply_delay;
 extern char *PrimaryConnInfo;
 extern char *PrimarySlotName;
diff --git a/src/include/access/xlog_internal.h b/src/include/access/xlog_internal.h
index c0da76cab4..bc2caecd53 100644
--- a/src/include/access/xlog_internal.h
+++ b/src/include/access/xlog_internal.h
@@ -287,6 +287,15 @@ typedef enum
 	RECOVERY_TARGET_ACTION_SHUTDOWN
 }			RecoveryTargetAction;
 
+/*
+ * Recovery end before target action.
+ */
+typedef enum
+{
+	RECOVERY_END_BEFORE_TARGET_ACTION_SHUTDOWN,
+	RECOVERY_END_BEFORE_TARGET_ACTION_PROMOTE
+}			RecoveryEndBeforeTargetAction;
+
 /*
  * Method table for resource managers.
  *
diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl
index 0d0636b85c..9f45b38ff5 100644
--- a/src/test/recovery/t/003_recovery_targets.pl
+++ b/src/test/recovery/t/003_recovery_targets.pl
@@ -6,7 +6,7 @@ use strict;
 use warnings;
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
-use Test::More tests => 9;
+use Test::More tests => 11;
 use Time::HiRes qw(usleep);
 
 # Create and test a standby from given backup, with a certain recovery target.
@@ -182,3 +182,51 @@ $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');
+
+# Check for a pattern in the logs associated to one format.
+sub check_server_logs
+{
+	local $Test::Builder::Level = $Test::Builder::Level + 1;
+
+	my $node      = shift;
+	my $pattern   = shift;
+	my $test_name = shift;
+
+	my $max_attempts = 180 * 10;
+
+	my $logcontents;
+	for (my $attempts = 0; $attempts < $max_attempts; $attempts++)
+	{
+		$logcontents = slurp_file($node->logfile());
+		last if $logcontents =~ m/$pattern/;
+		usleep(100_000);
+	}
+
+	like($logcontents, qr/$pattern/, "check server log for $test_name");
+	return;
+}
+
+# Check behavior when recovery ends before target is reached but
+# recovery_end_before_target_action is set to 'promote'
+
+$node_standby = PostgreSQL::Test::Cluster->new('standby_9');
+$node_standby->init_from_backup(
+	$node_primary, 'my_backup',
+	has_restoring => 1,
+	standby       => 0);
+$node_standby->append_conf('postgresql.conf',
+	"recovery_target_name = 'does_not_exist'
+	 recovery_end_before_target_action = 'promote'");
+
+$node_standby->start;
+
+my $msg = 'WARNING: .* recovery ended before configured recovery target was reached, but promoting the server as parameter "recovery_end_before_target_action" is set to "promote"';
+my $test_name = 'recovery ended before message with recovery_end_before_target_action set to promote';
+check_server_logs($node_standby, $msg, $test_name);
+
+$msg = 'LOG: .* database system is ready to accept connections';
+$test_name = 'database system is ready to accept connections message with recovery_end_before_target_action set to promote';
+check_server_logs($node_standby, $msg, $test_name);
+
+# Stop standby node
+$node_standby->teardown_node;
-- 
2.25.1