Horton

Thread

Commits

  1. Allow index AMs to cache data across aminsert calls within a SQL command.

  1. Caching index AM working data across aminsert calls

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-02-07T23:04:40Z

    It's always been possible for index AMs to cache data across successive
amgettuple calls within a single SQL command: the IndexScanDesc.opaque
field is meant for precisely that.  However, no comparable facility
exists for amortizing setup work across successive aminsert calls.
The attached proposed patch adds such a feature and teaches gin,
gist, and brin to use it.  (The other standard index AMs keep everything
they need in the relcache, so there's little to improve there.)

The improvement I see from this is fairly modest in a normal build.
In an example similar to the gin regression test's main insert query,

insert into gin_test_tbl select array[1, 2, g] from generate_series(1, 1000000) g;

the overall insertion speed increases perhaps 10%, which is nice but
not great.  gist and brin are less, maybe 5% or so.  However, because
most of what happens in the saved work is catalog lookups, the savings
in a CLOBBER_CACHE_ALWAYS test build are pretty substantial: the runtime
of the gin regression script, on my workstation, goes from 40 minutes
to 4 seconds.  (Yes, really.)  The gist and brin test scripts are less
insert-heavy but still lose several minutes apiece.  Since the core
regression tests are run multiple times (twice serially and once in
parallel) in the standard buildfarm cycle, I estimate that this patch
would cut over 1.5 hours from the cycle time for a CLOBBER_CACHE_ALWAYS
critter running on hardware similar to mine.  I think that alone makes it
worth doing.

The reason this has been hard up to now is that the aminsert function
is not passed any useful place to cache per-statement data.  What I
chose to do in the attached is to add suitable fields to struct IndexInfo
and pass that to aminsert.  That's not widening the index AM API very
much because IndexInfo is already within the ken of ambuild.  I figured
that this might be a particularly useful way to do it because it means
that aminsert also has access to the other data in the IndexInfo struct,
which might save it having to recompute some things.  And it makes the
DDL info available to ambuild and aminsert more similar, which seems good
on general principles.

I also looked into the idea of using the index relcache entry's
rd_amcache field for this purpose, but that fails immediately in a
CLOBBER_CACHE_ALWAYS build, because gininsert (at least, probably the
other ones too) is not robust against its GinState disappearing from under
it mid-insert.  Since rd_amcache goes away on a cache flush even if the
index is open, that doesn't work.  We could maybe fix that by introducing
some way for AMs to control the lifetime of rd_amcache, but it would
result in a substantially more complex and invasive patch than this one,
and I'm unconvinced that it'd be worth the trouble.

Thoughts, objections?

			regards, tom lane

  2. Re: Caching index AM working data across aminsert calls

    Robert Haas <robertmhaas@gmail.com> — 2017-02-07T23:57:51Z

    On Tue, Feb 7, 2017 at 6:04 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> It's always been possible for index AMs to cache data across successive
> amgettuple calls within a single SQL command: the IndexScanDesc.opaque
> field is meant for precisely that.  However, no comparable facility
> exists for amortizing setup work across successive aminsert calls.
> The attached proposed patch adds such a feature and teaches gin,
> gist, and brin to use it.  (The other standard index AMs keep everything
> they need in the relcache, so there's little to improve there.)
>
> The improvement I see from this is fairly modest in a normal build.
> In an example similar to the gin regression test's main insert query,
>
> insert into gin_test_tbl select array[1, 2, g] from generate_series(1, 1000000) g;
>
> the overall insertion speed increases perhaps 10%, which is nice but
> not great.  gist and brin are less, maybe 5% or so.

I think that's more than nice.  I think it's great.  It's not that
easy to squeeze 5-10% out of common operations.

(I have not reviewed the patch.)

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company