On Sun, Mar 5, 2017 at 9:41 PM, Amit Kapila <amit.kapil...@gmail.com> wrote: >> RCA: >> ==== >> From "Replace min_parallel_relation_size with two new GUCs" commit >> onwards, we are not assigning parallel workers for the child rel with >> zero heap pages. This means we won't be able to create a partial >> append path as this requires all the child rels within an Append Node >> to have a partial path. > > Right, but OTOH, if we assign parallel workers by default, then it is > quite possible that it would result in much worse plans. Consider a > case where partition hierarchy has 1000 partitions and only one of > them is big enough to allow parallel workers. Now in this case, with > your proposed fix it will try to scan all the partitions in parallel > workers which I think can easily result in bad performance. I think > the right way to make such plans parallel is by using Parallel Append > node (https://commitfest.postgresql.org/13/987/). Alternatively, if > you want to force parallelism in cases like the one you have shown in > example, you can use Alter Table .. Set (parallel_workers = 1).
Well, I think this is a bug in 51ee6f3160d2e1515ed6197594bda67eb99dc2cc. The commit message doesn't say anything about changing any behavior, just about renaming GUCs, and I don't remember any discussion about changing the behavior either, and the comment still implies that we have the old behavior when we really don't, and parallel append isn't committed yet. I think the problem here is that compute_parallel_worker() thinks it can use 0 as a sentinel value that means "ignore this", but it can't really, because a heap can actually contain 0 pages. Here's a patch which does the following: - changes the sentinel value to be -1 - adjusts the test so that a value of -1 is ignored when determining whether the relation is too small for parallelism - updates a comment that is out-of-date now that this is no longer part of the seqscan logic specifically - moves some variables to narrower scopes - changes the function parameter types to be doubles, because otherwise the call in cost_index() has an overflow hazard -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
compute-parallel-worker-fix.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
