On Mon, Jan 23, 2017 at 1:03 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> parallel_index_opt_exec_support_v6 - Removed the usage of
> GatherSupportsBackwardScan. Expanded the comments in
> ExecReScanIndexScan.
I looked through this and in general it looks reasonable to me.
However, I did notice one thing that I think is wrong. In the
parallel bitmap heap scan patch, the second argument to
compute_parallel_worker() is the number of pages that the parallel
scan is expected to fetch from the heap. In this patch, it's the
total number of pages in the index. The former seems to me to be
better, because the point of having a threshold relation size for
parallelism is that we don't want to use a lot of workers to scan a
small number of pages -- the distribution of work won't be even, and
the potential savings are limited. If we've got a big index but are
using a very selective qual to pull out only one or a small number of
rows on a single page or a small handful of pages, we shouldn't
generate a parallel path for that.
Now, against that theory, the GUC that controls the behavior of
compute_parallel_worker() is called min_parallel_relation_size, which
might make you think that the decision is supposed to be based on the
whole size of some relation. But I think that just means we need to
rename the GUC to something like min_parallel_scan_size. Possibly we
also ought consider reducing the default value somewhat, because it
seems like both sequential and index scans can benefit even when
scanning less than 8MB.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company