pg.ddx.io  pgsql-patches@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Gregory Stark <stark@enterprisedb.com>
To: Abhijit Menon-Sen <ams@oryx.com>
Cc: Zoltan Boszormenyi <zb@cybertec.at>
Cc: pgsql-hackers@postgresql.org
Subject: Re: posix advises ...
Date: Sat, 12 Jul 2008 00:52:42 +0100
Message-ID: <87abgo0yid.fsf@oxford.xeocode.com> (raw)
In-Reply-To: <20080711100215.GA14893@toroid.org>
References: <4826C200.8060807@cybertec.at>
	<Pine.GSO.4.64.0805132139250.20823@westnet.com>
	<485A3F20.1080907@cybertec.at>
	<20080711100215.GA14893@toroid.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!



view thread (13+ messages)  latest in thread

Message-ID: <87abgo0yid.fsf@oxford.xeocode.com>
Permalink:  ../87abgo0yid.fsf@oxford.xeocode.com/
Also on:    postgresql.org/message-id/87abgo0yid.fsf@oxford.xeocode.com

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-patches@postgresql.org
  Cc: stark@enterprisedb.com, ams@oryx.com, zb@cybertec.at, pgsql-hackers@postgresql.org
  Subject: Re: posix advises ...
  In-Reply-To: <87abgo0yid.fsf@oxford.xeocode.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

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