pg.ddx.io  pgsql-patches@postgresql.org mailing list archive  
help / color / mirror / Atom feed
posix advises ...
13+ messages / 7 participants
[nested] [flat]

* posix advises ...
@ 2008-05-11 09:53 Hans-Juergen Schoenig <postgres@cybertec.at>
  2008-05-14 02:04 ` Re: posix advises ... Greg Smith <gsmith@gregsmith.com>
  0 siblings, 1 reply; 13+ messages in thread

From: Hans-Juergen Schoenig @ 2008-05-11 09:53 UTC (permalink / raw)
  To: pgsql-patches; Zoltan Boszormenyi <zb@cybertec.at>

hello everybody,

recently we had a bit of a nightmare with some kernels and concurrent 
seq scans.
the thing we encountered was the following: a single "SELECT COUNT(*) 
FROM table" on a big table (50 gb) gave us constant 350 mb / sec I/O. as 
soon as a second scan dropped in speed dropped to 2 mb / sec. first i 
thought that some random I/O dropped in but synchronous scans worked 
fine. we found out that there is some madness in some linux kernel / 
controller combinations causing this issue.
it did some tests on my local boxes which was clearly not affected by 
this problem and I have seen a single SATA disks dropping from 65 mb / 
sec to around 45. this is not good.
i found a patch by grep stark implementing posix_fadvise for bitmap 
scans. i quickly hacked in suggestions to issue the same advises when a 
seq scan is done.
the impact was surprisingly high. single scans went up from 65 mb / sec 
to something around 70. concurrent scans are basically at steady, high 
speed - no dropping I/O speed anymore until something like 16 scans or so.
even the broken controller when up from "350mb -> 2mb" to "350 -> 50mb". 
by replacing the kernel and the driver we see steady behavior here as 
well now.

maybe it is worth to discuss posix_fadvise.
we hacked up a simple patch based on greg's work which nicely fixed the 
problem for us (brute force).
we also made some simple autoconf hack to check for broken posix_fadvise.
maybe people want to test if they see similar performance differences.
if a patch like that is likely to be accepted we would hack up some more 
clean implementation.

    many thanks,

       hans

-- 
Cybertec Schönig & Schönig GmbH
PostgreSQL Solutions and Support
Gröhrmühlgasse 26, A-2700 Wiener Neustadt
Tel: +43/1/205 10 35 / 340
www.postgresql-support.de, www.postgresql-support.com

Attachments:

  preread-seq-tunable.diff.gz (application/x-gzip, 96.3K) download

^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: posix advises ...
  2008-05-11 09:53 posix advises ... Hans-Juergen Schoenig <postgres@cybertec.at>
@ 2008-05-14 02:04 ` Greg Smith <gsmith@gregsmith.com>
  2008-06-19 11:12   ` Re: posix advises ... Zoltan Boszormenyi <zb@cybertec.at>
  0 siblings, 1 reply; 13+ messages in thread

From: Greg Smith @ 2008-05-14 02:04 UTC (permalink / raw)
  To: Hans-Juergen Schoenig <postgres@cybertec.at>; +Cc: pgsql-patches; Zoltan Boszormenyi <zb@cybertec.at>

On Sun, 11 May 2008, Hans-Juergen Schoenig wrote:

> we also made some simple autoconf hack to check for broken posix_fadvise.

Because of how you did that, your patch is extremely difficult to even 
test.  You really should at least scan the output from diff you're about 
to send before submitting a patch to make sure it makes sense--yours is 
over 30,000 lines long just to implement a small improvement.  The reason 
for that is that you regenerated "configure" using a later version of 
Autoconf than the official distribution, and the diff for the result is 
gigantic.  You even turned off the check in configure.in that specifically 
prevents you from making that mistake so it's not like you weren't warned.

You should just show the necessary modifications to configure.in in your 
patch, certainly shouldn't submit a patch that subverts the checks there, 
and leave out the resulting configure file if you didn't use the same 
version of Autoconf.

I find the concept behind this patch very useful and I'd like to see a 
useful one re-submitted.  I'm in the middle of setting up some new 
hardware this month and was planning to test the existing fadvise patches 
Greg Stark sent out as part of that.  Having another one to test for 
accelerating multiple sequential scans would be extremely helpful to add 
to that, because then I think I can reuse some of the test cases Jeff 
Davis put together for the 8.3 improvements in that area to see how well 
it works.  It wasn't as clear to me how to test the existing bitmap scan 
patch, yours seems a more straightforward patch to use as a testing ground 
for fadvise effectiveness.

--
* Greg Smith gsmith@gregsmith.com http://www.gregsmith.com Baltimore, MD



^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: posix advises ...
  2008-05-11 09:53 posix advises ... Hans-Juergen Schoenig <postgres@cybertec.at>
  2008-05-14 02:04 ` Re: posix advises ... Greg Smith <gsmith@gregsmith.com>
