On Sat, Nov 24, 2018 at 11:29:22AM -0500, Jeff Janes wrote:
> I noticed that the existing codebase does not have a consensus on what to
> pass to WaitLatch for the timeout when the timeout isn't relevant. I picked
> 0, but -1L also has precedent.
WaitLatch enforces the timeout to -1 if WL_TIMEOUT is not given to the
caller, so setting a value does not matter much. If WL_TIMEOUT is
defined, the code would blow up on an assertion if the timeout is
negative. In short I would let the timeout be set to 5000L, but just
add the flag on the fly instead of having a if/else with two calls of
WaitLatch() as your patch does.
"git diff --check" complains about a whitespace issue.
Anyway, the timeout is useful to have for another thing: we check if
the WAL receiver is still alive thanks to that periodically, and this
even if no promote file is defined. That's useful for failure
handling if the WAL receiver gets killed so as the startup process can
define more quickly a new source (or just create a new WAL sender) it
should use for feeding with fresh WAL data, no?
It seems to me that the comment on top of WaitLatch should be clearer
about that, and that the current state leads to confusion. Another
thing coming to my mind is that I think it would be useful to make the
timeout configurable so as instances can react more quickly in the
case of a sudden death of the WAL receiver (or to check faster for a
trigger file if the HA application is to lazy to send a signal to the
standby host).
Attached is a patch to improve the comment for now.
Thoughts?
--
Michael