On 28/12/2018 00:15, Michael Paquier wrote:
> Yes, I was also thinking something among those lines, and the patch is
> a bit confusing by linking standby mode with latest timeline. It
> seems to me that switching the default value to "latest" at GUC level
> would be the way to go, instead of picking up the TLI from the control
> file. Introducing a new value which maps to the current empty value
> may be useful as well, like "control_file"?
OK, here are patches for this approach:
1. Add value 'current' for recovery_target_timeline
2. Change default of recovery_target_timeline to 'latest'
The first is really a fixup of the recovery.conf-removal patch. In
<=PG11, you could not explicitly select the current timeline; it was
only available if you don't mention recovery_target_timeline at all.
The original patch contained a setting 'controlfile', similar to your
suggestion, but that sounds a bit low-level implementation detail to me.
I like the suggestion 'current'.
The second then just changes the GUC default, without any special
treatment for standby mode.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From d4949efdfb246585495566cf8a5d85761554986c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Fri, 28 Dec 2018 10:44:12 +0100
Subject: [PATCH v2 1/2] Add value 'current' for recovery_target_timeline
This value represents the default behavior of using the current
timeline. Previously, this was represented by an empty string.
(Before the removal of recovery.conf, this setting could not be chosen
explicitly but was used when recovery_target_timeline was not
mentioned at all.)
---
doc/src/sgml/config.sgml | 8 +++++---
src/backend/utils/misc/guc.c | 4 ++--
src/backend/utils/misc/postgresql.conf.sample | 3 +--
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index e94b305add..cf4d8ab219 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -3350,9 +3350,11 @@ <title>Recovery Target</title>
</term>
<listitem>
<para>
- Specifies recovering into a particular timeline. The default is
- to recover along the same timeline that was current when the
- base backup was taken. Setting this to <literal>latest</literal>
recovers
+ 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
+ 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
in complex re-recovery situations, where you need to return to
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 6fe1939881..6342872b0d 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",
check_recovery_target_timeline,
assign_recovery_target_timeline, NULL
},
@@ -11031,7 +11031,7 @@ check_recovery_target_timeline(char **newval, void
**extra, GucSource source)
RecoveryTargetTimeLineGoal rttg = RECOVERY_TARGET_TIMELINE_CONTROLFILE;
RecoveryTargetTimeLineGoal *myextra;
- if (strcmp(*newval, "") == 0)
+ if (strcmp(*newval, "current") == 0)
rttg = RECOVERY_TARGET_TIMELINE_CONTROLFILE;
else if (strcmp(*newval, "latest") == 0)
rttg = RECOVERY_TARGET_TIMELINE_LATEST;
diff --git a/src/backend/utils/misc/postgresql.conf.sample
b/src/backend/utils/misc/postgresql.conf.sample
index 1fa02d2c93..f7c1dee240 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -261,8 +261,7 @@
# just after the specified recovery target (on)
# just before the recovery target (off)
# (change requires restart)
-#recovery_target_timeline = '' # unset means read from control file (default),
- # or set to 'latest' or timeline ID
+#recovery_target_timeline = 'current' # 'current', 'latest', or timeline ID
# (change requires restart)
#recovery_target_action = 'pause' # 'pause', 'promote', 'shutdown'
# (change requires restart)
--
2.20.1
>From ee2f6456bd847b8acbdbc605aa7350ab1db4b45c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Fri, 28 Dec 2018 12:01:37 +0100
Subject: [PATCH v2 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.
---
doc/src/sgml/config.sgml | 9 ++++++---
doc/src/sgml/high-availability.sgml | 8 ++------
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, 14 insertions(+), 41 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index cf4d8ab219..3d3341ca6d 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -3353,10 +3353,13 @@ <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
+ to the latest timeline found in the archive. That is the default.
+ </para>
+
+ <para>
+ You 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..47a93aef5b 100644
--- a/doc/src/sgml/high-availability.sgml
+++ b/doc/src/sgml/high-availability.sgml
@@ -689,10 +689,7 @@ <title>Setting Up a Standby Server</title>
server (see <xref linkend="backup-pitr-recovery"/>). Create a file
<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
- that occurs at failover to another standby.
+ the WAL archive.
</para>
<note>
@@ -1023,8 +1020,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>.
+ servers will continue to stream from the new master.
</para>
<para>
diff --git a/src/backend/access/transam/xlog.c
b/src/backend/access/transam/xlog.c
index 5729867879..d3eb49bc33 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -325,7 +325,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 6342872b0d..e6e7ca9d5b 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
