Re: Automatic adjustment of bgwriter_lru_maxpages - Mailing list pgsql-patches
From | Bruce Momjian |
---|---|
Subject | Re: Automatic adjustment of bgwriter_lru_maxpages |
Date | |
Msg-id | 200705172247.l4HMlo100567@momjian.us Whole thread Raw |
In response to | Re: Automatic adjustment of bgwriter_lru_maxpages (Greg Smith <gsmith@gregsmith.com>) |
List | pgsql-patches |
Your patch has been added to the PostgreSQL unapplied patches list at: http://momjian.postgresql.org/cgi-bin/pgpatches It will be applied as soon as one of the PostgreSQL committers reviews and approves it. --------------------------------------------------------------------------- Greg Smith wrote: > Attached are two patches that try to recast the ideas of Itagaki > Takahiro's auto bgwriter_lru_maxpages patch in the direction I think this > code needs to move. Epic-length commentary follows. > > The original code came from before there was a pg_stat_bgwriter. The > first patch (buf-alloc-stats) takes the two most interesting pieces of > data the original patch collected, the number of buffers allocated > recently and the number that the clients wrote out, and ties all that into > the new stats structure. With this patch applied, you can get a feel for > things like churn/turnover in the buffer pool that were very hard to > quantify before. Also, it makes it easy to measure how well your > background writer is doing at writing buffers so the clients don't have > to. Applying this would complete one of my personal goals for the 8.3 > release, which was having stats to track every type of buffer write. > > I split this out because I think it's very useful to have regardless of > whether the automatic tuning portion is accepted, and I think these > smaller patches make the review easier. The main thing I would recommend > someone check is how am_bg_writer is (mis?)used here. I spliced some of > the debugging-only code from the original patch, and I can't tell if the > result is a robust enough approach to solving the problem of having every > client indirectly report their activity to the background writer. Other > than that, I think this code is ready for review and potentially > comitting. > > The second patch (limit-lru) adds on top of that a constraint of the LRU > writer so that it doesn't do any more work than it has to. Note that I > left verbose debugging code in here because I'm much less confident this > patch is complete. > > It predicts upcoming buffer allocations using a 16-period weighted moving > average of recent activity, which you can think of as the last 3.2 seconds > at the default interval. After testing a few systems that seemed a decent > compromise of smoothing in both directions. I found the 2X overallocation > fudge factor of the original patch way too aggressive, and just pick the > larger of the most recent allocation amount or the smoothed value. The > main thing that throws off the allocation estimation is when you hit a > checkpoint, which can give a big spike after the background writer returns > to BgBufferSync and notices all the buffers that were allocated during the > checkpoint write; the code then tries to find more buffers it can recycle > than it needs to. Since the checkpoint itself normally leaves a large > wake of reusable buffers behind it, I didn't find this to be a serious > problem. > > There's another communication issue here, which is that SyncOneBuffer > needs to return more information about the buffer than it currently does > once it gets it locked. The background writer needs to know more than > just if it was written to tune itself. The original patch used a clever > trick for this which worked but I found confusing. I happen to have a > bunch of other background writer tuning code I'm working on, and I had to > come up with a more robust way to communicate buffer internals back via > this channel. I used that code here, it's a bitmask setup similar to how > flags like BM_DIRTY are used. It's overkill for solving this particular > problem, but I think the interface is clean and it helps support future > enhancements in intelligent background writing. > > Now we get to the controversial part. The original patch removed the > bgwriter_lru_maxpages parameter and updated the documentation accordingly. > I didn't do that here. The reason is that after playing around in this > area I'm not convinced yet I can satisfy all the tuning scenarios I'd like > to be able to handle that way. I describe this patch as enforcing a > constraint instead; it allows you to set the LRU parameters much higher > than was reasonable before without having to be as concerned about the LRU > writer wasting resources. > > I already brought up some issues in this area on -hackers ( > http://archives.postgresql.org/pgsql-hackers/2007-04/msg00781.php ) but my > work hasn't advanced as fast as I'd hoped. I wanted to submit what I've > finished anyway because I think any approach here is going to have cope > with the issues addressed in these two patches, and I'm happy now with how > they're solved here. It's only a one-line delete to disable the LRU > limiting behavior of the second patch, at which point it's strictly > internals code with no expected functional impact that alternate > approaches might be built on. > > -- > * Greg Smith gsmith@gregsmith.com http://www.gregsmith.com Baltimore, MD Content-Description: [ Attachment, skipping... ] Content-Description: [ Attachment, skipping... ] > > ---------------------------(end of broadcast)--------------------------- > TIP 5: don't forget to increase your free space map settings -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
pgsql-patches by date: