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
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'");

Attachment: signature.asc
Description: PGP signature

Reply via email to