From f2cbffc7a6183a359b339cfed65e9d609de756f0 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Wed, 28 Nov 2018 12:36:49 +0100 Subject: [PATCH] Only allow one recovery target setting The previous recovery.conf regime accepted multiple recovery_target* settings and used the last one. This does not translate well to the general GUC system. Specifically, under EXEC_BACKEND, the settings are written out not in any particular order, so the order in which they were originally set is not available to new processes. Rather than redesign the GUC system, it was decided to abandon the old behavior and only allow one recovery target setting. A second setting will cause an error. However, it is allowed to set the same parameter multiple times or unset a parameter and set a different one. Discussion: https://www.postgresql.org/message-id/flat/27802171543235530%40iva2-6ec8f0a6115e.qloud-c.yandex.net#701a59c837ad0bf8c244344aaf3ef5a4 --- doc/src/sgml/config.sgml | 2 +- src/backend/utils/misc/guc.c | 46 +++++++++++++++++++-- src/test/recovery/t/003_recovery_targets.pl | 46 +++++++++------------ 3 files changed, 62 insertions(+), 32 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index db1a2d4e74..1e6e13c91c 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 recovery_target_lsn, recovery_target_name, recovery_target_time, or recovery_target_xid can be used; if more than one of these is specified in the configuration - file, the last entry will be used. + file, an error will be raised. These parameters can only be set at server start. diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 6497393c03..d469de73e9 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -11051,6 +11051,28 @@ assign_recovery_target_timeline(const char *newval, void *extra) recoveryTargetTLIRequested = 0; } +/* + * Recovery target settings: Only one of the several recovery_target* settings + * may be set. Setting a second one results in an error. The global variable + * recoveryTarget tracks which kind of recovery target was chosen. Other + * variables store the actual target value (for example a string or a xid). + * The assign functions of the parameters check whether a competing parameter + * was already set. But we want to allow setting the same parameter multiple + * times. We also want to allow unsetting a parameter and setting a different + * one, so we unset recoveryTarget when the parameter is set to an empty + * string. + */ + +static void +pg_attribute_noreturn() +error_multiple_recovery_targets(void) +{ + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("multiple recovery targets specified"), + errdetail("At most one of recovery_target, recovery_target_lsn, recovery_target_name, recovery_target_time, recovery_target_xid may be set."))); +} + static bool check_recovery_target(char **newval, void **extra, GucSource source) { @@ -11065,13 +11087,13 @@ check_recovery_target(char **newval, void **extra, GucSource source) static void assign_recovery_target(const char *newval, void *extra) { + if (recoveryTarget != RECOVERY_TARGET_UNSET && + recoveryTarget != RECOVERY_TARGET_IMMEDIATE) + error_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; } @@ -11098,6 +11120,10 @@ check_recovery_target_xid(char **newval, void **extra, GucSource source) static void assign_recovery_target_xid(const char *newval, void *extra) { + if (recoveryTarget != RECOVERY_TARGET_UNSET && + recoveryTarget != RECOVERY_TARGET_XID) + error_multiple_recovery_targets(); + if (newval && strcmp(newval, "") != 0) { recoveryTarget = RECOVERY_TARGET_XID; @@ -11161,6 +11187,10 @@ check_recovery_target_time(char **newval, void **extra, GucSource source) static void assign_recovery_target_time(const char *newval, void *extra) { + if (recoveryTarget != RECOVERY_TARGET_UNSET && + recoveryTarget != RECOVERY_TARGET_TIME) + error_multiple_recovery_targets(); + if (newval && strcmp(newval, "") != 0) { recoveryTarget = RECOVERY_TARGET_TIME; @@ -11186,6 +11216,10 @@ check_recovery_target_name(char **newval, void **extra, GucSource source) static void assign_recovery_target_name(const char *newval, void *extra) { + if (recoveryTarget != RECOVERY_TARGET_UNSET && + recoveryTarget != RECOVERY_TARGET_NAME) + error_multiple_recovery_targets(); + if (newval && strcmp(newval, "") != 0) { recoveryTarget = RECOVERY_TARGET_NAME; @@ -11240,6 +11274,10 @@ check_recovery_target_lsn(char **newval, void **extra, GucSource source) static void assign_recovery_target_lsn(const char *newval, void *extra) { + if (recoveryTarget != RECOVERY_TARGET_UNSET && + recoveryTarget != RECOVERY_TARGET_LSN) + error_multiple_recovery_targets(); + if (newval && strcmp(newval, "") != 0) { recoveryTarget = RECOVERY_TARGET_LSN; diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl index f6f2e8b745..b46b318f5a 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 => 8; # 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 @@ -116,30 +116,22 @@ 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); +# +# Multiple conflicting settings are not allowed, but setting the same +# parameter multiple times or unsetting a parameter and setting a +# different one is allowed. + @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_name = '$recovery_name'", + "recovery_target_name = ''", + "recovery_target_time = '$recovery_time'"); +test_recovery_standby('multiple overriding settings', + 'standby_6', $node_master, \@recovery_params, "3000", $lsn3); + +my $node_standby = get_new_node('standby_7'); +$node_standby->init_from_backup($node_master, 'my_backup', has_restoring => 1); +$node_standby->append_conf('postgresql.conf', "recovery_target_name = '$recovery_name' +recovery_target_time = '$recovery_time'"); +command_fails_like(['postgres', '-D', $node_standby->data_dir], + qr/multiple recovery targets specified/, + 'multiple conflicting settings'); -- 2.40.0