On 08/01/2019 04:37, Michael Paquier wrote: > On Mon, Dec 31, 2018 at 01:21:00PM +0200, David Steele wrote: >> This patch looks good to me. > > 0001 looks fine to me.
committed that one > - to the latest timeline found in the archive, which is useful in > - a standby server. Other than that you only need to set this parameter > + to the latest timeline found in the archive. That is the default. > + </para> > Isn't it useful to still mention that the default is useful especially > for standby servers? > > - the WAL archive. If you plan to have multiple standby servers for high > - availability purposes, set <varname>recovery_target_timeline</varname> to > - <literal>latest</literal>, to make the standby server follow the > timeline change > - that occurs at failover to another standby. > + the WAL archive. > I think that we should still keep this recommendation as well, as well > as the one below. Attached revised 0002 with those changes. >> There don't seem to be any tests for recovery_target_timeline=current. This >> is an preexisting condition but it probably wouldn't hurt to add one. > > Yes, I got to wonder while looking at this patch why we don't have one > yet in 003_recovery_targets.pl. That's easy enough to do thanks to > the extra rows inserted after doing the stuff for the LSN-based > restart point, and attached is a patch to add the test. Peter, could > you merge it with 0001? I am fine to take care of that myself if > necessary. In that test, if I change the 'current' to 'latest', the test doesn't fail, so it's probably not a good test. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From b8b4e27b322765b9592698a15946a626498365b5 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pe...@eisentraut.org> Date: Fri, 11 Jan 2019 10:36:10 +0100 Subject: [PATCH v3 2/2] Change default of recovery_target_timeline to 'latest' This is what one usually wants for recovery and almost always wants for a standby. Discussion: https://www.postgresql.org/message-id/flat/6dd2c23a-4162-8469-410f-bfe146e28...@2ndquadrant.com/ Reviewed-by: David Steele <da...@pgmasters.net> Reviewed-by: Michael Paquier <mich...@paquier.xyz> --- doc/src/sgml/config.sgml | 8 ++++++-- doc/src/sgml/high-availability.sgml | 6 +++--- src/backend/access/transam/xlog.c | 2 +- src/backend/utils/misc/guc.c | 7 ++++--- src/backend/utils/misc/postgresql.conf.sample | 2 +- src/bin/pg_rewind/RewindTest.pm | 2 -- src/test/recovery/t/004_timeline_switch.pl | 1 - src/test/recovery/t/009_twophase.pl | 12 ------------ src/test/recovery/t/012_subtransactions.pl | 12 ------------ 9 files changed, 15 insertions(+), 37 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index f64402adbe..b6f5822b84 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -3353,10 +3353,14 @@ <title>Recovery Target</title> Specifies recovering into a particular timeline. The value can be a numeric timeline ID or a special value. The value <literal>current</literal> recovers along the same timeline that was - current when the base backup was taken. That is the default. The + current when the base backup was taken. The value <literal>latest</literal> recovers to the latest timeline found in the archive, which is useful in - a standby server. Other than that you only need to set this parameter + a standby server. <literal>latest</literal> is the default. + </para> + + <para> + You usually only need to set this parameter in complex re-recovery situations, where you need to return to a state that itself was reached after a point-in-time recovery. See <xref linkend="backup-timelines"/> for discussion. diff --git a/doc/src/sgml/high-availability.sgml b/doc/src/sgml/high-availability.sgml index d8fd195da0..4882b20828 100644 --- a/doc/src/sgml/high-availability.sgml +++ b/doc/src/sgml/high-availability.sgml @@ -690,8 +690,8 @@ <title>Setting Up a Standby Server</title> <filename>standby.signal</filename> in the standby's cluster data directory. Set <xref linkend="guc-restore-command"/> to a simple command to copy files from the WAL archive. If you plan to have multiple standby servers for high - availability purposes, set <varname>recovery_target_timeline</varname> to - <literal>latest</literal>, to make the standby server follow the timeline change + availability purposes, make sure that <varname>recovery_target_timeline</varname> is set to + <literal>latest</literal> (the default), to make the standby server follow the timeline change that occurs at failover to another standby. </para> @@ -1024,7 +1024,7 @@ <title>Cascading Replication</title> <para> If an upstream standby server is promoted to become new master, downstream servers will continue to stream from the new master if - <varname>recovery_target_timeline</varname> is set to <literal>'latest'</literal>. + <varname>recovery_target_timeline</varname> is set to <literal>'latest'</literal> (the default). </para> <para> diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 9823b75767..2ab7d804f0 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -324,7 +324,7 @@ static bool recoveryStopAfter; * file was created.) During a sequential scan we do not allow this value * to decrease. */ -RecoveryTargetTimeLineGoal recoveryTargetTimeLineGoal = RECOVERY_TARGET_TIMELINE_CONTROLFILE; +RecoveryTargetTimeLineGoal recoveryTargetTimeLineGoal = RECOVERY_TARGET_TIMELINE_LATEST; TimeLineID recoveryTargetTLIRequested = 0; TimeLineID recoveryTargetTLI = 0; static List *expectedTLEs; diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 7eda7fdef9..ae925c1650 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -3387,7 +3387,7 @@ static struct config_string ConfigureNamesString[] = NULL }, &recovery_target_timeline_string, - "current", + "latest", check_recovery_target_timeline, assign_recovery_target_timeline, NULL }, @@ -11028,7 +11028,7 @@ show_data_directory_mode(void) static bool check_recovery_target_timeline(char **newval, void **extra, GucSource source) { - RecoveryTargetTimeLineGoal rttg = RECOVERY_TARGET_TIMELINE_CONTROLFILE; + RecoveryTargetTimeLineGoal rttg; RecoveryTargetTimeLineGoal *myextra; if (strcmp(*newval, "current") == 0) @@ -11037,6 +11037,8 @@ check_recovery_target_timeline(char **newval, void **extra, GucSource source) rttg = RECOVERY_TARGET_TIMELINE_LATEST; else { + rttg = RECOVERY_TARGET_TIMELINE_NUMERIC; + errno = 0; strtoul(*newval, NULL, 0); if (errno == EINVAL || errno == ERANGE) @@ -11044,7 +11046,6 @@ check_recovery_target_timeline(char **newval, void **extra, GucSource source) GUC_check_errdetail("recovery_target_timeline is not a valid number."); return false; } - rttg = RECOVERY_TARGET_TIMELINE_NUMERIC; } myextra = (RecoveryTargetTimeLineGoal *) guc_malloc(ERROR, sizeof(RecoveryTargetTimeLineGoal)); diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index f7c1dee240..a21865a77f 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -261,7 +261,7 @@ # just after the specified recovery target (on) # just before the recovery target (off) # (change requires restart) -#recovery_target_timeline = 'current' # 'current', 'latest', or timeline ID +#recovery_target_timeline = 'latest' # 'current', 'latest', or timeline ID # (change requires restart) #recovery_target_action = 'pause' # 'pause', 'promote', 'shutdown' # (change requires restart) diff --git a/src/bin/pg_rewind/RewindTest.pm b/src/bin/pg_rewind/RewindTest.pm index 3d07da5d94..85cae7e47b 100644 --- a/src/bin/pg_rewind/RewindTest.pm +++ b/src/bin/pg_rewind/RewindTest.pm @@ -161,7 +161,6 @@ sub create_standby $node_standby->append_conf( "postgresql.conf", qq( primary_conninfo='$connstr_master application_name=rewind_standby' -recovery_target_timeline='latest' )); $node_standby->set_standby_mode(); @@ -273,7 +272,6 @@ sub run_pg_rewind $node_master->append_conf( 'postgresql.conf', qq( primary_conninfo='port=$port_standby' -recovery_target_timeline='latest' )); $node_master->set_standby_mode(); diff --git a/src/test/recovery/t/004_timeline_switch.pl b/src/test/recovery/t/004_timeline_switch.pl index 79cbffb827..2b315854bc 100644 --- a/src/test/recovery/t/004_timeline_switch.pl +++ b/src/test/recovery/t/004_timeline_switch.pl @@ -51,7 +51,6 @@ $node_standby_2->append_conf( 'postgresql.conf', qq( primary_conninfo='$connstr_1 application_name=@{[$node_standby_2->name]}' -recovery_target_timeline='latest' )); $node_standby_2->restart; diff --git a/src/test/recovery/t/009_twophase.pl b/src/test/recovery/t/009_twophase.pl index dac2d4ec0d..2be1afcd8b 100644 --- a/src/test/recovery/t/009_twophase.pl +++ b/src/test/recovery/t/009_twophase.pl @@ -229,10 +229,6 @@ sub configure_and_reload # restart old master as new standby $cur_standby->enable_streaming($cur_master); -$cur_standby->append_conf( - 'postgresql.conf', qq( -recovery_target_timeline='latest' -)); $cur_standby->start; ############################################################################### @@ -267,10 +263,6 @@ sub configure_and_reload # restart old master as new standby $cur_standby->enable_streaming($cur_master); -$cur_standby->append_conf( - 'postgresql.conf', qq( -recovery_target_timeline='latest' -)); $cur_standby->start; $cur_master->psql('postgres', "COMMIT PREPARED 'xact_009_11'"); @@ -307,10 +299,6 @@ sub configure_and_reload # restart old master as new standby $cur_standby->enable_streaming($cur_master); -$cur_standby->append_conf( - 'postgresql.conf', qq( -recovery_target_timeline='latest' -)); $cur_standby->start; $cur_master->psql('postgres', "COMMIT PREPARED 'xact_009_12'"); diff --git a/src/test/recovery/t/012_subtransactions.pl b/src/test/recovery/t/012_subtransactions.pl index e26cc9c2ce..c184553694 100644 --- a/src/test/recovery/t/012_subtransactions.pl +++ b/src/test/recovery/t/012_subtransactions.pl @@ -119,10 +119,6 @@ # restore state ($node_master, $node_standby) = ($node_standby, $node_master); $node_standby->enable_streaming($node_master); -$node_standby->append_conf( - 'postgresql.conf', qq( -recovery_target_timeline='latest' -)); $node_standby->start; $node_standby->psql( 'postgres', @@ -170,10 +166,6 @@ # restore state ($node_master, $node_standby) = ($node_standby, $node_master); $node_standby->enable_streaming($node_master); -$node_standby->append_conf( - 'postgresql.conf', qq( -recovery_target_timeline='latest' -)); $node_standby->start; $psql_rc = $node_master->psql('postgres', "COMMIT PREPARED 'xact_012_1'"); is($psql_rc, '0', @@ -211,10 +203,6 @@ # restore state ($node_master, $node_standby) = ($node_standby, $node_master); $node_standby->enable_streaming($node_master); -$node_standby->append_conf( - 'postgresql.conf', qq( -recovery_target_timeline='latest' -)); $node_standby->start; $psql_rc = $node_master->psql('postgres', "ROLLBACK PREPARED 'xact_012_1'"); is($psql_rc, '0', -- 2.20.1
