could recovery_target_timeline=latest be the default in standby mode?
Is there ever a reason why you would *not* want
recovery_target_timeline=latest in standby mode?
pg_basebackup -R doesn't set it. That seems suboptimal.
Perhaps this could be the default in standby mode, or even the implicit
default, ignoring the recovery_target_timeline setting altogether.
How could we make things simpler here?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Hello
I am +1 for recovery_target_timeline=latest by default. This is common case in my opinion. And first release without recovery.conf is reasonable time for change default value.
But i doubt if we can ignore recovery_target_timeline in standby and always use latest in standby. I have no use case for this but i prefer change only default value.
regards, Sergei
On Fri, Dec 21, 2018 at 01:54:20PM +0300, Sergei Kornilov wrote:
I am +1 for recovery_target_timeline=latest by default. This is
common case in my opinion.
I agree also that switching to the latest timeline should be the
default. People get confused because of the current default.
--
Michael
On 22/12/2018 00:38, Michael Paquier wrote:
On Fri, Dec 21, 2018 at 01:54:20PM +0300, Sergei Kornilov wrote:
I am +1 for recovery_target_timeline=latest by default. This is
common case in my opinion.I agree also that switching to the latest timeline should be the
default. People get confused because of the current default.
How about this patch then?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0001-Always-use-latest-timeline-in-standby-mode.patchtext/plain; charset=UTF-8; name=0001-Always-use-latest-timeline-in-standby-mode.patch; x-mac-creator=0; x-mac-type=0Download
From 5fd266a61c63104d8f3f4cd3e19bfc99fd22ba21 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Thu, 27 Dec 2018 11:49:47 +0100
Subject: [PATCH] Always use latest timeline in standby mode
In standby mode, ignore the setting of recovery_target_timeline and
always behave as if 'latest' was specified.
---
doc/src/sgml/config.sgml | 15 ++++++++++++---
doc/src/sgml/high-availability.sgml | 8 ++------
src/backend/access/transam/xlog.c | 7 +++++++
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 ------------
7 files changed, 21 insertions(+), 36 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index e94b305add..76a83246ee 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -3352,13 +3352,22 @@ <title>Recovery Target</title>
<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
- 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
+ base backup was taken. Setting this to <literal>latest</literal> recovers
+ to the latest timeline found in the archive.
+ </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.
</para>
+
+ <para>
+ In standby mode, this parameter is ignored and the latest timeline is
+ always used. This allows a standby to follow timeline changes in
+ upstream servers.
+ </para>
</listitem>
</varlistentry>
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..315fff48c7 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -5420,6 +5420,13 @@ validateRecoveryParameters(void)
!EnableHotStandby)
recoveryTargetAction = RECOVERY_TARGET_ACTION_SHUTDOWN;
+ /*
+ * In standby mode, always use latest timeline, ignoring
+ * recovery_target_timeline setting.
+ */
+ if (StandbyModeRequested)
+ recoveryTargetTimeLineGoal = RECOVERY_TARGET_TIMELINE_LATEST;
+
/*
* If user specified recovery_target_timeline, validate it or compute the
* "latest" value. We can't do this until after we've gotten the restore
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
On 12/27/18 1:02 PM, Peter Eisentraut wrote:
On 22/12/2018 00:38, Michael Paquier wrote:
On Fri, Dec 21, 2018 at 01:54:20PM +0300, Sergei Kornilov wrote:
I am +1 for recovery_target_timeline=latest by default. This is
common case in my opinion.I agree also that switching to the latest timeline should be the
default. People get confused because of the current default.How about this patch then?
I like the idea of defaulting to the latest timeline since this is what
you want to do most of the time, but I think we'd then need a value for
following the current timelime, i.e. the one that the backup was taken
on (which is the current default).
I also think that changing the behavior of this setting based on
standby_mode is going to be confusing. Recovering to the most recent
timeline is the general case whether setting up a standby or not.
Imagine the following case:
1) Primary fails
2) Switch to standby
3) Standby runs for a while but fails before the old primary is rebuilt
4) We recover a new primary from the most recent backup which is
probably on the old timeline, but we'd rather recover to the new
timeline established after the failover.
Or, we recover a cluster for reporting and promote it so we can create
temp tables. We'd also like that to be on the most recent timeline.
I would recommend:
1) Make the default for recovery_target_timeline always be latest.
2) Add a new value, current, that replicates the current behavior.
Regards,
--
-David
david@pgmasters.net
On Thu, Dec 27, 2018 at 06:36:23PM +0200, David Steele wrote:
I like the idea of defaulting to the latest timeline since this is what you
want to do most of the time, but I think we'd then need a value for
following the current timelime, i.e. the one that the backup was taken on
(which is the current default).[...]
I would recommend:
1) Make the default for recovery_target_timeline always be latest.
2) Add a new value, current, that replicates the current behavior.
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"?
--
Michael
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
Attachments:
v2-0001-Add-value-current-for-recovery_target_timeline.patchtext/plain; charset=UTF-8; name=v2-0001-Add-value-current-for-recovery_target_timeline.patch; x-mac-creator=0; x-mac-type=0Download
From d4949efdfb246585495566cf8a5d85761554986c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@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
v2-0002-Change-default-of-recovery_target_timeline-to-lat.patchtext/plain; charset=UTF-8; name=v2-0002-Change-default-of-recovery_target_timeline-to-lat.patch; x-mac-creator=0; x-mac-type=0Download
From ee2f6456bd847b8acbdbc605aa7350ab1db4b45c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@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
Hi Peter,
On 12/28/18 1:08 PM, Peter Eisentraut wrote:
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'.
This patch looks good to me.
The second then just changes the GUC default, without any special
treatment for standby mode.
Yes, that's exactly what I was thinking.
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.
Regards,
--
-David
david@pgmasters.net
On Mon, Dec 31, 2018 at 01:21:00PM +0200, David Steele wrote:
This patch looks good to me.
(Sorry for the delay here)
0001 looks fine to me.
- 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.- RecoveryTargetTimeLineGoal rttg = RECOVERY_TARGET_TIMELINE_CONTROLFILE;
+ RecoveryTargetTimeLineGoal rttg;
Good to remove this noise.Yes, that's exactly what I was thinking.
Agreed.
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.
--
Michael
Attachments:
timeline-current-test.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl
index b46b318f5a..fe59221b57 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 => 8;
+use Test::More tests => 9;
# 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
@@ -92,8 +92,11 @@ $node_master->safe_psql('postgres',
my $lsn5 = my $recovery_lsn =
$node_master->safe_psql('postgres', "SELECT pg_current_wal_lsn()");
+# Rows used to restore up to the end of current timeline.
$node_master->safe_psql('postgres',
"INSERT INTO tab_int VALUES (generate_series(5001,6000))");
+my $lsn6 =
+ $node_master->safe_psql('postgres', "SELECT pg_current_wal_lsn()");
# Force archiving of WAL file
$node_master->safe_psql('postgres', "SELECT pg_switch_wal()");
@@ -114,6 +117,9 @@ 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);
+@recovery_params = ("recovery_target_timeline = 'current'");
+test_recovery_standby('LSN', 'standby_6', $node_master, \@recovery_params,
+ "6000", $lsn6);
# Multiple targets
#
@@ -126,9 +132,9 @@ test_recovery_standby('LSN', 'standby_5', $node_master, \@recovery_params,
"recovery_target_name = ''",
"recovery_target_time = '$recovery_time'");
test_recovery_standby('multiple overriding settings',
- 'standby_6', $node_master, \@recovery_params, "3000", $lsn3);
+ 'standby_7', $node_master, \@recovery_params, "3000", $lsn3);
-my $node_standby = get_new_node('standby_7');
+my $node_standby = get_new_node('standby_8');
$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'");
On 1/8/19 5:37 AM, Michael Paquier wrote:
- 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?
Agreed.
- 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.
Agreed.
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.
The new test looks good.
Regards,
--
-David
david@pgmasters.net
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
Attachments:
v3-0002-Change-default-of-recovery_target_timeline-to-lat.patchtext/plain; charset=UTF-8; name=v3-0002-Change-default-of-recovery_target_timeline-to-lat.patch; x-mac-creator=0; x-mac-type=0Download
From b8b4e27b322765b9592698a15946a626498365b5 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@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-bfe146e28c0c@2ndquadrant.com/
Reviewed-by: David Steele <david@pgmasters.net>
Reviewed-by: Michael Paquier <michael@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
On Fri, Jan 11, 2019 at 11:17:48AM +0100, Peter Eisentraut wrote:
Attached revised 0002 with those changes.
This one looks fine.
In that test, if I change the 'current' to 'latest', the test doesn't
fail, so it's probably not a good test.
I can see your point. You would need a diverging timeline to test for
'latest', which can surely be done as part of 003_recovery_targets.pl.
It seems to me that that the test has initial value to make sure that
we replay up to the end of the produced timeline's data, which is
something untested now as the script has only restart points set to
before the end of the timeline. If you think that's not a good
addition now, I am also fine to not include it.
--
Michael
On 12/01/2019 00:53, Michael Paquier wrote:
On Fri, Jan 11, 2019 at 11:17:48AM +0100, Peter Eisentraut wrote:
Attached revised 0002 with those changes.
This one looks fine.
committed
In that test, if I change the 'current' to 'latest', the test doesn't
fail, so it's probably not a good test.I can see your point. You would need a diverging timeline to test for
'latest', which can surely be done as part of 003_recovery_targets.pl.
It seems to me that that the test has initial value to make sure that
we replay up to the end of the produced timeline's data, which is
something untested now as the script has only restart points set to
before the end of the timeline. If you think that's not a good
addition now, I am also fine to not include it.
I'm not sure what the coverage is in detail in this area. It seems we
already have tests for not-specific-recovery-target, maybe not in this
file, but most of the other tests rely on that, no?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Sun, Jan 13, 2019 at 10:17:32AM +0100, Peter Eisentraut wrote:
I'm not sure what the coverage is in detail in this area. It seems we
already have tests for not-specific-recovery-target, maybe not in this
file, but most of the other tests rely on that, no?
[... Checking ...]
There are no tests on HEAD using directly recovery_target_timeline in
any .pl or .pm file, which is definitely not a good idea.
004_timeline_switch.pl is actually designed for checking the latest
timeline, which is fine. 002_archiving.pl somewhat tests for the
current timeline, still it is not really designed for this purpose.
--
Michael