Re: posix advises ...

From: Zoltan Boszormenyi <zb(at)cybertec(dot)at>
To: Greg Smith <gsmith(at)gregsmith(dot)com>
Cc: Hans-Juergen Schoenig <postgres(at)cybertec(dot)at>, pgsql-patches <pgsql-patches(at)postgresql(dot)org>
Subject: Re: posix advises ...
Date: 2008-06-19 11:12:32
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers 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
> 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 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(at)gregsmith(dot)com Baltimore, MD

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

Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH

Attachment Content-Type Size
preread-seq-tunable-8.4-v4.diff.gz application/x-tar 9.3 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Vanessa Gonzalez 2008-06-19 12:09:14 G8: SOS Cambio Climático
Previous Message Peter Eisentraut 2008-06-19 09:31:02 ANY/SOME/ALL with noncommutable operators

Browse pgsql-patches by date

  From Date Subject
Next Message Greg Smith 2008-06-19 23:19:46 Re: posix advises ...
Previous Message Tom Lane 2008-06-19 05:28:43 Re: Rewrite sinval messaging to reduce contention