Hello
> I would think we'd have the different GUCs and then the check functions
> would only validate that they're valid inputs and then when we get to
> the point where we're starting to do recovery we check and make sure
> we've been given a sane overall configuration- which means that only
> *one* is set, and it matches the recovery target requested.
How about something like attached patch?
regards, Sergei
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index db1a2d4..d95d0a7 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -3221,7 +3221,7 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
<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>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index c80b14e..01fbab8 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -261,11 +261,18 @@ 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;
TransactionId recoveryTargetXid;
@@ -5381,9 +5388,42 @@ readRecoverySignalFile(void)
static void
validateRecoveryParameters(void)
{
+ uint8 targetSettingsCount = 0;
+
if (!ArchiveRecoveryRequested)
return;
+ /* Check for mutually exclusive target actions */
+ if (strcmp(recovery_target_string, "") != 0)
+ {
+ ++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..fd85484 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -199,7 +199,6 @@ 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 +548,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;
@@ -3385,7 +3378,7 @@ static struct config_string ConfigureNamesString[] =
},
&recovery_target_string,
"",
- check_recovery_target, assign_recovery_target, NULL
+ check_recovery_target, NULL, NULL
},
{
{"recovery_target_xid", PGC_POSTMASTER, WAL_RECOVERY_TARGET,
@@ -11062,18 +11055,6 @@ check_recovery_target(char **newval, void **extra, GucSource source)
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)
@@ -11100,11 +11081,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 +11141,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 +11163,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 +11214,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/include/access/xlog.h b/src/include/access/xlog.h
index f3a7ba4..0addbb7 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 char *recovery_target_string;
+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..1a90ad9 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 => 5;
# 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
@@ -114,32 +114,3 @@ test_recovery_standby('name', 'standby_4', $node_master, \@recovery_params,
@recovery_params = ("recovery_target_lsn = '$recovery_lsn'");
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);
