Hello
Updated patch attached:
- added testcase to verify database does not start with multiple recovery
targets
- recovery_target = immediate was replaced with recovery_target_immediate bool
GUC
thank you!
regards, Sergei
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index db1a2d4..49caa0c 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -3217,19 +3217,19 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
<para>
By default, recovery will recover to the end of the WAL log. The
following parameters can be used to specify an earlier stopping point.
- At most one of <varname>recovery_target</varname>,
+ At most one of <varname>recovery_target_immediate</varname>,
<varname>recovery_target_lsn</varname>, <varname>recovery_target_name</varname>,
<varname>recovery_target_time</varname>, or <varname>recovery_target_xid</varname>
can be used; if more than one of these is specified in the configuration
- file, the last entry will be used.
+ file, a fatal error will be raised.
These parameters can only be set at server start.
</para>
<variablelist>
- <varlistentry id="guc-recovery-target" xreflabel="recovery_target">
- <term><varname>recovery_target</varname><literal> = 'immediate'</literal>
+ <varlistentry id="guc-recovery-target-immediate" xreflabel="recovery_target_immediate">
+ <term><varname>recovery_target_immediate</varname> (<type>boolean</type>)
<indexterm>
- <primary><varname>recovery_target</varname> configuration parameter</primary>
+ <primary><varname>recovery_target_immediate</varname> configuration parameter</primary>
</indexterm>
</term>
<listitem>
@@ -3239,10 +3239,6 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
from an online backup, this means the point where taking the backup
ended.
</para>
- <para>
- Technically, this is a string parameter, but <literal>'immediate'</literal>
- is currently the only allowed value.
- </para>
</listitem>
</varlistentry>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index c80b14e..cd29606 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -261,13 +261,21 @@ static bool restoredFromArchive = false;
static char *replay_image_masked = NULL;
static char *master_image_masked = NULL;
+/* recovery_target* original GUC values */
+char *recovery_target_string;
+char *recovery_target_xid_string;
+char *recovery_target_time_string;
+char *recovery_target_name_string;
+char *recovery_target_lsn_string;
+
/* options formerly taken from recovery.conf for archive recovery */
char *recoveryRestoreCommand = NULL;
char *recoveryEndCommand = NULL;
char *archiveCleanupCommand = NULL;
-RecoveryTargetType recoveryTarget = RECOVERY_TARGET_UNSET;
+static RecoveryTargetType recoveryTarget = RECOVERY_TARGET_UNSET;
bool recoveryTargetInclusive = true;
int recoveryTargetAction = RECOVERY_TARGET_ACTION_PAUSE;
+bool recoveryTargetImmediate;
TransactionId recoveryTargetXid;
TimestampTz recoveryTargetTime;
char *recoveryTargetName;
@@ -5381,9 +5389,42 @@ readRecoverySignalFile(void)
static void
validateRecoveryParameters(void)
{
+ uint8 targetSettingsCount = 0;
+
if (!ArchiveRecoveryRequested)
return;
+ /* Check for mutually exclusive target actions */
+ if (recoveryTargetImmediate)
+ {
+ ++targetSettingsCount;
+ recoveryTarget = RECOVERY_TARGET_IMMEDIATE;
+ }
+ if (strcmp(recovery_target_name_string, "") != 0)
+ {
+ ++targetSettingsCount;
+ recoveryTarget = RECOVERY_TARGET_NAME;
+ }
+ if (strcmp(recovery_target_lsn_string, "") != 0)
+ {
+ ++targetSettingsCount;
+ recoveryTarget = RECOVERY_TARGET_LSN;
+ }
+ if (strcmp(recovery_target_xid_string, "") != 0)
+ {
+ ++targetSettingsCount;
+ recoveryTarget = RECOVERY_TARGET_XID;
+ }
+ if (strcmp(recovery_target_time_string, "") != 0)
+ {
+ ++targetSettingsCount;
+ recoveryTarget = RECOVERY_TARGET_TIME;
+ }
+ if (targetSettingsCount > 1)
+ ereport(FATAL,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("can not specify multiple recovery targets")));
+
/*
* Check for compulsory parameters
*/
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 6497393..9d6879c 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -198,8 +198,6 @@ static const char *show_log_file_mode(void);
static const char *show_data_directory_mode(void);
static bool check_recovery_target_timeline(char **newval, void **extra, GucSource source);
static void assign_recovery_target_timeline(const char *newval, void *extra);
-static bool check_recovery_target(char **newval, void **extra, GucSource source);
-static void assign_recovery_target(const char *newval, void *extra);
static bool check_recovery_target_xid(char **newval, void **extra, GucSource source);
static void assign_recovery_target_xid(const char *newval, void *extra);
static bool check_recovery_target_time(char **newval, void **extra, GucSource source);
@@ -549,12 +547,6 @@ static bool data_checksums;
static bool integer_datetimes;
static bool assert_enabled;
static char *recovery_target_timeline_string;
-static char *recovery_target_string;
-static char *recovery_target_xid_string;
-static char *recovery_target_time_string;
-static char *recovery_target_name_string;
-static char *recovery_target_lsn_string;
-
/* should be static, but commands/variable.c needs to get at this */
char *role_string;
@@ -1674,6 +1666,16 @@ static struct config_bool ConfigureNamesBool[] =
},
{
+ {"recovery_target_immediate", PGC_POSTMASTER, WAL_RECOVERY_TARGET,
+ gettext_noop("End recovery as soon as a consistent state is reached."),
+ NULL
+ },
+ &recoveryTargetImmediate,
+ false,
+ NULL, NULL, NULL
+ },
+
+ {
{"hot_standby", PGC_POSTMASTER, REPLICATION_STANDBY,
gettext_noop("Allows connections and queries during recovery."),
NULL
@@ -3379,15 +3381,6 @@ static struct config_string ConfigureNamesString[] =
},
{
- {"recovery_target", PGC_POSTMASTER, WAL_RECOVERY_TARGET,
- gettext_noop("Set to 'immediate' to end recovery as soon as a consistent state is reached."),
- NULL
- },
- &recovery_target_string,
- "",
- check_recovery_target, assign_recovery_target, NULL
- },
- {
{"recovery_target_xid", PGC_POSTMASTER, WAL_RECOVERY_TARGET,
gettext_noop("Sets the transaction ID up to which recovery will proceed."),
NULL
@@ -11052,30 +11045,6 @@ assign_recovery_target_timeline(const char *newval, void *extra)
}
static bool
-check_recovery_target(char **newval, void **extra, GucSource source)
-{
- if (strcmp(*newval, "immediate") != 0 && strcmp(*newval, "") != 0)
- {
- GUC_check_errdetail("The only allowed value is \"immediate\".");
- return false;
- }
- return true;
-}
-
-static void
-assign_recovery_target(const char *newval, void *extra)
-{
- if (newval && strcmp(newval, "") != 0)
- recoveryTarget = RECOVERY_TARGET_IMMEDIATE;
- else
- /*
- * Reset recoveryTarget to RECOVERY_TARGET_UNSET to proper handle user
- * setting multiple recovery_target with blank value on last.
- */
- recoveryTarget = RECOVERY_TARGET_UNSET;
-}
-
-static bool
check_recovery_target_xid(char **newval, void **extra, GucSource source)
{
if (strcmp(*newval, "") != 0)
@@ -11100,11 +11069,8 @@ assign_recovery_target_xid(const char *newval, void *extra)
{
if (newval && strcmp(newval, "") != 0)
{
- recoveryTarget = RECOVERY_TARGET_XID;
recoveryTargetXid = *((TransactionId *) extra);
}
- else
- recoveryTarget = RECOVERY_TARGET_UNSET;
}
static bool
@@ -11163,11 +11129,8 @@ assign_recovery_target_time(const char *newval, void *extra)
{
if (newval && strcmp(newval, "") != 0)
{
- recoveryTarget = RECOVERY_TARGET_TIME;
recoveryTargetTime = *((TimestampTz *) extra);
}
- else
- recoveryTarget = RECOVERY_TARGET_UNSET;
}
static bool
@@ -11188,11 +11151,8 @@ assign_recovery_target_name(const char *newval, void *extra)
{
if (newval && strcmp(newval, "") != 0)
{
- recoveryTarget = RECOVERY_TARGET_NAME;
recoveryTargetName = (char *) newval;
}
- else
- recoveryTarget = RECOVERY_TARGET_UNSET;
}
static bool
@@ -11242,11 +11202,8 @@ assign_recovery_target_lsn(const char *newval, void *extra)
{
if (newval && strcmp(newval, "") != 0)
{
- recoveryTarget = RECOVERY_TARGET_LSN;
recoveryTargetLSN = *((XLogRecPtr *) extra);
}
- else
- recoveryTarget = RECOVERY_TARGET_UNSET;
}
static bool
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index ee9ec6a..ccdc94d 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -246,8 +246,8 @@
# Set these only when performing a targeted recovery.
-#recovery_target = '' # 'immediate' to end recovery as soon as a
- # consistent state is reached
+#recovery_target_immediate = false # end recovery as soon as a
+ # consistent state is reached
# (change requires restart)
#recovery_target_name = '' # the named restore point to which recovery will proceed
# (change requires restart)
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index f3a7ba4..d257f4f 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -127,13 +127,17 @@ extern int recoveryTargetAction;
extern int recovery_min_apply_delay;
extern char *PrimaryConnInfo;
extern char *PrimarySlotName;
+extern bool recoveryTargetImmediate;
+extern char *recovery_target_xid_string;
+extern char *recovery_target_time_string;
+extern char *recovery_target_name_string;
+extern char *recovery_target_lsn_string;
/* indirectly set via GUC system */
extern TransactionId recoveryTargetXid;
extern TimestampTz recoveryTargetTime;
extern char *recoveryTargetName;
extern XLogRecPtr recoveryTargetLSN;
-extern RecoveryTargetType recoveryTarget;
extern char *PromoteTriggerFile;
extern RecoveryTargetTimeLineGoal recoveryTargetTimeLineGoal;
extern TimeLineID recoveryTargetTLIRequested;
diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl
index f6f2e8b..df261f0 100644
--- a/src/test/recovery/t/003_recovery_targets.pl
+++ b/src/test/recovery/t/003_recovery_targets.pl
@@ -3,7 +3,7 @@ use strict;
use warnings;
use PostgresNode;
use TestLib;
-use Test::More tests => 9;
+use Test::More tests => 6;
# 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
@@ -53,7 +53,7 @@ $node_master->init(has_archiving => 1, allows_streaming => 1);
$node_master->start;
# Create data before taking the backup, aimed at testing
-# recovery_target = 'immediate'
+# recovery_target_immediate
$node_master->safe_psql('postgres',
"CREATE TABLE tab_int AS SELECT generate_series(1,1000) AS a");
my $lsn1 =
@@ -99,7 +99,7 @@ $node_master->safe_psql('postgres',
$node_master->safe_psql('postgres', "SELECT pg_switch_wal()");
# Test recovery targets
-my @recovery_params = ("recovery_target = 'immediate'");
+my @recovery_params = ("recovery_target_immediate = true");
test_recovery_standby('immediate target',
'standby_1', $node_master, \@recovery_params, "1000", $lsn1);
@recovery_params = ("recovery_target_xid = '$recovery_txid'");
@@ -115,31 +115,11 @@ test_recovery_standby('name', 'standby_4', $node_master, \@recovery_params,
test_recovery_standby('LSN', 'standby_5', $node_master, \@recovery_params,
"5000", $lsn5);
-# Multiple targets
-# Last entry has priority (note that an array respects the order of items
-# not hashes).
-@recovery_params = (
- "recovery_target_name = '$recovery_name'",
- "recovery_target_xid = '$recovery_txid'",
- "recovery_target_time = '$recovery_time'");
-test_recovery_standby('name + XID + time',
- 'standby_6', $node_master, \@recovery_params, "3000", $lsn3);
-@recovery_params = (
- "recovery_target_time = '$recovery_time'",
- "recovery_target_name = '$recovery_name'",
- "recovery_target_xid = '$recovery_txid'");
-test_recovery_standby('time + name + XID',
- 'standby_7', $node_master, \@recovery_params, "2000", $lsn2);
-@recovery_params = (
- "recovery_target_xid = '$recovery_txid'",
- "recovery_target_time = '$recovery_time'",
- "recovery_target_name = '$recovery_name'");
-test_recovery_standby('XID + time + name',
- 'standby_8', $node_master, \@recovery_params, "4000", $lsn4);
-@recovery_params = (
- "recovery_target_xid = '$recovery_txid'",
- "recovery_target_time = '$recovery_time'",
- "recovery_target_name = '$recovery_name'",
- "recovery_target_lsn = '$recovery_lsn'",);
-test_recovery_standby('XID + time + name + LSN',
- 'standby_9', $node_master, \@recovery_params, "5000", $lsn5);
+# multiple recovery targets are disallowed
+my $node_standby6 = get_new_node('standby_6');
+$node_standby6->init_from_backup($node_master, 'my_backup',
+ has_restoring => 1);
+$node_standby6->append_conf('postgresql.conf', "recovery_target_lsn = '$recovery_lsn'");
+$node_standby6->append_conf('postgresql.conf', "recovery_target_xid = '$recovery_txid'");
+$node_standby6->command_fails(['pg_ctl', '-D', $node_standby6->data_dir, '-l',
+ $node_standby6->logfile, 'start']);
