Peter Geoghegan <peter@2ndquadrant.com> writes:
> Attached patch removes the questionable SetLatch() call, under the
> assumption that it's okay if the WALWriter, having entered hibernation
> due to sustained inactivity (10 seconds) subsequently takes up to 5
> seconds (2.5 on average) to notice that it has work to do. These
> values may need to be tweaked. I have not bothered with making the
> sleep time adaptive, because it's probably too late to do that.
Now that I've actually read the patch, rather than just responding to
your description of it, I find myself entirely unhappy with the proposed
changes in the walwriter's sleep logic. You have introduced race
conditions (it is NOT okay to reset the latch somewhere below the top of
the loop) and degraded the walwriter's signal response time in normal
non-hibernation state, to solve a problem not in evidence; to wit that
backends spend too much time signaling the walwriter. Given the
location of the only existing SetLatch call for the purpose, I find that
proposition more than a bit doubtful. I see little or no value in
trying to keep the walwriter's procLatch set when it's not hibernating,
and I definitely don't think it is worth the risk of introducing bugs
when we're about to start beta. I'm intending to rip all that out and
go back to the plain "ResetLatch at the top of the loop, WaitLatch at
the bottom" design, with the hibernation logic just controlling the
timeout on the WaitLatch call.
regards, tom lane