From 2609e91fcf9dcf36af40cd0c5b755dccf6057df6 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Tue, 14 Mar 2017 14:33:14 -0400 Subject: [PATCH] Fix regression in parallel planning against inheritance tables. Commit 51ee6f3160d2e1515ed6197594bda67eb99dc2cc accidentally changed the behavior around inheritance hierarchies; before, we always considered parallel paths even for very small inheritance children, because otherwise an inheritance hierarchy with even one small child wouldn't be eligible for parallelism. That exception was inadverently removed; put it back. In passing, also adjust the degree-of-parallelism comptuation for index-only scans not to consider the number of heap pages fetched. Otherwise, we'll avoid parallel index-only scans on tables that are mostly all-visible, which isn't especially logical. Robert Haas and Amit Kapila, per a report from Ashutosh Sharma. Discussion: http://postgr.es/m/CAE9k0PmgSoOHRd60SHu09aRVTHRSs8s6pmyhJKWHxWw9C_x+XA@mail.gmail.com --- src/backend/optimizer/path/allpaths.c | 46 +++++++++++++++------------ src/backend/optimizer/path/costsize.c | 11 +++++-- src/include/optimizer/paths.h | 4 +-- 3 files changed, 36 insertions(+), 25 deletions(-) diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index b263359fde..4db1d16c79 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -692,7 +692,7 @@ create_plain_partial_paths(PlannerInfo *root, RelOptInfo *rel) { int parallel_workers; - parallel_workers = compute_parallel_worker(rel, rel->pages, 0); + parallel_workers = compute_parallel_worker(rel, rel->pages, -1); /* If any limit was set to zero, the user doesn't want a parallel scan. */ if (parallel_workers <= 0) @@ -2938,7 +2938,7 @@ create_partial_bitmap_paths(PlannerInfo *root, RelOptInfo *rel, pages_fetched = compute_bitmap_pages(root, rel, bitmapqual, 1.0, NULL, NULL); - parallel_workers = compute_parallel_worker(rel, pages_fetched, 0); + parallel_workers = compute_parallel_worker(rel, pages_fetched, -1); if (parallel_workers <= 0) return; @@ -2953,16 +2953,16 @@ create_partial_bitmap_paths(PlannerInfo *root, RelOptInfo *rel, * be scanned and the size of the index to be scanned, then choose a minimum * of those. * - * "heap_pages" is the number of pages from the table that we expect to scan. - * "index_pages" is the number of pages from the index that we expect to scan. + * "heap_pages" is the number of pages from the table that we expect to scan, or + * -1 if we don't expect to scan any. + * + * "index_pages" is the number of pages from the index that we expect to scan, or + * -1 if we don't expect to scan any. */ int -compute_parallel_worker(RelOptInfo *rel, BlockNumber heap_pages, - BlockNumber index_pages) +compute_parallel_worker(RelOptInfo *rel, double heap_pages, double index_pages) { int parallel_workers = 0; - int heap_parallel_workers = 1; - int index_parallel_workers = 1; /* * If the user has set the parallel_workers reloption, use that; otherwise @@ -2972,23 +2972,24 @@ compute_parallel_worker(RelOptInfo *rel, BlockNumber heap_pages, parallel_workers = rel->rel_parallel_workers; else { - int heap_parallel_threshold; - int index_parallel_threshold; - /* - * If this relation is too small to be worth a parallel scan, just - * return without doing anything ... unless it's an inheritance child. - * In that case, we want to generate a parallel path here anyway. It - * might not be worthwhile just for this relation, but when combined - * with all of its inheritance siblings it may well pay off. + * If the number of pages being scanned is insufficient to justify a + * parallel scan, just return zero ... unless it's an inheritance + * child. In that case, we want to generate a parallel path here + * anyway. It might not be worthwhile just for this relation, but + * when combined with all of its inheritance siblings it may well pay + * off. */ - if (heap_pages < (BlockNumber) min_parallel_table_scan_size && - index_pages < (BlockNumber) min_parallel_index_scan_size && - rel->reloptkind == RELOPT_BASEREL) + if (rel->reloptkind == RELOPT_BASEREL && + ((heap_pages >= 0 && heap_pages < min_parallel_table_scan_size) || + (index_pages >= 0 && index_pages < min_parallel_index_scan_size))) return 0; - if (heap_pages > 0) + if (heap_pages >= 0) { + int heap_parallel_threshold; + int heap_parallel_workers = 1; + /* * Select the number of workers based on the log of the size of * the relation. This probably needs to be a good deal more @@ -3008,8 +3009,11 @@ compute_parallel_worker(RelOptInfo *rel, BlockNumber heap_pages, parallel_workers = heap_parallel_workers; } - if (index_pages > 0) + if (index_pages >= 0) { + int index_parallel_workers = 1; + int index_parallel_threshold; + /* same calculation as for heap_pages above */ index_parallel_threshold = Max(min_parallel_index_scan_size, 1); while (index_pages >= (BlockNumber) (index_parallel_threshold * 3)) diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c index 8883586202..cd6dcaab19 100644 --- a/src/backend/optimizer/path/costsize.c +++ b/src/backend/optimizer/path/costsize.c @@ -662,6 +662,14 @@ cost_index(IndexPath *path, PlannerInfo *root, double loop_count, if (partial_path) { + /* + * For index only scans compute workers based on number of index pages + * fetched; the number of heap pages we fetch might be so small as + * to effectively rule out parallelism, which we don't want to do. + */ + if (indexonly) + rand_heap_pages = -1; + /* * Estimate the number of parallel workers required to scan index. Use * the number of heap pages computed considering heap fetches won't be @@ -669,8 +677,7 @@ cost_index(IndexPath *path, PlannerInfo *root, double loop_count, * order. */ path->path.parallel_workers = compute_parallel_worker(baserel, - (BlockNumber) rand_heap_pages, - (BlockNumber) index_pages); + rand_heap_pages, index_pages); /* * Fall out if workers can't be assigned for parallel scan, because in diff --git a/src/include/optimizer/paths.h b/src/include/optimizer/paths.h index 247fd11879..25fe78cddd 100644 --- a/src/include/optimizer/paths.h +++ b/src/include/optimizer/paths.h @@ -54,8 +54,8 @@ extern RelOptInfo *standard_join_search(PlannerInfo *root, int levels_needed, List *initial_rels); extern void generate_gather_paths(PlannerInfo *root, RelOptInfo *rel); -extern int compute_parallel_worker(RelOptInfo *rel, BlockNumber heap_pages, - BlockNumber index_pages); +extern int compute_parallel_worker(RelOptInfo *rel, double heap_pages, + double index_pages); extern void create_partial_bitmap_paths(PlannerInfo *root, RelOptInfo *rel, Path *bitmapqual);