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:

Previous
From: Bruce Momjian
Date:
Subject: Re: Updated bitmap index patch
Next
From: Tom Lane
Date:
Subject: Re: UTF8MatchText