@ 2008-06-19 11:12   ` Zoltan Boszormenyi <zb@cybertec.at>
  2008-06-19 23:19     ` Re: posix advises ... Greg Smith <gsmith@gregsmith.com>
  2008-07-11 10:02     ` Re: posix advises ... Abhijit Menon-Sen <ams@oryx.com>
  0 siblings, 2 replies; 13+ messages in thread

From: Zoltan Boszormenyi @ 2008-06-19 11:12 UTC (permalink / raw)
  To: Greg Smith <gsmith@gregsmith.com>; +Cc: Hans-Juergen Schoenig <postgres@cybertec.at>; pgsql-patches

Greg Smith írta:
> On Sun, 11 May 2008, Hans-Juergen Schoenig wrote:
>
>> we also made some simple autoconf hack to check for broken
>> posix_fadvise.
>
> Because of how you did that, your patch is extremely difficult to even
> test.  You really should at least scan the output from diff you're
> about to send before submitting a patch to make sure it makes
> sense--yours is over 30,000 lines long just to implement a small
> improvement.  The reason for that is that you regenerated "configure"
> using a later version of Autoconf than the official distribution, and
> the diff for the result is gigantic.  You even turned off the check in
> configure.in that specifically prevents you from making that mistake
> so it's not like you weren't warned.

Sorry, that old autoconf version was nowhere to be found for Fedora 8.
Fortunately PostgreSQL 8.4 switched already to autoconf 2.61... :-)

> You should just show the necessary modifications to configure.in in
> your patch, certainly shouldn't submit a patch that subverts the
> checks there, and leave out the resulting configure file if you didn't
> use the same version of Autoconf.
>
> I find the concept behind this patch very useful and I'd like to see a
> useful one re-submitted.  I'm in the middle of setting up some new
> hardware this month and was planning to test the existing fadvise
> patches Greg Stark sent out as part of that.

This patch (revisited and ported to current CVS HEAD) is indeed using
Greg's original patch and also added another patch written by Mark Wong
that helps evicting closed XLOGs from memory faster. Our additions are:
- advise POSIX_FADV_SEQUENTIAL for seqscans
- configure check
- small documentation for Greg's patch and its GUC
- adapt ginget.c that started using tbm_iterate() recently

The configure check implicitely assumes segfaults (which are
returned as exit code 139 here) can be handled. IIRC Tom Lane
talked about unmatched glibc and Linux kernel versions may
segfault when posix_fadvise() was called. (kernel lacked the feature,
glibc erroneously assumed it can use it)

>   Having another one to test for accelerating multiple sequential
> scans would be extremely helpful to add to that, because then I think
> I can reuse some of the test cases Jeff Davis put together for the 8.3
> improvements in that area to see how well it works.  It wasn't as
> clear to me how to test the existing bitmap scan patch, yours seems a
> more straightforward patch to use as a testing ground for fadvise
> effectiveness.
>
> -- 
> * Greg Smith gsmith@gregsmith.com http://www.gregsmith.com Baltimore, MD
>

Best regards,
Zoltán Böszörményi

-- 
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
http://www.postgresql.at/

Attachments:

  preread-seq-tunable-8.4-v4.diff.gz (application/x-tar, 9.3K) download

^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: posix advises ...
  2008-05-11 09:53 posix advises ... Hans-Juergen Schoenig <postgres@cybertec.at>
  2008-05-14 02:04 ` Re: posix advises ... Greg Smith <gsmith@gregsmith.com>
  2008-06-19 11:12   ` Re: posix advises ... Zoltan Boszormenyi <zb@cybertec.at>
@ 2008-06-19 23:19     ` Greg Smith <gsmith@gregsmith.com>
  2008-06-20 05:49       ` Re: posix advises ... Hans-Juergen Schoenig <postgres@cybertec.at>
  2008-06-20 10:24       ` Re: posix advises ... Zoltan Boszormenyi <zb@cybertec.at>
  1 sibling, 2 replies; 13+ messages in thread

From: Greg Smith @ 2008-06-19 23:19 UTC (permalink / raw)
  To: Zoltan Boszormenyi <zb@cybertec.at>; +Cc: Hans-Juergen Schoenig <postgres@cybertec.at>; pgsql-patches

On Thu, 19 Jun 2008, Zoltan Boszormenyi wrote:

> This patch (revisited and ported to current CVS HEAD) is indeed using
> Greg's original patch and also added another patch written by Mark Wong
> that helps evicting closed XLOGs from memory faster.

Great, that will save me some trouble.  I've got a stack of Linux 
performance testing queued up (got stuck behind a kernel bug impacting 
pgbench) for the next couple of weeks and I'll include this in that 
testing.  I think I've got a similar class of hardware as you tested on 
for initial evaluation--I'm getting around 200MB/s sequential I/O right 
now out of my small RAID setup,.

I added your patch to the queue for next month's CommitFest and listed 
myself as the initial reviewer, but a commit that soon is unlikely. 
Performance tests like this usually take a while to converge, and since 
this is using a less popular API I expect a round of portability concerns, 
too.

Where did Marc's patch come from?  I'd like to be able to separate out 
that change from the rest if necessary.

Also, if you have any specific test cases you ran that I could start by 
trying to replicate a speedup on, those would be handy as well.

--
* Greg Smith gsmith@gregsmith.com http://www.gregsmith.com Baltimore, MD



^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: posix advises ...
  2008-05-11 09:53 posix advises ... Hans-Juergen Schoenig <postgres@cybertec.at>
  2008-05-14 02:04 ` Re: posix advises ... Greg Smith <gsmith@gregsmith.com>
  2008-06-19 11:12   ` Re: posix advises ... Zoltan Boszormenyi <zb@cybertec.at>
  2008-06-19 23:19     ` Re: posix advises ... Greg Smith <gsmith@gregsmith.com>
@ 2008-06-20 05:49       ` Hans-Juergen Schoenig <postgres@cybertec.at>
  1 sibling, 0 replies; 13+ messages in thread

From: Hans-Juergen Schoenig @ 2008-06-20 05:49 UTC (permalink / raw)
  To: Greg Smith <gsmith@gregsmith.com>; +Cc: Zoltan Boszormenyi <zb@cybertec.at>; pgsql-patches

good morning,

this is wonderful news.
this is pretty much what we observed as well. the kernel has acted as  
showstopper for many setups recently. this patch fixed most cases  
related to kernel read ahead and so on for us.
in fact, posix_fadvise was the only way to prevent a big germany  
company from replacing postgres with oracle.
the problem was that synchronized scans led to a significant decrease  
of I/O throughput as the kernel was simply confused by processes  
concurrently reading the same file.

I hope zoltan's autoconf magic fixes the portability issues.

	hans


On Jun 20, 2008, at 1:19 AM, Greg Smith wrote:

> On Thu, 19 Jun 2008, Zoltan Boszormenyi wrote:
>
>> This patch (revisited and ported to current CVS HEAD) is indeed using
>> Greg's original patch and also added another patch written by Mark  
>> Wong
>> that helps evicting closed XLOGs from memory faster.
>
> Great, that will save me some trouble.  I've got a stack of Linux  
> performance testing queued up (got stuck behind a kernel bug  
> impacting pgbench) for the next couple of weeks and I'll include  
> this in that testing.  I think I've got a similar class of hardware  
> as you tested on for initial evaluation--I'm getting around 200MB/s  
> sequential I/O right now out of my small RAID setup,.
>
> I added your patch to the queue for next month's CommitFest and  
> listed myself as the initial reviewer, but a commit that soon is  
> unlikely. Performance tests like this usually take a while to  
> converge, and since this is using a less popular API I expect a  
> round of portability concerns, too.
>
> Where did Marc's patch come from?  I'd like to be able to separate  
> out that change from the rest if necessary.
>
> Also, if you have any specific test cases you ran that I could  
> start by trying to replicate a speedup on, those would be handy as  
> well.
>
> --
> * Greg Smith gsmith@gregsmith.com http://www.gregsmith.com  
> Baltimore, MD
>
> -- 
> Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-patches



