On 11/7/19 4:36 PM, Grigory Smolkin wrote:

On 11/7/19 12:56 PM, Kyotaro Horiguchi wrote:
At Thu, 7 Nov 2019 12:22:28 +0300, Grigory Smolkin <g.smol...@postgrespro.ru> wrote in
On 11/7/19 8:36 AM, Kyotaro Horiguchi wrote:
At Thu, 7 Nov 2019 02:28:39 +0300, Grigory Smolkin
<g.smol...@postgrespro.ru> wrote in
On 11/6/19 1:55 PM, Grigory Smolkin wrote:
On 11/6/19 12:56 PM, Fujii Masao wrote:
What happens if this parameter is set to latest in the standby mode?
Or the combination of those settings should be prohibited?
Currently it will behave just like regular standby, so I think, to
avoid confusion and keep things simple, the combination of them should
be prohibited.
Thank you for pointing this out, I will work on it.
Attached new patch revision, now it is impossible to use
recovery_target 'latest' in standby mode.
TAP test is updated to reflect this behavior.
In the first place, latest (or anything it could be named as) is
defined as the explit label for the default behavior. Thus the latest
should work as if nothing is set to recovery_target* following the
definition.  That might seems somewhat strange but I think at least it
is harmless.

Well, it was more about getting default behavior by using some
explicit recovery_target, not the other way around. Because it will
break some 3rd party backup and replication applications, that may
rely on old behavior of ignoring recovery_target_action when no
recovery_target is provided.
But you think that it is worth pursuing, I can do that.
Ah. Sorry for the misleading statement. What I had in my mind was
somewhat the mixture of them.  I thought that recovery_target =''
behaves the same way as now, r_t_action is ignored. And 'latest' just
makes recovery_target_action work as the current non-empty
recovery_target's does. But I'm not confident that it is a good
design.

recovery_target=immediate + r_t_action=shutdown for a standby works as
commanded. Do we need to inhibit that, too?
Why something, that work as expected, should be inhibited?
To make sure, I don't think we should do that. I meant by the above
that standby mode is already accepting recovery_target_action so
inhibiting that only for 'latest' is not orthogonal and could be more
confusing for users, and complicatig the code. So my opinion is we
shouldn't inhibit 'latest' unless r_t_action harms.

I gave it some thought and now think that prohibiting recovery_target 'latest' and standby was a bad idea. All recovery_targets follow the same pattern of usage, so recovery_target "latest" also must be capable of working in standby mode. All recovery_targets have a clear deterministic 'target' where recovery should end. In case of recovery_target "latest" this target is the end of available WAL, therefore the end of available WAL must be more clearly defined.
I will work on it.

Thank you for a feedback.


