pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedCommitfest patches
11+ messages / 6 participants
[nested] [flat]
* Commitfest patches
@ 2008-03-28 09:08 Gregory Stark <stark@enterprisedb.com>
2008-03-28 09:44 ` Re: Commitfest patches Simon Riggs <simon@2ndquadrant.com>
2008-03-28 15:45 ` Re: Commitfest patches Bruce Momjian <bruce@momjian.us>
0 siblings, 2 replies; 11+ messages in thread
From: Gregory Stark @ 2008-03-28 09:08 UTC (permalink / raw)
To: pgsql-hackers
Bruce, you seem to have removed one of my three patches from the queue. I
would actually prefer you remove the other two and put back that one. It's the
one I most urgently need feedback on to continue.
The patch I'm so interested in receiving feedback on is the patch to preread
pages in bitmap index scans using posix_fadvise. This is basically complete
modulo documentation and autoconf tests.
However there is a larger patch struggling to get out here. The patch I
submitted just adds a new low level routine to preread blocks and doesn't
change the buffer manager routines at all. That means we have to make two
trips through the buffer manager to check if the block is present and then
subsequently to assign a buffer and read it in (from cache) synchronously.
A more invasive form of this patch would be to assign and pin a buffer when
the preread is done. That would men subsequently we would have a pinned buffer
ready to go and not need to go back to the buffer manager a second time. We
would instead just "complete" the i/o by issuing a normal read call.
The up-side would be fewer trips through the buffer manager and attendant
locking. The down-side would be stealing buffers from the shared buffers which
could be holding other pages.
I don't feel like writing the more invasive patch only to throw it out and
take the one I've already written, but I don't know what other people's
feelings are between the two. I'm leaning towards the more invasive buffer
manager changes myself.
--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's Slony Replication support!
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Commitfest patches
2008-03-28 09:08 Commitfest patches Gregory Stark <stark@enterprisedb.com>
@ 2008-03-28 09:44 ` Simon Riggs <simon@2ndquadrant.com>
2008-03-28 11:26 ` Re: Commitfest patches Gregory Stark <stark@enterprisedb.com>
1 sibling, 1 reply; 11+ messages in thread
From: Simon Riggs @ 2008-03-28 09:44 UTC (permalink / raw)
To: Gregory Stark <stark@enterprisedb.com>; +Cc: pgsql-hackers
On Fri, 2008-03-28 at 09:08 +0000, Gregory Stark wrote:
> A more invasive form of this patch would be to assign and pin a buffer when
> the preread is done. That would men subsequently we would have a pinned buffer
> ready to go and not need to go back to the buffer manager a second time. We
> would instead just "complete" the i/o by issuing a normal read call.
So if posix_fadvise did nothing or there was a longer than optimal
delay, this would be a net loss.
You'd need reasonable evidence that the posix_fadvise facility was a win
on all platforms and recent release levels before we could agree with
that.
I think we need a more thorough examination of this area before we
commit anything. Maybe you've done this, but I haven't seen the
analysis. Can you say more, please? Or at least say what you don't know,
so other experts listening can fill in the blanks.
--
Simon Riggs
2ndQuadrant http://www.2ndQuadrant.com
PostgreSQL UK 2008 Conference: http://www.postgresql.org.uk
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Commitfest patches
2008-03-28 09:08 Commitfest patches Gregory Stark <stark@enterprisedb.com>
2008-03-28 09:44 ` Re: Commitfest patches Simon Riggs <simon@2ndquadrant.com>
@ 2008-03-28 11:26 ` Gregory Stark <stark@enterprisedb.com>
2008-03-28 11:43 ` Re: Commitfest patches Simon Riggs <simon@2ndquadrant.com>
2008-03-30 05:02 ` Re: Commitfest patches Greg Smith <gsmith@gregsmith.com>
0 siblings, 2 replies; 11+ messages in thread
From: Gregory Stark @ 2008-03-28 11:26 UTC (permalink / raw)
To: Simon Riggs <simon@2ndquadrant.com>; +Cc: pgsql-hackers
"Simon Riggs" <simon@2ndquadrant.com> writes:
> On Fri, 2008-03-28 at 09:08 +0000, Gregory Stark wrote:
>
>> A more invasive form of this patch would be to assign and pin a buffer when
>> the preread is done. That would men subsequently we would have a pinned buffer
>> ready to go and not need to go back to the buffer manager a second time. We
>> would instead just "complete" the i/o by issuing a normal read call.
>
> So if posix_fadvise did nothing or there was a longer than optimal
> delay, this would be a net loss.
>
> You'd need reasonable evidence that the posix_fadvise facility was a win
> on all platforms and recent release levels before we could agree with
> that.
I posted a test program and asked for others to test it on various platforms
and various sized raid arrays. matthew@flymine.org was the only person who
bothered to run it and he only found a 16x speedup on his hardware.
http://archives.postgresql.org/pgsql-performance/2008-01/msg00285.php
(unfortunately our mail archives seem to have failed to archive his message,
this is my followup to it)
> I think we need a more thorough examination of this area before we
> commit anything. Maybe you've done this, but I haven't seen the
> analysis. Can you say more, please? Or at least say what you don't know,
> so other experts listening can fill in the blanks.
For heaven's sake. I've been posting about this for months. Search the
archives for "RAID arrays and performance", "There's random access and then
there's random access", and "Bitmap index scan preread using posix_fadvise".
I described which interfaces worked on Linux and Solaris based on empirical
tests. I posted source code for synthetic benchmarks so we could test it on a
wide range of hardware. I posted graphs based on empirical results. I posted
mathematical formulas analysing just how much preread would be expected to
exercise a raid array fully. I'm not sure what else I can do to effect a more
thorough examination.
--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's PostGIS support!
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Commitfest patches
2008-03-28 09:08 Commitfest patches Gregory Stark <stark@enterprisedb.com>
2008-03-28 09:44 ` Re: Commitfest patches Simon Riggs <simon@2ndquadrant.com>
2008-03-28 11:26 ` Re: Commitfest patches Gregory Stark <stark@enterprisedb.com>
@ 2008-03-28 11:43 ` Simon Riggs <simon@2ndquadrant.com>
1 sibling, 0 replies; 11+ messages in thread
From: Simon Riggs @ 2008-03-28 11:43 UTC (permalink / raw)
To: Gregory Stark <stark@enterprisedb.com>; +Cc: pgsql-hackers
On Fri, 2008-03-28 at 11:26 +0000, Gregory Stark wrote:
> "Simon Riggs" <simon@2ndquadrant.com> writes:
> For heaven's sake. I've been posting about this for months.
Any chance of getting all that together on a single Wiki page, so we can
review everything? We'll need those docs after its committed anyway.
--
Simon Riggs
2ndQuadrant http://www.2ndQuadrant.com
PostgreSQL UK 2008 Conference: http://www.postgresql.org.uk
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Commitfest patches
2008-03-28 09:08 Commitfest patches Gregory Stark <stark@enterprisedb.com>
2008-03-28 09:44 ` Re: Commitfest patches Simon Riggs <simon@2ndquadrant.com>
2008-03-28 11:26 ` Re: Commitfest patches Gregory Stark <stark@enterprisedb.com>
@ 2008-03-30 05:02 ` Greg Smith <gsmith@gregsmith.com>
2008-04-05 02:25 ` Re: Commitfest patches Gregory Stark <stark@enterprisedb.com>
1 sibling, 1 reply; 11+ messages in thread
From: Greg Smith @ 2008-03-30 05:02 UTC (permalink / raw)
To: Gregory Stark <stark@enterprisedb.com>; +Cc: pgsql-hackers
On Fri, 28 Mar 2008, Gregory Stark wrote:
> I described which interfaces worked on Linux and Solaris based on empirical
> tests. I posted source code for synthetic benchmarks so we could test it on a
> wide range of hardware. I posted graphs based on empirical results.
Is it possible to post whatever script that generates the graph (gnuplot?)
so people can compare the results they get to yours? It's when I realized
I didn't have that and would have to recreate one myself that my intention
to just run some quick results and ship them to you lost momentum and I
never circled back to it. It would be nice if you made it easier for
people to generate fancy results here as immediate gratification for them
(while of course just asking them to send the raw data).
I can run some tests on smaller Linux/Solaris systems to see if they don't
show a regression, that was my main concern about this experiment. Some
of the discussion that followed your original request for tests was kind
of confusing as far as how to interpret the results as well; I think I
know what to look for but certainly wouldn't mind some more guidance
there, too.
--
* Greg Smith gsmith@gregsmith.com http://www.gregsmith.com Baltimore, MD
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Commitfest patches
2008-03-28 09:08 Commitfest patches Gregory Stark <stark@enterprisedb.com>
2008-03-28 09:44 ` Re: Commitfest patches Simon Riggs <simon@2ndquadrant.com>
2008-03-28 11:26 ` Re: Commitfest patches Gregory Stark <stark@enterprisedb.com>
2008-03-30 05:02 ` Re: Commitfest patches Greg Smith <gsmith@gregsmith.com>
@ 2008-04-05 02:25 ` Gregory Stark <stark@enterprisedb.com>
0 siblings, 0 replies; 11+ messages in thread
From: Gregory Stark @ 2008-04-05 02:25 UTC (permalink / raw)
To: Greg Smith <gsmith@gregsmith.com>; +Cc: pgsql-hackers
"Greg Smith" <gsmith@gregsmith.com> writes:
> On Fri, 28 Mar 2008, Gregory Stark wrote:
>
>> I described which interfaces worked on Linux and Solaris based on empirical
>> tests. I posted source code for synthetic benchmarks so we could test it on a
>> wide range of hardware. I posted graphs based on empirical results.
>
> Is it possible to post whatever script that generates the graph (gnuplot?) so
> people can compare the results they get to yours?
Unfortunately I couldn't figure out how to get gnuplot to do this. I ended up
doing it in gnumeric. I'll look into fixing it the script up to be more
automatic. Maybe look at gnuplot again.
> I can run some tests on smaller Linux/Solaris systems to see if they don't show
> a regression, that was my main concern about this experiment.
Heikki suggested some things to test for regressions. I'll look at that.
What I'm more curious about and hoped I could get data from others is whether
posix_fadvise(WILL_NEED) does anything useful on various versions of FreeBSD,
OSX, etc. Afaict the syscall doesn't exist at all on Solaris though, which
surprises me :(
--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's RemoteDBA services!
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Commitfest patches
2008-03-28 09:08 Commitfest patches Gregory Stark <stark@enterprisedb.com>
@ 2008-03-28 15:45 ` Bruce Momjian <bruce@momjian.us>
2008-03-28 17:34 ` Re: Commitfest patches Gregory Stark <stark@enterprisedb.com>
1 sibling, 1 reply; 11+ messages in thread
From: Bruce Momjian @ 2008-03-28 15:45 UTC (permalink / raw)
To: Gregory Stark <stark@enterprisedb.com>; +Cc: pgsql-hackers
Gregory Stark wrote:
>
> Bruce, you seem to have removed one of my three patches from the queue. I
> would actually prefer you remove the other two and put back that one. It's the
> one I most urgently need feedback on to continue.
I talked to Greg on IM. The complaint was that his posix_fadvise()
patch was added to the TODO list as a URL, rather than getting him
feedback. He is getting feedback now in another thread.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Commitfest patches
2008-03-28 09:08 Commitfest patches Gregory Stark <stark@enterprisedb.com>
2008-03-28 15:45 ` Re: Commitfest patches Bruce Momjian <bruce@momjian.us>
@ 2008-03-28 17:34 ` Gregory Stark <stark@enterprisedb.com>
2008-03-28 18:15 ` Re: Commitfest patches Heikki Linnakangas <heikki@enterprisedb.com>
2008-03-28 18:48 ` Re: Commitfest patches Martijn van Oosterhout <kleptog@svana.org>
0 siblings, 2 replies; 11+ messages in thread
From: Gregory Stark @ 2008-03-28 17:34 UTC (permalink / raw)
To: Bruce Momjian <bruce@momjian.us>; +Cc: pgsql-hackers
> Gregory Stark wrote:
>>
>> Bruce, you seem to have removed one of my three patches from the queue. I
>> would actually prefer you remove the other two and put back that one. It's the
>> one I most urgently need feedback on to continue.
>
> I talked to Greg on IM. The complaint was that his posix_fadvise()
> patch was added to the TODO list as a URL, rather than getting him
> feedback. He is getting feedback now in another thread.
Sort of. The feedback I'm getting is from people who want to discuss the
"easy" parameters of the patch like how much prefetching to do and when
without actually reading to see what's already been done. I'm happy to discuss
them as I find these things interesting too.
But what I really need is someone to read the patch and say "looks good" or
point out things they don't like. In particular, what I really, really want is
some guidance on the singular key question I asked.
I want to know if we're interested in the more invasive patch restructuring
the buffer manager. My feeling is that we probably are eventually. But I
wonder if people wouldn't feel more comfortable taking baby steps at first
which will have less impact in cases where it's not being heavily used.
It's a lot more work to do the invasive patch and there's not much point in
doing it if we're not interested in taking it when it's ready.
Here's an updated patch. It's mainly just a CVS update but it also includes
some window dressing: an autoconf test, some documentation, and some fancy
arithmetic to auto-tune the amount of prefetching based on a more real-world
parameter "effective_spindle_count". It also includes printing out how much
the prefetching target got ramped up to in the explain output -- though I'm
not sure we really want that in the tree, but it's nice for performance
testing.
--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's 24x7 Postgres support!
Attachments:
bitmap-preread-v9.diff.gz (application/octet-stream, 9.1K) download
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Commitfest patches
2008-03-28 09:08 Commitfest patches Gregory Stark <stark@enterprisedb.com>
2008-03-28 15:45 ` Re: Commitfest patches Bruce Momjian <bruce@momjian.us>
2008-03-28 17:34 ` Re: Commitfest patches Gregory Stark <stark@enterprisedb.com>
@ 2008-03-28 18:15 ` Heikki Linnakangas <heikki@enterprisedb.com>
1 sibling, 0 replies; 11+ messages in thread
From: Heikki Linnakangas @ 2008-03-28 18:15 UTC (permalink / raw)
To: Gregory Stark <stark@enterprisedb.com>; +Cc: Bruce Momjian <bruce@momjian.us>; pgsql-hackers
Gregory Stark wrote:
> I want to know if we're interested in the more invasive patch restructuring
> the buffer manager. My feeling is that we probably are eventually. But I
> wonder if people wouldn't feel more comfortable taking baby steps at first
> which will have less impact in cases where it's not being heavily used.
>
> It's a lot more work to do the invasive patch and there's not much point in
> doing it if we're not interested in taking it when it's ready.
I like the simplicity of this patch. My biggest worry is the impact on
performance when the posix_fadvise calls are not helping. I'd like to
see some testing of that. How expensive are the extra bufmgr hash table
lookups, compared to all the other stuff that's involved in a bitmap
heap scan?
Another thing I wonder is whether this approach can easily be extended
to other types of scans than bitmap heap scans. Normal index scans seem
most interesting; they can generate a lot of random I/O. I don't see any
big problems there, except again the worst-case performance: If you
issue AdviseBuffer calls for all the heap tuples in an index page, in
the naive way, you can issue hundreds of posix_fadvise calls. But what
if they're all actually on the same page? Surely you only want to go
through the buffer manager once (like we do in the scan, thanks to
ReleaseAndReadBuffer()) in that case, and only call posix_fadvise once.
But again, maybe you can convince me that it's a non-issue by some
benchmarking.
> Here's an updated patch. It's mainly just a CVS update but it also includes
> some window dressing: an autoconf test, some documentation, and some fancy
> arithmetic to auto-tune the amount of prefetching based on a more real-world
> parameter "effective_spindle_count".
I like the "effective_spindle_count". That's very intuitive.
The formula to turn that into preread_pages looks complex, though. I can
see the reasoning behind it, but I doubt thing behave that beautifully
in the real world. (That's not important right now, we have plenty of
time to tweak that after more testing.)
> It also includes printing out how much
> the prefetching target got ramped up to in the explain output -- though I'm
> not sure we really want that in the tree, but it's nice for performance
> testing.
I don't understand the ramp up logic. Can you explain what that's for
and how it works?
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Commitfest patches
2008-03-28 09:08 Commitfest patches Gregory Stark <stark@enterprisedb.com>
2008-03-28 15:45 ` Re: Commitfest patches Bruce Momjian <bruce@momjian.us>
2008-03-28 17:34 ` Re: Commitfest patches Gregory Stark <stark@enterprisedb.com>
@ 2008-03-28 18:48 ` Martijn van Oosterhout <kleptog@svana.org>
2008-04-05 02:21 ` Re: Commitfest patches Gregory Stark <stark@enterprisedb.com>
1 sibling, 1 reply; 11+ messages in thread
From: Martijn van Oosterhout @ 2008-03-28 18:48 UTC (permalink / raw)
To: Gregory Stark <stark@enterprisedb.com>; +Cc: Bruce Momjian <bruce@momjian.us>; pgsql-hackers
On Fri, Mar 28, 2008 at 05:34:30PM +0000, Gregory Stark wrote:
> But what I really need is someone to read the patch and say "looks good" or
> point out things they don't like. In particular, what I really, really want is
> some guidance on the singular key question I asked.
I was going to write all sorts of stuff, till I noticed Heikki said
basically everything I was going to say:
- I think normal index scans could benefit from this (it was measurable
when I was playing with AIO in postgres a few years back).
- The integration with the bitmap scan is good, neat even
- I think the number of preread_count is far too small, given you get a
benefit even if you only have one spindle.
- I don't understand the ramp-up method either.
People spend a lot of time worrying about hundreds of posix_fadvise()
calls but you don't need anywhere near that much to be effective. With
AIO I limited the number of outstanding requests to a dozen and it was
still useful. You lose nothing by capping the number of requests at any
point.
> I want to know if we're interested in the more invasive patch restructuring
> the buffer manager. My feeling is that we probably are eventually. But I
> wonder if people wouldn't feel more comfortable taking baby steps at first
> which will have less impact in cases where it's not being heavily used.
I think the way it is now is neat and simple and enough for now.
Have a nice day,
--
Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/
> Please line up in a tree and maintain the heap invariant while
> boarding. Thank you for flying nlogn airlines.
Attachments:
signature.asc (application/pgp-signature, 188B) download
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Commitfest patches
2008-03-28 09:08 Commitfest patches Gregory Stark <stark@enterprisedb.com>
2008-03-28 15:45 ` Re: Commitfest patches Bruce Momjian <bruce@momjian.us>
2008-03-28 17:34 ` Re: Commitfest patches Gregory Stark <stark@enterprisedb.com>
2008-03-28 18:48 ` Re: Commitfest patches Martijn van Oosterhout <kleptog@svana.org>
@ 2008-04-05 02:21 ` Gregory Stark <stark@enterprisedb.com>
0 siblings, 0 replies; 11+ messages in thread
From: Gregory Stark @ 2008-04-05 02:21 UTC (permalink / raw)
To: Martijn van Oosterhout <kleptog@svana.org>; +Cc: Bruce Momjian <bruce@momjian.us>; pgsql-hackers
"Martijn van Oosterhout" <kleptog@svana.org> writes:
> - I think normal index scans could benefit from this (it was measurable
> when I was playing with AIO in postgres a few years back).
I don't want to torture any existing code paths to get prefetching to work.
Heikki suggested I take advantage of the page-at-a-time index scanning though
which does sound like it could be convenient.
> - I think the number of preread_count is far too small, given you get a
> benefit even if you only have one spindle.
> - I don't understand the ramp-up method either.
So the idea is that I was deathly afraid of being accused of doing unnecessary
additional I/O. So I was worried about the user doing something like SELECT
... LIMIT 1. Or worse, just starting a select and discarding it after only
selecting a handful of records.
I also didn't want to start the bitmap scan by doing hundreds of syscalls
before we even return the first record.
So I figured it's safer to read only the first block at first. Only if the
user goes on to the next record do we try prefetching the next block. Each
record the user reads we bump up the prefetch amount by 1 until we hit the
goal value for the size raid array we're using.
That also nicely spreads out the syscalls so we get one prefetch between each
record returned.
We also don't know how densely packed the records are on the pages. If they're
densely packed then we'll have prefetched a whole bunch of records in time for
the second page. But if there's only one record on a page and we're already on
the second page I figured that indicated we would be reading many pages with
sparsely distributed records. So we ramp up the prefetch exponentially by
multiplying by two each time we move to the next page.
The net result is that if you do a bitmap scan which is processing lots of
pointers it'll quickly reach the point where it's prefetching the number of
blocks based on effective_spindle_count. If you're just processing the first
few tuples it'll only read a small number of extra pages. And if you're only
processing one record it won't prefetch at all.
> People spend a lot of time worrying about hundreds of posix_fadvise()
> calls but you don't need anywhere near that much to be effective. With
> AIO I limited the number of outstanding requests to a dozen and it was
> still useful. You lose nothing by capping the number of requests at any
> point.
Well you leave money on the table. But yes, I'm trying to be conservative
about how much to prefetch when we don't know that it's in our favour.
>> I want to know if we're interested in the more invasive patch restructuring
>> the buffer manager. My feeling is that we probably are eventually. But I
>> wonder if people wouldn't feel more comfortable taking baby steps at first
>> which will have less impact in cases where it's not being heavily used.
>
> I think the way it is now is neat and simple and enough for now.
Thanks.
--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Get trained by Bruce Momjian - ask me about EnterpriseDB's PostgreSQL training!
^ permalink raw reply [nested|flat] 11+ messages in thread
end of thread, other threads:[~2008-04-05 02:25 UTC | newest]
Thread overview: 11+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2008-03-28 09:08 Commitfest patches Gregory Stark <stark@enterprisedb.com>
2008-03-28 09:44 ` Simon Riggs <simon@2ndquadrant.com>
2008-03-28 11:26 ` Gregory Stark <stark@enterprisedb.com>
2008-03-28 11:43 ` Simon Riggs <simon@2ndquadrant.com>
2008-03-30 05:02 ` Greg Smith <gsmith@gregsmith.com>
2008-04-05 02:25 ` Gregory Stark <stark@enterprisedb.com>
2008-03-28 15:45 ` Bruce Momjian <bruce@momjian.us>
2008-03-28 17:34 ` Gregory Stark <stark@enterprisedb.com>
2008-03-28 18:15 ` Heikki Linnakangas <heikki@enterprisedb.com>
2008-03-28 18:48 ` Martijn van Oosterhout <kleptog@svana.org>
2008-04-05 02:21 ` Gregory Stark <stark@enterprisedb.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox