Re: bgwriter idle-mode behavior (was Re: Latch for the WAL writer) - Mailing list pgsql-hackers

On 10.05.2012 00:34, Tom Lane wrote:
> After further study of the bgwriter hibernation patch (commit
> 6d90eaaa89a007e0d365f49d6436f35d2392cfeb), I think that my worries about
> race conditions in the use of the bgwriter's latch are really the least
> of its problems.  BgBufferSync embodies a rather carefully tuned
> feedback control loop, and I think these changes broke it.  In the first
> place, that feedback loop is built on the assumption that BgBufferSync
> is executed at fixed intervals, which isn't true anymore; and in the
> second place, the loop is not driven so much by the rate of buffers
> being dirtied as it is by the rate of buffers being allocated.  To be
> concrete, if there is a constant but slow rate of buffer consumption,
> the bgwriter will eventually lap the strategy scan and then stay there,
> resulting in BgBufferSync returning true every time even though actually
> the system is doing things.  This results in bgwriter.c deciding it's in
> hibernation mode, whereupon we have a scenario where backends will be
> signaling it all the time.  The way BgWriterNap is coded, that means
> BgBufferSync is not executed at a fixed BgWriterDelay interval, but at
> variable intervals from BgWriterDelay up to BGWRITER_HIBERNATE_MS, which
> pretty much destroys the predictability of the feedback loop.
>
> My proposal for fixing this is that
>
> (1) BgBufferSync should return true (OK to hibernate) only if it's
> lapped the strategy scan *and* recent_alloc is zero, meaning no new
> buffers were allocated anywhere since last time.
>
> (2) We should remove the bgwriter wakening calls from MarkBufferDirty
> and SetBufferCommitInfoNeedsSave, and instead place one in buffer
> allocation.

Hmm, that means that if you don't dirty any pages, bgwriter will wake up 
even though it has no real work to do. It only wakes up to advance its 
scan. I guess that's OK, if the system is busy doing read-only things 
anyway, a few extra wakeups don't matter.

Waking bgwriter in buffer allocation makes sense also because the buffer 
allocation stats will be updated more promptly that way. At the moment, 
if the bgwriter hibernates, the bgwriter only updates the stats every 10 
seconds.

> (3) The bottom-of-loop logic in bgwriter should be along the lines of
>
>     rc = WaitLatch(..., BgWriterDelay);
>     if (rc == WL_TIMEOUT&&  can_hibernate)
>     {
>         set global flag to tell backends to kick bgwriter
>             if they allocate a buffer;
>         WaitLatch(..., BGWRITER_HIBERNATE_MS);
>         clear global flag;
>     }
>
> In comparison to the existing code, this method guarantees immediate
> response to any signal (latch-setting event), and it ensures that
> if we extend the normal sleep time for the bgwriter, the extra sleep
> covers only an interval in which no new buffer allocations happened.
> That provision seems to me to justify pretending that that interval
> simply didn't exist for the purposes of the feedback control loop,
> which allows us to not have to rethink how that loop works.
>
> Comments?

Seems reasonable. Would you like me to write a patch, or are you already 
on it?

--   Heikki Linnakangas  EnterpriseDB   http://www.enterprisedb.com