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 [EMAIL PROTECTED] 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  <[EMAIL PROTECTED]>          http://momjian.us
  EnterpriseDB                               http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

               http://archives.postgresql.org

Reply via email to