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.
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.
regards, tom lane