Treat negative values of recovery_min_apply_delay as having no effect.

tglsfdc · tglsfdc · commit e7c1188756c9 · 2015-01-03T13:14:12.000-05:00
At one point in the development of this feature, it was claimed that
allowing negative values would be useful to compensate for timezone
differences between master and slave servers.  That was based on a mistaken
assumption that commit timestamps are recorded in local time; but of course
they're in UTC.  Nor is a negative apply delay likely to be a sane way of
coping with server clock skew.  However, the committed patch still treated
negative delays as doing something, and the timezone misapprehension
survived in the user documentation as well.

If recovery_min_apply_delay were a proper GUC we'd just set the minimum
allowed value to be zero; but for the moment it seems better to treat
negative settings as if they were zero.

In passing do some extra wordsmithing on the parameter's documentation,
including correcting a second misstatement that the parameter affects
processing of Restore Point records.

Issue noted by Michael Paquier, who also provided the code patch; doc
changes by me.  Back-patch to 9.4 where the feature was introduced.
diff --git a/doc/src/sgml/recovery-config.sgml b/doc/src/sgml/recovery-config.sgml
@@ -415,9 +415,9 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"'  # Windows
        <para>
         By default, a standby server restores WAL records from the
         primary as soon as possible. It may be useful to have a time-delayed
-        copy of the data, offering various options to correct data loss errors.
+        copy of the data, offering opportunities to correct data loss errors.
         This parameter allows you to delay recovery by a fixed period of time,
-        specified in milliseconds if no unit is specified.  For example, if
+        measured in milliseconds if no unit is specified.  For example, if
         you set this parameter to <literal>5min</literal>, the standby will
         replay each transaction commit only when the system time on the standby
         is at least five minutes past the commit time reported by the master.
@@ -426,27 +426,26 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"'  # Windows
         It is possible that the replication delay between servers exceeds the
         value of this parameter, in which case no delay is added.
         Note that the delay is calculated between the WAL timestamp as written
-        on master and the time on the current standby. Delays
-        in transfer because of networks or cascading replication configurations
+        on master and the current time on the standby. Delays in transfer
+        because of network lag or cascading replication configurations
         may reduce the actual wait time significantly. If the system
         clocks on master and standby are not synchronized, this may lead to
         recovery applying records earlier than expected; but that is not a
-        major issue because useful settings of the parameter are much larger
-        than typical time deviations between servers. Be careful to allow for
-        different timezone settings on master and standby.
+        major issue because useful settings of this parameter are much larger
+        than typical time deviations between servers.
        </para>
        <para>
-        The delay occurs only on WAL records for COMMIT and Restore Points.
-        Other records may be replayed earlier than the specified delay, which
-        is not an issue for MVCC though it may potentially increase the number
-        of recovery conflicts generated.
+        The delay occurs only on WAL records for transaction commits.
+        Other records are replayed as quickly as possible, which
+        is not a problem because MVCC visibility rules ensure their effects
+        are not visible until the corresponding commit record is applied.
        </para>
        <para>
         The delay occurs until the standby is promoted or triggered. After that
         the standby will end recovery without further waiting.
        </para>
        <para>
-        This parameter is intended for use with streaming replication deployments,
+        This parameter is intended for use with streaming replication deployments;
         however, if the parameter is specified it will be honored in all cases.
         Synchronous replication is not affected by this setting because there is
         not yet any setting to request synchronous apply of transaction commits.
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
@@ -5752,7 +5752,7 @@ recoveryApplyDelay(XLogRecord *record)
 	int			microsecs;
 
 	/* nothing to do if no delay configured */
-	if (recovery_min_apply_delay == 0)
+	if (recovery_min_apply_delay <= 0)
 		return false;
 
 	/*
