Horton

Re: executor relation handling

Tom Lane <tgl@sss.pgh.pa.us>

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Cc: David Rowley <david.rowley@2ndquadrant.com>, Alvaro Herrera <alvherre@2ndquadrant.com>, Jesper Pedersen <jesper.pedersen@redhat.com>, Amit Langote <amitlangote09@gmail.com>, Pg Hackers <pgsql-hackers@postgresql.org>
Date: 2018-10-07T23:18:15Z
Lists: pgsql-hackers

Attachments

I wrote:
> Still need to think a bit more about whether we want 0005 in
> anything like its current form.

So I poked at that for a bit, and soon realized that the *main* problem
there is that ExecFindRowMark() eats O(N^2) time due to repeated searches
of the es_rowMarks list.  While the patch as stated would improve that
for cases where most of the partitions can be pruned at plan time, it
does nothing much for cases where they can't.  However, it's pretty
trivial to fix that: let's just use an array not a list.  Patch 0001
attached does that.

A further improvement we could consider is to avoid opening the relcache
entries for pruned-away relations.  I could not find a way to do that
that was less invasive than removing ExecRowMark.relation and requiring
callers to call a new function to get the relation if they need it.
Patch 0002 attached is a delta on top of 0001 that does that.

Replicating your select-lt-for-share test case as best I can (you never
actually specified it carefully), I find that the TPS rate on my
workstation goes from about 250 tps with HEAD to 920 tps with patch 0001
or 1130 tps with patch 0002.  This compares to about 1600 tps for the
non-FOR-SHARE version of the query.

However, we should keep in mind that without partitioning overhead
(ie "select * from lt_999 where b = 999 for share"), the TPS rate
is over 25800 tps.  Most of the overhead in the partitioned case seems
to be from acquiring locks on rangetable entries that we won't ever
use, and none of these patch variants are touching that problem.
So ISTM that the *real* win for this scenario is going to come from
teaching the system to prune unwanted relations from the query
altogether, not just from the PlanRowMark list.

Keeping that comparison in mind, I'm inclined to think that 0001
is the best thing to do for now.  The incremental win from 0002
is not big enough to justify the API break it creates, while your
0005 is not really attacking the problem the right way.

			regards, tom lane

Commits

  1. Avoid O(N^2) cost in ExecFindRowMark().

  2. Remove some unnecessary fields from Plan trees.

  3. Restore sane locking behavior during parallel query.

  4. Remove more redundant relation locking during executor startup.

  5. In the executor, use an array of pointers to access the rangetable.

  6. Centralize executor's opening/closing of Relations for rangetable entries.

  7. Change executor to just Assert that table locks were already obtained.

  8. Change rewriter/planner/executor/plancache to depend on RTE rellockmode.

  9. Add assertions that we hold some relevant lock during relation open.

  10. Create an RTE field to record the query's lock mode for each relation.