Re: [HACKERS] Patch: add recovery_timeout option to control timeout of restore_command nonzero status code

On Sat, Dec 27, 2014 at 3:42 AM, Alexey Vasiliev <leopard_ne@inbox.ru> wrote:

Cool, thanks. I just had an extra look at it.

+ This is useful, if I using for restore of wal logs some
+ external storage (like AWS S3) and no matter what the slave database
+ will lag behind the master. The problem, what for each request to
+ AWS S3 need to pay, what is why for N nodes, which try to get next
+ wal log each 5 seconds will be bigger price, than for example each
+ 30 seconds.
I reworked this portion of the docs, it is rather incorrect as the
documentation should not use first-person subjects, and I don't
believe that referencing any commercial products is a good thing in
this context.

+# specifies an optional timeout after nonzero code of restore_command.
+# This can be useful to increase/decrease number of a restore_command calls.
This is still referring to a timeout. That's not good. And the name of
the parameter at the top of this comment block is missing.

+static int restore_command_retry_interval = 5000L;
I think that it would be more adapted to set that to 5000, and
multiply by 1L. I am also wondering about having a better lower bound,
like 100ms to avoid some abuse with this feature in the retries?

+ ereport(ERROR,
+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("\"%s\" must
be bigger zero",
+ "restore_command_retry_interval")));
I'd rather rewrite that to "must have a strictly positive value".

- * Wait for more WAL to
arrive. Time out after 5 seconds,
+ * Wait for more WAL to
arrive. Time out after
+ *
restore_command_retry_interval (5 seconds by default),
                                         * like when polling the
archive, to react to a trigger
                                         * file promptly.
                                         */
                                        WaitLatch(&XLogCtl->recoveryWakeupLatch,
                                                          WL_LATCH_SET
| WL_TIMEOUT,
- 5000L);
+
restore_command_retry_interval);
I should have noticed earlier, but in its current state your patch
actually does not work. What you are doing here is tuning the time
process waits for WAL from stream. In your case what you want to
control is the retry time for a restore_command in archive recovery,
no?
--
Michael


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers