On 7 May 2012 18:09, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Peter Geoghegan <peter@2ndquadrant.com> writes:
>> This latest revision also covers the checkpointer. The code for that
>> is far simpler than that for the WAL Writer, so it doesn't
>> particularly feel like I'm pushing my luck by slipping that into
>> something to be slipped in.
>
> Well ... maybe, or maybe not, or maybe you are just poking at a sore
> spot that was already created by the patch to make a separate
> checkpointer process. What bothers me in looking at this is that the
> main loop of the checkpointer includes an AbsorbFsyncRequests() call,
> which is now the only wakeup condition that isn't covered by latch
> logic or a predictable time delay. A long sleep period could easily
> result in overflow of the fsync request queue, which is not good for
> performance. I'm inclined to think that we'd better add logic to
> ForwardFsyncRequest() to set the latch once the queue is, say, more
> than half full.
OK
> I also notice that the separate-checkpointer patch failed to rename
> assorted things like BgWriterCommLock, BgWriterRequest,
> BgWriterShmemStruct, which are all 100% inappropriately named now.
> And it still contains various obsolete comments referring to itself
> as the background writer. Will see about cleaning that up.
For want of a better name, keeping them the same seemed best.
If you have a suggested name change, I'd be happy to oblige.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services