Skip site navigation (1) Skip section navigation (2)

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
Message-ID: 485A3F20.1080907@cybertec.at (view raw or flat)
Thread:
Lists: pgsql-hackerspgsql-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(at)gregsmith(dot)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/


Attachment: preread-seq-tunable-8.4-v4.diff.gz
Description: application/x-tar (9.3 KB)

In response to

Responses

pgsql-hackers by date

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

pgsql-patches by date

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

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group