Hi
> The attached seems to be the simplest way to fix this. (Needs
> documentation updates, test updates, error message refinement.)
Thank you!
I add documentation change, tests and rephrased error message.
regards, Sergei
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index db1a2d4..7cbea6e 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 error will be raised.
These parameters can only be set at server start.
</para>
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 6497393..290a157 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -11065,14 +11065,11 @@ check_recovery_target(char **newval, void **extra, GucSource source)
static void
assign_recovery_target(const char *newval, void *extra)
{
+ if (recoveryTarget)
+ elog(ERROR, "can not specify multiple recovery targets");
+
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
@@ -11098,13 +11095,14 @@ check_recovery_target_xid(char **newval, void **extra, GucSource source)
static void
assign_recovery_target_xid(const char *newval, void *extra)
{
+ if (recoveryTarget)
+ elog(ERROR, "can not specify multiple recovery targets");
+
if (newval && strcmp(newval, "") != 0)
{
recoveryTarget = RECOVERY_TARGET_XID;
recoveryTargetXid = *((TransactionId *) extra);
}
- else
- recoveryTarget = RECOVERY_TARGET_UNSET;
}
static bool
@@ -11161,13 +11159,14 @@ check_recovery_target_time(char **newval, void **extra, GucSource source)
static void
assign_recovery_target_time(const char *newval, void *extra)
{
+ if (recoveryTarget)
+ elog(ERROR, "can not specify multiple recovery targets");
+
if (newval && strcmp(newval, "") != 0)
{
recoveryTarget = RECOVERY_TARGET_TIME;
recoveryTargetTime = *((TimestampTz *) extra);
}
- else
- recoveryTarget = RECOVERY_TARGET_UNSET;
}
static bool
@@ -11186,13 +11185,14 @@ check_recovery_target_name(char **newval, void **extra, GucSource source)
static void
assign_recovery_target_name(const char *newval, void *extra)
{
+ if (recoveryTarget)
+ elog(ERROR, "can not specify multiple recovery targets");
+
if (newval && strcmp(newval, "") != 0)
{
recoveryTarget = RECOVERY_TARGET_NAME;
recoveryTargetName = (char *) newval;
}
- else
- recoveryTarget = RECOVERY_TARGET_UNSET;
}
static bool
@@ -11240,13 +11240,14 @@ check_recovery_target_lsn(char **newval, void **extra, GucSource source)
static void
assign_recovery_target_lsn(const char *newval, void *extra)
{
+ if (recoveryTarget)
+ elog(ERROR, "can not specify multiple recovery targets");
+
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..1fd4aec 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -79,7 +79,7 @@ extern HotStandbyState standbyState;
*/
typedef enum
{
- RECOVERY_TARGET_UNSET,
+ RECOVERY_TARGET_UNSET = 0,
RECOVERY_TARGET_XID,
RECOVERY_TARGET_TIME,
RECOVERY_TARGET_NAME,
diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl
index f6f2e8b..cba367c 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 => 10;
# 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
@@ -115,31 +115,44 @@ 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).
+# multiple recovery targets are disallowed
+sub test_recovery_multiple_targets
+{
+ my $test_name = shift;
+ my $node_name = shift;
+ my $node_master = shift;
+ my $recovery_params = shift;
+
+ my $node_standby = get_new_node($node_name);
+ $node_standby->init_from_backup($node_master, 'my_backup',
+ has_restoring => 1);
+
+ foreach my $param_item (@$recovery_params)
+ {
+ $node_standby->append_conf('postgresql.conf', qq($param_item));
+ }
+
+ $node_standby->command_fails(['pg_ctl', '-D', $node_standby->data_dir, '-l',
+ $node_standby->logfile, 'start'], "check standby content for $test_name");
+}
+
+@recovery_params = (
+ "recovery_target = 'immediate'",
+ "recovery_target_xid = '$recovery_txid'",);
+test_recovery_multiple_targets('multi XID', 'standby_6', $node_master, \@recovery_params);
@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_target = 'immediate'",
+ "recovery_target_time = '$recovery_time'",);
+test_recovery_multiple_targets('multi time', 'standby_7', $node_master, \@recovery_params);
@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_target = 'immediate'",
+ "recovery_target_name = '$recovery_name'",);
+test_recovery_multiple_targets('multi name', 'standby_8', $node_master, \@recovery_params);
@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_target = 'immediate'",
+ "recovery_target_lsn = '$recovery_lsn'",);
+test_recovery_multiple_targets('multi LSN', 'standby_9', $node_master, \@recovery_params);
@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);
+ "recovery_target_xid = '$recovery_txid'",
+ "recovery_target = 'immediate'",);
+test_recovery_multiple_targets('multi immediate', 'standby_10', $node_master, \@recovery_params);