Re: Parallel Index Scans
Rahila Syed <rahilasyed90@gmail.com>
From: Rahila Syed <rahilasyed90@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Anastasia Lubennikova <a.lubennikova@postgrespro.ru>, Anastasia Lubennikova <lubennikovaav@gmail.com>,
pgsql-hackers <pgsql-hackers@postgresql.org>, Rahila Syed <rahilasyed.90@gmail.com>
Date: 2017-01-31T12:23:50Z
Lists: pgsql-hackers
Hello, >Agreed, that it makes sense to consider only the number of pages to >scan for computation of parallel workers. I think for index scan we >should consider both index and heap pages that need to be scanned >(costing of index scan consider both index and heap pages). I thin >where considering heap pages matter more is when the finally selected >rows are scattered across heap pages or we need to apply a filter on >rows after fetching from the heap. OTOH, we can consider just pages >in the index as that is where mainly the parallelism works IMO, considering just index pages will give a better estimate of work to be done in parallel. As the amount of work/number of pages divided amongst workers is irrespective of the number of heap pages scanned. Thank you, Rahila Syed On Mon, Jan 30, 2017 at 2:52 PM, Amit Kapila <amit.kapila16@gmail.com> wrote: > On Sat, Jan 28, 2017 at 1:34 AM, Robert Haas <robertmhaas@gmail.com> > wrote: > > 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. > > > > Agreed, that it makes sense to consider only the number of pages to > scan for computation of parallel workers. I think for index scan we > should consider both index and heap pages that need to be scanned > (costing of index scan consider both index and heap pages). I thin > where considering heap pages matter more is when the finally selected > rows are scattered across heap pages or we need to apply a filter on > rows after fetching from the heap. OTOH, we can consider just pages > in the index as that is where mainly the parallelism works. In the > attached patch (parallel_index_opt_exec_support_v7.patch), I have > considered both index and heap pages, let me know if you think some > other way is better. I have also prepared a separate independent > patch (compute_index_pages_v1) on HEAD to compute index pages which > can be used by parallel index scan. There is no change in > parallel_index_scan (parallel btree scan) patch, so I am not attaching > its new version. > > > 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. > > > > Agreed, but let's consider it separately. > > > The order in which patches needs to be applied: > compute_index_pages_v1.patch, parallel_index_scan_v6.patch[1], > parallel_index_opt_exec_support_v7.patch > > > [1] - https://www.postgresql.org/message-id/CAA4eK1J% > 3DLSBpDx7i_izGJxGVUryqPe-2SKT02De-PrQvywiMxw%40mail.gmail.com > > -- > With Regards, > Amit Kapila. > EnterpriseDB: http://www.enterprisedb.com >
Commits
-
Replace min_parallel_relation_size with two new GUCs.
- 51ee6f3160d2 10.0 landed
-
Factor out logic for computing number of parallel workers.
- 716c7d4b242f 10.0 cited
-
Support condition variables.
- e8ac886c2477 10.0 cited
-
Allow parallel custom and foreign scans.
- 69d34408e5e7 9.6.0 cited
-
Add a C API for parallel heap scans.
- ee7ca559fcf4 9.6.0 cited