--
Cybertec Schönig & Schönig GmbH
PostgreSQL Solutions and Support
Gröhrmühlgasse 26, 2700 Wiener Neustadt
Tel: +43/1/205 10 35 / 340
www.postgresql-support.de, www.postgresql-support.com

^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: posix advises ...
  2008-05-11 09:53 posix advises ... Hans-Juergen Schoenig <postgres@cybertec.at>
  2008-05-14 02:04 ` Re: posix advises ... Greg Smith <gsmith@gregsmith.com>
  2008-06-19 11:12   ` Re: posix advises ... Zoltan Boszormenyi <zb@cybertec.at>
  2008-06-19 23:19     ` Re: posix advises ... Greg Smith <gsmith@gregsmith.com>
@ 2008-06-20 10:24       ` Zoltan Boszormenyi <zb@cybertec.at>
  1 sibling, 0 replies; 13+ messages in thread

From: Zoltan Boszormenyi @ 2008-06-20 10:24 UTC (permalink / raw)
  To: Greg Smith <gsmith@gregsmith.com>; +Cc: Hans-Juergen Schoenig <postgres@cybertec.at>; pgsql-patches

Greg Smith írta:
> On Thu, 19 Jun 2008, Zoltan Boszormenyi wrote:
>
>> This patch (revisited and ported to current CVS HEAD) is indeed using
>> Greg's original patch and also added another patch written by Mark Wong
>> that helps evicting closed XLOGs from memory faster.
>
> Great, that will save me some trouble.  I've got a stack of Linux
> performance testing queued up (got stuck behind a kernel bug impacting
> pgbench) for the next couple of weeks and I'll include this in that
> testing.  I think I've got a similar class of hardware as you tested
> on for initial evaluation--I'm getting around 200MB/s sequential I/O
> right now out of my small RAID setup,.
>
> I added your patch to the queue for next month's CommitFest and listed
> myself as the initial reviewer, but a commit that soon is unlikely.
> Performance tests like this usually take a while to converge, and
> since this is using a less popular API I expect a round of portability
> concerns, too.
>
> Where did Marc's patch come from?  I'd like to be able to separate out
> that change from the rest if necessary.

That patch was posted here:
http://archives.postgresql.org/pgsql-patches/2008-03/msg00000.php

> Also, if you have any specific test cases you ran that I could start
> by trying to replicate a speedup on, those would be handy as well.

We experienced synchronized seqscans slowing down after some (10+) clients
which seems to be strange as it should be a strong selling point of 8.3.
With the posix_fadvise() patchs, the dropoff was pushed further.
The query involved multiple tables, it was not a trivial one table
seqscan case.
Without the patch, both a simpler SATA system (each disk at ~63MB/sec)
and a hw RAID with 400+ MB/sec showed slowdown.
The initial 60-63MB/sec with 1-3 clients on the single SATA disk system
quickly dropped to 11-17MB/sec with more clients.
With the patch, it only dropped to 40-47MB/sec.
I cannot recall the RAID figures.

> -- 
> * Greg Smith gsmith@gregsmith.com http://www.gregsmith.com Baltimore, MD
>


-- 
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
http://www.postgresql.at/




^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: posix advises ...
  2008-05-11 09:53 posix advises ... Hans-Juergen Schoenig <postgres@cybertec.at>
  2008-05-14 02:04 ` Re: posix advises ... Greg Smith <gsmith@gregsmith.com>
  2008-06-19 11:12   ` Re: posix advises ... Zoltan Boszormenyi <zb@cybertec.at>
@ 2008-07-11 10:02     ` Abhijit Menon-Sen <ams@oryx.com>
  2008-07-11 23:52       ` Re: posix advises ... Gregory Stark <stark@enterprisedb.com>
  1 sibling, 1 reply; 13+ messages in thread

From: Abhijit Menon-Sen @ 2008-07-11 10:02 UTC (permalink / raw)
  To: Zoltan Boszormenyi <zb@cybertec.at>; +Cc: pgsql-hackers@postgresql.org

Hi Zoltán.

I was reading through your posix_fadvise patch, and I wanted to ask
about this change in particular:

> --- a/src/backend/executor/nodeIndexscan.c
> +++ b/src/backend/executor/nodeIndexscan.c
> @@ -290,7 +290,7 @@ ExecIndexEvalArrayKeys(ExprContext *econtext,
>         /* We want to keep the arrays in per-tuple memory */
>         oldContext = MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory);
>  
> -       for (j = 0; j < numArrayKeys; j++)
> +       for (j = numArrayKeys-1; j >= 0; j--)
>         {
>                 ScanKey         scan_key = arrayKeys[j].scan_key;
>                 ExprState  *array_expr = arrayKeys[j].array_expr;

Why is this loop reversed? (I could have missed some discussion about
this, I just wanted to make sure it was intentional.)

I can confirm that the patch applies to HEAD, that the configure test
correctly #defines USE_POSIX_FADVISE, that it compiles cleanly, and it
looks basically sensible to me. The nodeBitmapHeapscan.c and iterator
changes need a second look by someone who understands the code better
than I do.

The documentation patch could use a little tweaking:

> +        <productname>PostgreSQL</productname> can give hints to POSIX compatible
> +        operating systems using posix_fadvise(2) to pre-read pages that will
> +        be needed in the near future. Reading the pages into the OS kernel's
> +        disk buffer will be done asynchronically while <productname>PostgreSQL</productname>
> +        works on pages which are already in memory. This may speed up bitmap scans
> +        considerably. This setting only applies to bitmap scans.
> +        Hinting for sequential scans is also used but no GUC is needed in this case.

I would suggest something like this instead:

    <productname>PostgreSQL</productname> can use posix_fadvise(2) to
    tell the operating system about pages that it knows will be needed
    in the near future. The performance of bitmap scans is considerably
    improved when the kernel pre-reads such pages into its disk buffers.
    This variable controls how many pages are marked for pre-reading at
    a time during a bitmap scan.

But I'm not convinced that this GUC is well-advised; at least, it needs
some advice about how to determine a sensible size for the parameter
(and maybe a better name). But is it really necessary at all?

Thanks.

-- ams



^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: posix advises ...
  2008-05-11 09:53 posix advises ... Hans-Juergen Schoenig <postgres@cybertec.at>
  2008-05-14 02:04 ` Re: posix advises ... Greg Smith <gsmith@gregsmith.com>
  2008-06-19 11:12   ` Re: posix advises ... Zoltan Boszormenyi <zb@cybertec.at>
  2008-07-11 10:02     ` Re: posix advises ... Abhijit Menon-Sen <ams@oryx.com>
@ 2008-07-11 23:52       ` Gregory Stark <stark@enterprisedb.com>
  2008-07-12 11:34         ` Re: posix advises ... Abhijit Menon-Sen <ams@oryx.com>
  0 siblings, 1 reply; 13+ messages in thread

From: Gregory Stark @ 2008-07-11 23:52 UTC (permalink / raw)
  To: Abhijit Menon-Sen <ams@oryx.com>; +Cc: Zoltan Boszormenyi <zb@cybertec.at>; pgsql-hackers@postgresql.org

"Abhijit Menon-Sen" <ams@oryx.com> writes:

> Hi Zoltán.
>
> I was reading through your posix_fadvise patch, 

Actually Zoltan's patch was based on an earlier patch from me. The sections
you're highlighting here are from my original patch.

> and I wanted to ask about this change in particular:
>
>> --- a/src/backend/executor/nodeIndexscan.c
>> +++ b/src/backend/executor/nodeIndexscan.c
>> @@ -290,7 +290,7 @@ ExecIndexEvalArrayKeys(ExprContext *econtext,
>>         /* We want to keep the arrays in per-tuple memory */
>>         oldContext = MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory);
>>  
>> -       for (j = 0; j < numArrayKeys; j++)
>> +       for (j = numArrayKeys-1; j >= 0; j--)

> Why is this loop reversed? (I could have missed some discussion about
> this, I just wanted to make sure it was intentional.)

There was some discussion about this change and in fact if you look at CVS
HEAD you'll find it already applied. It probably doesn't make a big difference
at least in most cases but it seems more sensible to increment the least
significant key elements first since that maximizes the chances of fetching
index keys from the same index pages or nearby pages. Incrementing the most
significant index keys would maximize the distance we're jumpin around in the
index tree.

> But I'm not convinced that this GUC is well-advised; at least, it needs
> some advice about how to determine a sensible size for the parameter
> (and maybe a better name). But is it really necessary at all?

I'm not sure which version of my patch Zoltan's was based on. The later
versions of mine had a GUC named effective_spindle_count which I think is
nicely abstracted away from the implementation details. We do need something
like that because we have no way to determine how wide a raid stripe Postgres
is sitting on and the optimal read-ahead depends on that.

The alternative would be to measure the bandwith with various amounts of
read-ahead. But I fear that would be quite hard on a production system with
the amount of noise from other i/o. It's hard enough on an idle machine with
synthetic benchmarks.

-- 
  Gregory Stark
  EnterpriseDB          http://www.enterprisedb.com
  Ask me about EnterpriseDB's Slony Replication support!



^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: posix advises ...
  2008-05-11 09:53 posix advises ... Hans-Juergen Schoenig <postgres@cybertec.at>
  2008-05-14 02:04 ` Re: posix advises ... Greg Smith <gsmith@gregsmith.com>
  2008-06-19 11:12   ` Re: posix advises ... Zoltan Boszormenyi <zb@cybertec.at>
  2008-07-11 10:02     ` Re: posix advises ... Abhijit Menon-Sen <ams@oryx.com>
  2008-07-11 23:52       ` Re: posix advises ... Gregory Stark <stark@enterprisedb.com>
@ 2008-07-12 11:34         ` Abhijit Menon-Sen <ams@oryx.com>
  2008-07-12 15:26           ` Re: posix advises ... Tom Lane <tgl@sss.pgh.pa.us>
  2008-07-15 05:07           ` Re: posix advises ... Greg Smith <gsmith@gregsmith.com>
  0 siblings, 2 replies; 13+ messages in thread

From: Abhijit Menon-Sen @ 2008-07-12 11:34 UTC (permalink / raw)
  To: Gregory Stark <stark@enterprisedb.com>; +Cc: Zoltan Boszormenyi <zb@cybertec.at>; pgsql-hackers@postgresql.org

At 2008-07-12 00:52:42 +0100, stark@enterprisedb.com wrote:
>
> There was some discussion about this change and in fact if you
> look at CVS HEAD you'll find it already applied.

Not as far as I can see.

> Incrementing the most significant index keys would maximize the
> distance we're jumpin around in the index tree.

I see. Thanks.

> The later versions of mine had a GUC named effective_spindle_count
> which I think is nicely abstracted away from the implementation
> details.

Yes, that does sound much better. (The patch I read had a
preread_pages_bitmapscan variable instead.)

-- ams



^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: posix advises ...
  2008-05-11 09:53 posix advises ... Hans-Juergen Schoenig <postgres@cybertec.at>
  2008-05-14 02:04 ` Re: posix advises ... Greg Smith <gsmith@gregsmith.com>
  2008-06-19 11:12   ` Re: posix advises ... Zoltan Boszormenyi <zb@cybertec.at>
  2008-07-11 10:02     ` Re: posix advises ... Abhijit Menon-Sen <ams@oryx.com>
  2008-07-11 23:52       ` Re: posix advises ... Gregory Stark <stark@enterprisedb.com>
  2008-07-12 11:34         ` Re: posix advises ... Abhijit Menon-Sen <ams@oryx.com>
@ 2008-07-12 15:26           ` Tom Lane <tgl@sss.pgh.pa.us>
  1 sibling, 0 replies; 13+ messages in thread

From: Tom Lane @ 2008-07-12 15:26 UTC (permalink / raw)
  To: Abhijit Menon-Sen <ams@oryx.com>; +Cc: Gregory Stark <stark@enterprisedb.com>; Zoltan Boszormenyi <zb@cybertec.at>; pgsql-hackers@postgresql.org

Abhijit Menon-Sen <ams@oryx.com> writes:
> At 2008-07-12 00:52:42 +0100, stark@enterprisedb.com wrote:
>> There was some discussion about this change and in fact if you
>> look at CVS HEAD you'll find it already applied.

> Not as far as I can see.

The place where it matters is in ExecIndexAdvanceArrayKeys(),
not initial setup.

			regards, tom lane



^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: posix advises ...
  2008-05-11 09:53 posix advises ... Hans-Juergen Schoenig <postgres@cybertec.at>
  2008-05-14 02:04 ` Re: posix advises ... Greg Smith <gsmith@gregsmith.com>
  2008-06-19 11:12   ` Re: posix advises ... Zoltan Boszormenyi <zb@cybertec.at>
  2008-07-11 10:02     ` Re: posix advises ... Abhijit Menon-Sen <ams@oryx.com>
  2008-07-11 23:52       ` Re: posix advises ... Gregory Stark <stark@enterprisedb.com>
  2008-07-12 11:34         ` Re: posix advises ... Abhijit Menon-Sen <ams@oryx.com>
@ 2008-07-15 05:07           ` Greg Smith <gsmith@gregsmith.com>
  2008-09-01 02:39             ` Re: posix advises ... Alvaro Herrera <alvherre@commandprompt.com>
  1 sibling, 1 reply; 13+ messages in thread

From: Greg Smith @ 2008-07-15 05:07 UTC (permalink / raw)
  To: Abhijit Menon-Sen <ams@oryx.com>; +Cc: Gregory Stark <stark@enterprisedb.com>; Zoltan Boszormenyi <zb@cybertec.at>; pgsql-hackers@postgresql.org

On Sat, 12 Jul 2008, Abhijit Menon-Sen wrote:

> At 2008-07-12 00:52:42 +0100, stark@enterprisedb.com wrote:
>>
>> The later versions of mine had a GUC named effective_spindle_count
>> which I think is nicely abstracted away from the implementation
>> details.
>
> Yes, that does sound much better. (The patch I read had a
> preread_pages_bitmapscan variable instead.)

This patch does need a bit of general care in a couple of areas.  The 
reviewing game plan I'm working through goes like this:

1) Update the original fadvise test program Greg Stark wrote to be a bit 
easier to use for testing general compatibility of this approach.  I want 
to collect some data from at least two Linux and Solaris systems with 
different disk setups.

2) Check out effective_spindle_count and see if it looks like a reasonable 
way to tune this feature.  If so, will probably need to merge that in to 
Zoltan's version of the patch.  May need some other cleanup in that patch 
set as well--I'm not sure that closed XLOG patch that got pushed into here 
as well is really helpful for example.

3) Generate a sequential scan test program aimed to hobble the Linux 
kernel in the way Zoltan described as motivation for his work.  I'm 
working with Jeff Davis this week to try and repurpose some of his 
syncronized scan test programs to handle this while we're both in the same 
place for a bit.

4) Generate a bitmap scan test program to check the original patch.

5) If the performance results look useful and consistant, then move toward 
cleaning up broader compatibility issues like the segfault concerns Zoltan 
mentioned.

Going to take a while to work through all that, but performance patches 
with platform-specific benefit are always painful like this.

--
* Greg Smith gsmith@gregsmith.com http://www.gregsmith.com Baltimore, MD



^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: posix advises ...
  2008-05-11 09:53 posix advises ... Hans-Juergen Schoenig <postgres@cybertec.at>
  2008-05-14 02:04 ` Re: posix advises ... Greg Smith <gsmith@gregsmith.com>
  2008-06-19 11:12   ` Re: posix advises ... Zoltan Boszormenyi <zb@cybertec.at>
  2008-07-11 10:02     ` Re: posix advises ... Abhijit Menon-Sen <ams@oryx.com>
  2008-07-11 23:52       ` Re: posix advises ... Gregory Stark <stark@enterprisedb.com>
  2008-07-12 11:34         ` Re: posix advises ... Abhijit Menon-Sen <ams@oryx.com>
  2008-07-15 05:07           ` Re: posix advises ... Greg Smith <gsmith@gregsmith.com>
@ 2008-09-01 02:39             ` Alvaro Herrera <alvherre@commandprompt.com>
  2008-09-02 12:38               ` Re: posix advises ... Greg Smith <gsmith@gregsmith.com>
  0 siblings, 1 reply; 13+ messages in thread

From: Alvaro Herrera @ 2008-09-01 02:39 UTC (permalink / raw)
  To: Greg Smith <gsmith@gregsmith.com>; +Cc: Abhijit Menon-Sen <ams@oryx.com>; Gregory Stark <stark@enterprisedb.com>; Zoltan Boszormenyi <zb@cybertec.at>; pgsql-hackers@postgresql.org

Greg Smith wrote:

> This patch does need a bit of general care in a couple of areas.  The  
> reviewing game plan I'm working through goes like this:

Did this review effort go anywhere?


-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.



^ permalink  raw  reply  [nested|flat] 13+ messages in thread

* Re: posix advises ...
  2008-05-11 09:53 posix advises ... Hans-Juergen Schoenig <postgres@cybertec.at>
  2008-05-14 02:04 ` Re: posix advises ... Greg Smith <gsmith@gregsmith.com>
  2008-06-19 11:12   ` Re: posix advises ... Zoltan Boszormenyi <zb@cybertec.at>
  2008-07-11 10:02     ` Re: posix advises ... Abhijit Menon-Sen <ams@oryx.com>
  2008-07-11 23:52       ` Re: posix advises ... Gregory Stark <stark@enterprisedb.com>
  2008-07-12 11:34         ` Re: posix advises ... Abhijit Menon-Sen <ams@oryx.com>
  2008-07-15 05:07           ` Re: posix advises ... Greg Smith <gsmith@gregsmith.com>
  2008-09-01 02:39             ` Re: posix advises ... Alvaro Herrera <alvherre@commandprompt.com>
@ 2008-09-02 12:38               ` Greg Smith <gsmith@gregsmith.com>
  0 siblings, 0 replies; 13+ messages in thread

From: Greg Smith @ 2008-09-02 12:38 UTC (permalink / raw)
  To: Alvaro Herrera <alvherre@commandprompt.com>; +Cc: Abhijit Menon-Sen <ams@oryx.com>; Gregory Stark <stark@enterprisedb.com>; Zoltan Boszormenyi <zb@cybertec.at>; pgsql-hackers@postgresql.org

On Sun, 31 Aug 2008, Alvaro Herrera wrote:

> Greg Smith wrote:
>
>> This patch does need a bit of general care in a couple of areas.  The
>> reviewing game plan I'm working through goes like this:
>
> Did this review effort go anywhere?

Haven't made much progress--all my spare time for work like this lately 
has been stuck fighting broken hardware and OS issues on my test systems. 
I have a lot more time set aside for PostgreSQL hacking this month, and 
I'll look at that and the checkpoint block sorting patch I've also been 
tinkering with soon and kick back an initial review of each.

But if anybody else is keen on working on either patch, let me know.  Be 
glad to assist instead of lead.  It takes fairly serious hardware to work 
on either of these usefully though.

--
* Greg Smith gsmith@gregsmith.com http://www.gregsmith.com Baltimore, MD



^ permalink  raw  reply  [nested|flat] 13+ messages in thread


end of thread, other threads:[~2008-09-02 12:38 UTC | newest]

Thread overview: 13+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2008-05-11 09:53 posix advises ... Hans-Juergen Schoenig <postgres@cybertec.at>
2008-05-14 02:04 ` Greg Smith <gsmith@gregsmith.com>
2008-06-19 11:12   ` Zoltan Boszormenyi <zb@cybertec.at>
2008-06-19 23:19     ` Greg Smith <gsmith@gregsmith.com>
2008-06-20 05:49       ` Hans-Juergen Schoenig <postgres@cybertec.at>
2008-06-20 10:24       ` Zoltan Boszormenyi <zb@cybertec.at>
2008-07-11 10:02     ` Abhijit Menon-Sen <ams@oryx.com>
2008-07-11 23:52       ` Gregory Stark <stark@enterprisedb.com>
2008-07-12 11:34         ` Abhijit Menon-Sen <ams@oryx.com>
2008-07-12 15:26           ` Tom Lane <tgl@sss.pgh.pa.us>
2008-07-15 05:07           ` Greg Smith <gsmith@gregsmith.com>
2008-09-01 02:39             ` Alvaro Herrera <alvherre@commandprompt.com>
2008-09-02 12:38               ` Greg Smith <gsmith@gregsmith.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