Hi, 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? > +# 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? > + 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. > + now = GetCurrentTimestamp(); > + if > (!TimestampDifferenceExceeds(last_fail_time, now, > + > restore_command_retry_interval)) > { > - pg_usleep(1000000L * (5 - (now > - last_fail_time))); > - now = (pg_time_t) time(NULL); > + long secs; > + int > microsecs; > + > + > TimestampDifference(last_fail_time, now, &secs, µsecs); > + > pg_usleep(restore_command_retry_interval * 1000L - (1000000L * secs + 1L * > microsecs)); > + now = GetCurrentTimestamp(); > } > last_fail_time = now; > currentSource = XLOG_FROM_ARCHIVE; 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. Not this patch's fault, but I'm getting a bit tired seing the above open coded. How about adding a function that does the sleeping based on a timestamptz and a ms interval? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers