Re: [HACKERS] Parallel Append implementation

Rafia Sabih <rafia.sabih@enterprisedb.com>

From: Rafia Sabih <rafia.sabih@enterprisedb.com>
To: amul sul <sulamul@gmail.com>
Cc: Amit Khandekar <amitdkhan.pg@gmail.com>, Robert Haas <robertmhaas@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, Andres Freund <andres@anarazel.de>, Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2017-11-24T05:36:17Z
Lists: pgsql-hackers

Attachments

On Tue, Nov 21, 2017 at 5:27 PM, amul sul <sulamul@gmail.com> wrote:
>
> I've spent little time to debug this crash. The crash happens in
ExecAppend()
> due to subnode in node->appendplans array is referred using incorrect
> array index (out of bound value) in the following code:
>
>         /*
>          * figure out which subplan we are currently processing
>          */
>         subnode = node->appendplans[node->as_whichplan];
>
> This incorrect value to node->as_whichplan is get assigned in the
> choose_next_subplan_for_worker().
>
> By doing following change on the v19 patch does the fix for me:
>
> --- a/src/backend/executor/nodeAppend.c
> +++ b/src/backend/executor/nodeAppend.c
> @@ -489,11 +489,9 @@ choose_next_subplan_for_worker(AppendState *node)
>     }
>
>     /* Pick the plan we found, and advance pa_next_plan one more time. */
> -   node->as_whichplan = pstate->pa_next_plan;
> +   node->as_whichplan = pstate->pa_next_plan++;
>     if (pstate->pa_next_plan == node->as_nplans)
>         pstate->pa_next_plan = append->first_partial_plan;
> -   else
> -       pstate->pa_next_plan++;
>
>     /* If non-partial, immediately mark as finished. */
>     if (node->as_whichplan < append->first_partial_plan)
>
> Attaching patch does same changes to Amit's
ParallelAppend_v19_rebased.patch.
>
Thanks for the patch, I tried it and worked fine for me. The performance
numbers for this patch are as follows,

Query | head | Patch |
1 |241633.69  | 243916.798
3 |74000.394 | 75966.013
4 |12241.87 | 12310.405
5 |65190.68 | 64968.069
6 |8718.477 | 7150.98
7 |69920.367 | 68504.058
8 |21722.406 | 21488.255
10 |37807.3 | 36308.253
12 |40654.877 | 36532.134
14 |10910.043 | 9982.559
15 |57074.768 | 51328.908
18 |293655.538 | 282611.02
21 |1905000.232 | 1803922.924

All the values of execution time are in ms. The setup used for the
experiment is same as mentioned upthread,
I was trying to get the performance of this patch at commit id -
11e264517dff7a911d9e6494de86049cab42cde3 and TPC-H scale factor 20
with the following parameter settings,
work_mem = 1 GB
shared_buffers = 10GB
effective_cache_size = 10GB
max_parallel_workers_per_gather = 4
enable_partitionwise_join = on

and the details of the partitioning scheme is as follows,
tables partitioned = lineitem on l_orderkey and orders on o_orderkey
number of partitions in each table = 10

Please find the attached zip for the explain analyse outputs for head and
patch for the above mentioned queries.

Overall, performance wise the presence of patch doesn't adds much, may be
because of scale factor, I don't know. If anybody has better ideas
regarding setup please enlighten me. Otherwise we may investigate further
the performance for this patch, by spending some time looking into the
plans and check for what queries append was the bottleneck, or with
parallel-append in picture which nodes get faster.

-- 
Regards,
Rafia Sabih
EnterpriseDB: http://www.enterprisedb.com/

Commits

  1. Update parallel.sgml for Parallel Append

  2. Support Parallel Append plan nodes.

  3. Remove BufFile's isTemp flag.

  4. Improve comments for parallel executor estimation functions.

  5. Separate reinitialization of shared parallel-scan state from ExecReScan.

  6. Eat XIDs more efficiently in recovery TAP test.

  7. Avoid syntax error on platforms that have neither LOCALE_T nor ICU.

  8. Preparatory refactoring for parallel merge join support.