Horton

Re: Parallel tuplesort (for parallel B-Tree index creation)

Peter Geoghegan <pg@heroku.com>

View thread View on postgr.es
From: Peter Geoghegan <pg@heroku.com>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>, Claudio Freire <klaussfreire@gmail.com>, Robert Haas <robertmhaas@gmail.com>, Pg Hackers <pgsql-hackers@postgresql.org>, Corey Huinker <corey.huinker@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>
Date: 2016-12-04T02:37:49Z
Lists: pgsql-hackers 
On Sat, Dec 3, 2016 at 5:45 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> I don't think a patch must necessarily consider all possible uses that
> the new feature may have.  If we introduce parallel index creation,
> that's great; if pg_restore doesn't start using it right away, that's
> okay.  You, or somebody else, can still patch it later.  The patch is
> still a step forward.

While I agree, right now pg_restore will tend to use or not use
parallelism for CREATE INDEX more or less by accident, based on
whether or not pg_class.reltuples has already been set by something
else (e.g., an earlier CREATE INDEX against the same table in the
restoration). That seems unacceptable. I haven't just suppressed the
use of parallel CREATE INDEX within pg_restore because that would be
taking a position on something I have a hard time defending any
particular position on. And so, I am slightly concerned about the
entire ecosystem of tools that could implicitly use parallel CREATE
INDEX, with undesirable consequences. Especially pg_restore.

It's not so much a hard question as it is an awkward one. I want to
handle any possible objection about there being future compatibility
issues with going one way or the other ("This paints us into a corner
with..."). And, there is no existing, simple way for pg_restore and
other tools to disable the use of parallelism due to the cost model
automatically kicking in, while still allowing the proposed new index
storage parameter ("parallel_workers") to force the use of
parallelism, which seems like something that should happen. (I might
have to add a new GUC like "enable_maintenance_paralleism", since
"max_parallel_workers_maintenance = 0" disables parallelism no matter
how it might be invoked).

In general, I have a positive outlook on this patch, since it appears
to compete well with similar implementations in other systems
scalability-wise. It does what it's supposed to do.

-- 
Peter Geoghegan

Commits

  1. Support parallel btree index builds.

  2. Report an ERROR if a parallel worker fails to start properly.

  3. Transfer state pertaining to pending REINDEX operations to workers.

  4. Add a barrier primitive for synchronizing backends.

  5. Allow DML commands that create tables to use parallel query.

  6. Refactor GetOldestXmin() to use flags

  7. Fix regression in parallel planning against inheritance tables.

  8. Don't create "holes" in BufFiles, in the new logtape code.

  9. Simplify the code for logical tape read buffers.

  10. Fix excessive memory consumption in the new sort pre-reading code.

  11. Implement binary heap replace-top operation in a smarter way.

  12. Cosmetic code cleanup in commands/extension.c.

  13. Speed up planner's scanning for parallel-query hazards.

  14. Read from the same worker repeatedly until it returns no tuple.

  15. Improve tuplesort.c to support variable merge order. The original coding