Attached new patch revision, now end of available WAL is defined as the fact of missing required WAL. In case of standby, the end of WAL is defined as 2 consecutive switches of WAL source, that didn`t provided requested record. In case of streaming standby, each switch of WAL source is forced after 3 failed attempts to get new data from walreceiver.

All constants are taken off the top of my head and serves as proof of concept.
TAP test is updated.





regards.

--
Grigory Smolkin
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 619ac8c50c..f0fa0b09c8 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -3316,21 +3316,19 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"'  # Windows
 
      <variablelist>
      <varlistentry id="guc-recovery-target" xreflabel="recovery_target">
-      <term><varname>recovery_target</varname><literal> = 'immediate'</literal>
+      <term><varname>recovery_target</varname> (<type>string</type>)
       <indexterm>
         <primary><varname>recovery_target</varname> configuration parameter</primary>
       </indexterm>
       </term>
       <listitem>
        <para>
-        This parameter specifies that recovery should end as soon as a
-        consistent state is reached, i.e. as early as possible. When restoring
-        from an online backup, this means the point where taking the backup
-        ended.
-       </para>
-       <para>
-        Technically, this is a string parameter, but <literal>'immediate'</literal>
-        is currently the only allowed value.
+        This parameter determines how far recovery should proceed. The value
+        <literal>immediate</literal> means that recovery should end as soon as a consistent
+        state is reached, i.e. as early as possible. When restoring from an online
+        backup, this means the point where taking the backup ended.
+        The second possible value <literal>latest</literal> means that recovery
+        should proceed to the end of the available WAL log.
        </para>
       </listitem>
      </varlistentry>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 77ad765989..74b1a6696d 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -843,6 +843,13 @@ static bool updateMinRecoveryPoint = true;
  */
 bool		reachedConsistency = false;
 
+/*
+ * Have we reached an end of available WAL?
+ * Only ever set when RECOVERY_TARGET_LATEST is used.
+ */
+bool	reachedEndOfWAL = false;
+int 	standby_src_switch_count = 0;
+
 static bool InRedo = false;
 
 /* Have we launched bgwriter during recovery? */
@@ -4309,6 +4316,14 @@ ReadRecord(XLogReaderState *xlogreader, XLogRecPtr RecPtr, int emode,
 
 		if (record)
 		{
+			/*
+			 * When recovering to the end of WAL in standby mode, reset
+			 * the counter of WAL source switches after page
+			 * is successfully readed.
+			 */
+			if (recoveryTarget == RECOVERY_TARGET_LATEST && StandbyMode)
+				standby_src_switch_count = 0;
+
 			/* Great, got a record */
 			return record;
 		}
@@ -4372,7 +4387,7 @@ ReadRecord(XLogReaderState *xlogreader, XLogRecPtr RecPtr, int emode,
 			}
 
 			/* In standby mode, loop back to retry. Otherwise, give up. */
-			if (StandbyMode && !CheckForStandbyTrigger())
+			if (StandbyMode && !CheckForStandbyTrigger() && !reachedEndOfWAL)
 				continue;
 			else
 				return NULL;
@@ -6350,6 +6365,9 @@ StartupXLOG(void)
 		else if (recoveryTarget == RECOVERY_TARGET_IMMEDIATE)
 			ereport(LOG,
 					(errmsg("starting point-in-time recovery to earliest consistent point")));
+		else if (recoveryTarget == RECOVERY_TARGET_LATEST)
+			ereport(LOG,
+					(errmsg("starting point-in-time recovery until all available WAL is applied")));
 		else
 			ereport(LOG,
 					(errmsg("starting archive recovery")));
@@ -7261,6 +7279,18 @@ StartupXLOG(void)
 			 * end of main redo apply loop
 			 */
 
+			/*
+			 * If all avaialble WAL is replayed,
+			 * then RECOVERY_TARGET_LATEST is satisfied
+			 */
+			if (recoveryTarget == RECOVERY_TARGET_LATEST && reachedEndOfWAL)
+			{
+				ereport(LOG,
+					(errmsg("recovery stopping after reaching the end of available WAL")));
+
+				reachedStopPoint = true;
+			}
+
 			if (reachedStopPoint)
 			{
 				if (!reachedConsistency)
@@ -7462,6 +7492,8 @@ StartupXLOG(void)
 					 recoveryStopName);
 		else if (recoveryTarget == RECOVERY_TARGET_IMMEDIATE)
 			snprintf(reason, sizeof(reason), "reached consistency");
+		else if (recoveryTarget == RECOVERY_TARGET_LATEST)
+			snprintf(reason, sizeof(reason), "applied all available WAL");
 		else
 			snprintf(reason, sizeof(reason), "no recovery target specified");
 
@@ -11737,6 +11769,7 @@ WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
 	static TimestampTz last_fail_time = 0;
 	TimestampTz now;
 	bool		streaming_reply_sent = false;
+	int standby_failed_read_attempts = 0;
 
 	/*-------
 	 * Standby mode is implemented by a state machine:
@@ -11796,7 +11829,10 @@ WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
 					 * and pg_wal.
 					 */
 					if (!StandbyMode)
+					{
+						reachedEndOfWAL = true;
 						return false;
+					}
 
 					/*
 					 * If primary_conninfo is set, launch walreceiver to try
@@ -11934,9 +11970,27 @@ WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
 		}
 
 		if (currentSource != oldSource)
+		{
+			/*
+			 * If the amount of WAL source switches exceeds the limit,
+			 * when recovering to end of WAL in standby mode,
+			 * then declate the current state to be the end of WAL.
+			 */
+			if (recoveryTarget == RECOVERY_TARGET_LATEST)
+			{
+				standby_src_switch_count++;
+
+				if (standby_src_switch_count >= 2)
+				{
+					reachedEndOfWAL = true;
+					return false;
+				}
+			}
+
 			elog(DEBUG2, "switched WAL source from %s to %s after %s",
 				 xlogSourceNames[oldSource], xlogSourceNames[currentSource],
 				 lastSourceFailed ? "failure" : "success");
+		}
 
 		/*
 		 * We've now handled possible failure. Try to read from the chosen
@@ -11968,9 +12022,13 @@ WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
 				if (readFile >= 0)
 					return true;	/* success! */
 
+				if (recoveryTarget == RECOVERY_TARGET_LATEST)
+					standby_failed_read_attempts++;
+
 				/*
 				 * Nope, not found in archive or pg_wal.
 				 */
+
 				lastSourceFailed = true;
 				break;
 
@@ -12047,6 +12105,11 @@ WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
 						}
 						break;
 					}
+					else
+					{
+						if (recoveryTarget == RECOVERY_TARGET_LATEST)
+							standby_failed_read_attempts++;
+					}
 
 					/*
 					 * Data not here yet. Check for trigger, then wait for
@@ -12102,6 +12165,18 @@ WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
 		 * process.
 		 */
 		HandleStartupProcInterrupts();
+
+		/*
+		 * If the amount of failed attempts to get data from current
+		 * WAL source exceeds the limit, when recovering to the end
+		 * of WAL in standby mode, then force the WAL source switch.
+		 */
+		if (recoveryTarget == RECOVERY_TARGET_LATEST &&
+			standby_failed_read_attempts >= 3)
+		{
+			lastSourceFailed = true;
+			break;
+		}
 	}
 
 	return false;				/* not reached */
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index f0ed326a1b..e7288a79b6 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -3503,7 +3503,10 @@ static struct config_string ConfigureNamesString[] =
 
 	{
 		{"recovery_target", PGC_POSTMASTER, WAL_RECOVERY_TARGET,
-			gettext_noop("Set to \"immediate\" to end recovery as soon as a consistent state is reached."),
+			gettext_noop("Set to \"immediate\" to end recovery as "
+						 "soon as a consistent state is reached or "
+						 " to \"latest\" to end recovery after "
+						 "replaying all available WAL in the archive."),
 			NULL
 		},
 		&recovery_target_string,
@@ -11498,9 +11501,10 @@ error_multiple_recovery_targets(void)
 static bool
 check_recovery_target(char **newval, void **extra, GucSource source)
 {
-	if (strcmp(*newval, "immediate") != 0 && strcmp(*newval, "") != 0)
+	if (strcmp(*newval, "immediate") != 0 && strcmp(*newval, "latest") != 0 &&
+		strcmp(*newval, "") != 0)
 	{
-		GUC_check_errdetail("The only allowed value is \"immediate\".");
+		GUC_check_errdetail("The only allowed values are \"immediate\" and \"latest\".");
 		return false;
 	}
 	return true;
@@ -11510,11 +11514,17 @@ static void
 assign_recovery_target(const char *newval, void *extra)
 {
 	if (recoveryTarget != RECOVERY_TARGET_UNSET &&
-		recoveryTarget != RECOVERY_TARGET_IMMEDIATE)
+		recoveryTarget != RECOVERY_TARGET_IMMEDIATE &&
+		recoveryTarget != RECOVERY_TARGET_LATEST)
 		error_multiple_recovery_targets();
 
 	if (newval && strcmp(newval, "") != 0)
-		recoveryTarget = RECOVERY_TARGET_IMMEDIATE;
+	{
+		if (strcmp(newval, "immediate") == 0)
+			recoveryTarget = RECOVERY_TARGET_IMMEDIATE;
+		else if (strcmp(newval, "latest") == 0)
+			recoveryTarget = RECOVERY_TARGET_LATEST;
+	}
 	else
 		recoveryTarget = RECOVERY_TARGET_UNSET;
 }
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index d519252aad..55e7e4bd2c 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -84,7 +84,8 @@ typedef enum
 	RECOVERY_TARGET_TIME,
 	RECOVERY_TARGET_NAME,
 	RECOVERY_TARGET_LSN,
-	RECOVERY_TARGET_IMMEDIATE
+	RECOVERY_TARGET_IMMEDIATE,
+	RECOVERY_TARGET_LATEST
 } RecoveryTargetType;
 
 /*
diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl
index d8fbd50011..01dab1aebf 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
@@ -77,7 +77,7 @@ my $lsn3 =
   $node_master->safe_psql('postgres', "SELECT pg_current_wal_lsn();");
 my $recovery_time = $node_master->safe_psql('postgres', "SELECT now()");
 
-# Even more data, this time with a recovery target name
+# More data, with a recovery target name
 $node_master->safe_psql('postgres',
 	"INSERT INTO tab_int VALUES (generate_series(3001,4000))");
 my $recovery_name = "my_target";
@@ -86,14 +86,17 @@ my $lsn4 =
 $node_master->safe_psql('postgres',
 	"SELECT pg_create_restore_point('$recovery_name');");
 
-# And now for a recovery target LSN
+# Even more data, this time with a recovery target LSN
 $node_master->safe_psql('postgres',
 	"INSERT INTO tab_int VALUES (generate_series(4001,5000))");
 my $lsn5 = my $recovery_lsn =
   $node_master->safe_psql('postgres', "SELECT pg_current_wal_lsn()");
 
+# And now for a recovery target 'latest'
 $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 = 'latest'");
+test_recovery_standby('latest target', '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(

Reply via email to