On Sat, Jan 17, 2015 at 2:44 AM, Andres Freund <and...@2ndquadrant.com> wrote:
> On 2015-01-05 17:16:43 +0900, Michael Paquier wrote:
>> + <varlistentry id="restore-command-retry-interval"
>> xreflabel="restore_command_retry_interval">
>> + <term><varname>restore_command_retry_interval</varname>
>> (<type>integer</type>)
>> + <indexterm>
>> + <primary><varname>restore_command_retry_interval</> recovery
>> parameter</primary>
>> + </indexterm>
>> + </term>
>> + <listitem>
>> + <para>
>> + If <varname>restore_command</> returns nonzero exit status code,
>> retry
>> + command after the interval of time specified by this parameter,
>> + measured in milliseconds if no unit is specified. Default value is
>> + <literal>5s</>.
>> + </para>
>> + <para>
>> + This command is particularly useful for warm standby configurations
>> + to leverage the amount of retries done to restore a WAL segment file
>> + depending on the activity of a master node.
>> + </para>
>
> Don't really like this paragraph. What's "leveraging the amount of
> retries done" supposed to mean?
Actually I have reworked the whole paragraph with the renaming of the
parameter. Hopefully that's more clear.
>> +# restore_command_retry_interval
>> +#
>> +# specifies an optional retry interval for restore_command command, if
>> +# previous command returned nonzero exit status code. This can be useful
>> +# to increase or decrease the number of calls of restore_command.
> It's not really the number but frequency, right?
Sure.
>> + else if (strcmp(item->name, "restore_command_retry_interval")
>> == 0)
>> + {
>> + const char *hintmsg;
>> +
>> + if (!parse_int(item->value,
>> &restore_command_retry_interval, GUC_UNIT_MS,
>> + &hintmsg))
>> + ereport(ERROR,
>> +
>> (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
>> + errmsg("parameter \"%s\"
>> requires a temporal value",
>> +
>> "restore_command_retry_interval"),
>> + hintmsg ? errhint("%s",
>> _(hintmsg)) : 0));
>
> "temporal value" sounds odd to my ears. I'd rather keep in line with
> what guc.c outputs in such a case.
Yeah, I have been pondering about that a bit and I think that
wal_availability_check_interval is the closest thing as we want to
check if WAL is available for a walreceiver at record level and at
segment level for a restore_command.
> Am I missing something or does this handle/affect streaming failures
> just the same? That might not be a bad idea, because the current
> interval is far too long for e.g. a sync replication setup. But it
> certainly makes the name a complete misnomer.
Hm.
> Not this patch's fault, but I'm getting a bit tired seeing the above open
> coded. How about adding a function that does the sleeping based on a
> timestamptz and a ms interval?
You mean in plugins, right? I don't recall seeing similar patterns in
other code paths of backend. But I think that we can use something
like that in timestamp.c then because we need to leverage that between
two timestamps, the last failure and now():
TimestampSleepDifference(start_time, stop_time, internal_ms);
Perhaps you have something else in mind?
Attached is an updated patch.
--
Michael
From 0206c417f5b28eadb5f9d67ee0643320d7c0b621 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@otacoo.com>
Date: Mon, 19 Jan 2015 16:08:48 +0900
Subject: [PATCH] Add wal_availability_check_interval to control WAL fetching
on failure
Default value is 5s.
---
doc/src/sgml/recovery-config.sgml | 21 +++++++++++++
src/backend/access/transam/recovery.conf.sample | 10 +++++++
src/backend/access/transam/xlog.c | 39 ++++++++++++++++++++-----
src/backend/utils/adt/timestamp.c | 19 ++++++++++++
src/include/utils/timestamp.h | 2 ++
5 files changed, 83 insertions(+), 8 deletions(-)
diff --git a/doc/src/sgml/recovery-config.sgml b/doc/src/sgml/recovery-config.sgml
index 0c64ff2..7fd51ce 100644
--- a/doc/src/sgml/recovery-config.sgml
+++ b/doc/src/sgml/recovery-config.sgml
@@ -145,6 +145,27 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows
</listitem>
</varlistentry>
+ <varlistentry id="wal-availability-check-interval" xreflabel="wal_availability_check_interval">
+ <term><varname>wal_availability_check_interval</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>wal_availability_check_interval</> recovery parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ This parameter specifies the amount of time to wait when
+ WAL is not available for a node in recovery. Default value is
+ <literal>5s</>.
+ </para>
+ <para>
+ A node in recovery will wait for this amount of time if
+ <varname>restore_command</> returns nonzero exit status code when
+ fetching new WAL segment files from archive or when a WAL receiver
+ is not able to fetch a WAL record when using streaming replication.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</sect1>
diff --git a/src/backend/access/transam/recovery.conf.sample b/src/backend/access/transam/recovery.conf.sample
index b777400..70d3946 100644
--- a/src/backend/access/transam/recovery.conf.sample
+++ b/src/backend/access/transam/recovery.conf.sample
@@ -58,6 +58,16 @@
#
#recovery_end_command = ''
#
+#
+# wal_availability_check_interval
+#
+# specifies an optional interval to check for availability of WAL when
+# recovering a node. This interval of time represents the frequency of
+# retries if a previous command of restore_command returned nonzero exit
+# status code or if a walreceiver did not stream completely a WAL record.
+#
+#wal_availability_check_interval = '5s'
+#
#---------------------------------------------------------------------------
# RECOVERY TARGET PARAMETERS
#---------------------------------------------------------------------------
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 629a457..0da9f5c 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -235,6 +235,7 @@ static TimestampTz recoveryTargetTime;
static char *recoveryTargetName;
static int recovery_min_apply_delay = 0;
static TimestampTz recoveryDelayUntilTime;
+static int wal_availability_check_interval = 5000;
/* options taken from recovery.conf for XLOG streaming */
static bool StandbyModeRequested = false;
@@ -4881,6 +4882,26 @@ readRecoveryCommandFile(void)
(errmsg_internal("trigger_file = '%s'",
TriggerFile)));
}
+ else if (strcmp(item->name, "wal_availability_check_interval") == 0)
+ {
+ const char *hintmsg;
+
+ if (!parse_int(item->value, &wal_availability_check_interval, GUC_UNIT_MS,
+ &hintmsg))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("parameter \"%s\" requires a temporal value",
+ "wal_availability_check_interval"),
+ hintmsg ? errhint("%s", _(hintmsg)) : 0));
+ ereport(DEBUG2,
+ (errmsg_internal("wal_availability_check_interval = '%s'", item->value)));
+
+ if (wal_availability_check_interval <= 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("\"%s\" must have a value strictly positive",
+ "wal_availability_check_interval")));
+ }
else if (strcmp(item->name, "recovery_min_apply_delay") == 0)
{
const char *hintmsg;
@@ -10340,8 +10361,8 @@ static bool
WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
bool fetching_ckpt, XLogRecPtr tliRecPtr)
{
- static pg_time_t last_fail_time = 0;
- pg_time_t now;
+ TimestampTz now = GetCurrentTimestamp();
+ TimestampTz last_fail_time = now;
/*-------
* Standby mode is implemented by a state machine:
@@ -10490,14 +10511,16 @@ WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
* machine, so we've exhausted all the options for
* obtaining the requested WAL. We're going to loop back
* and retry from the archive, but if it hasn't been long
- * since last attempt, sleep 5 seconds to avoid
- * busy-waiting.
+ * since last attempt, sleep the amount of time specified
+ * by wal_availability_check_interval to avoid busy-waiting.
*/
- now = (pg_time_t) time(NULL);
- if ((now - last_fail_time) < 5)
+ now = GetCurrentTimestamp();
+ if (!TimestampDifferenceExceeds(last_fail_time, now,
+ wal_availability_check_interval))
{
- pg_usleep(1000000L * (5 - (now - last_fail_time)));
- now = (pg_time_t) time(NULL);
+ TimestampSleepDifference(last_fail_time, now,
+ wal_availability_check_interval);
+ now = GetCurrentTimestamp();
}
last_fail_time = now;
currentSource = XLOG_FROM_ARCHIVE;
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 67e0cf9..1098fc9 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -1674,6 +1674,25 @@ TimestampDifferenceExceeds(TimestampTz start_time,
}
/*
+ * TimestampSleepDifference -- sleep for the amout of interval time
+ * specified, reduced by the difference between two timestamps.
+ *
+ * Both inputs must be ordinary finite timestamps (in current usage,
+ * they'll be results from GetCurrentTimestamp()).
+ */
+void
+TimestampSleepDifference(TimestampTz start_time,
+ TimestampTz stop_time,
+ int interval_msec)
+{
+ long secs;
+ int microsecs;
+
+ TimestampDifference(start_time, stop_time, &secs, µsecs);
+ pg_usleep(interval_msec * 1000L - (1000000L * secs + 1L * microsecs));
+}
+
+/*
* Convert a time_t to TimestampTz.
*
* We do not use time_t internally in Postgres, but this is provided for use
diff --git a/src/include/utils/timestamp.h b/src/include/utils/timestamp.h
index 70118f5..e271d0b 100644
--- a/src/include/utils/timestamp.h
+++ b/src/include/utils/timestamp.h
@@ -217,6 +217,8 @@ extern void TimestampDifference(TimestampTz start_time, TimestampTz stop_time,
extern bool TimestampDifferenceExceeds(TimestampTz start_time,
TimestampTz stop_time,
int msec);
+extern void TimestampSleepDifference(TimestampTz start_time,
+ TimestampTz stop_time, int interval_msec);
/*
* Prototypes for functions to deal with integer timestamps, when the native
--
2.2.2
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
