From: | Greg Smith <gsmith(at)gregsmith(dot)com> |
---|---|
To: | pgsql-patches(at)postgresql(dot)org |
Subject: | Re: Automatic adjustment of bgwriter_lru_maxpages |
Date: | 2007-05-13 01:32:29 |
Message-ID: | Pine.GSO.4.64.0705121926420.9615@westnet.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers pgsql-patches pgsql-performance |
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(at)gregsmith(dot)com http://www.gregsmith.com Baltimore, MD
Attachment | Content-Type | Size |
---|---|---|
limit-lru.patch | text/plain | 9.4 KB |
buf-alloc-stats.patch | application/octet-stream | 24.5 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2007-05-13 02:05:35 | What is happening on buildfarm member baiji? |
Previous Message | Greg Smith | 2007-05-12 23:24:51 | Re: Performance monitoring |
From | Date | Subject | |
---|---|---|---|
Next Message | Neil Conway | 2007-05-13 04:41:24 | Re: Performance monitoring |
Previous Message | Greg Smith | 2007-05-12 23:24:51 | Re: Performance monitoring |
From | Date | Subject | |
---|---|---|---|
Next Message | Andrew McMillan | 2007-05-13 06:34:20 | Re: Best OS for Postgres 8.2 |
Previous Message | Jim C. Nasby | 2007-05-12 16:55:41 | Re: Kernel cache vs shared_buffers |