Re: [HACKERS] Parallel Append implementation

From: Rafia Sabih <rafia(dot)sabih(at)enterprisedb(dot)com>
To: amul sul <sulamul(at)gmail(dot)com>
Cc: Amit Khandekar <amitdkhan(dot)pg(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] Parallel Append implementation
Date: 2017-11-24 05:36:17
Message-ID: CAOGQiiM=ySANeVpqjwn2aBzzgKZXWzmTFEvMbPrEg5g4xqKG6Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Nov 21, 2017 at 5:27 PM, amul sul <sulamul(at)gmail(dot)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/

Attachment Content-Type Size
tpch20_pa_perf.zip application/zip 201.1 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Craig Ringer 2017-11-24 05:37:05 Re: Failed to delete old ReorderBuffer spilled files
Previous Message Amit Langote 2017-11-24 05:22:18 Re: [HACKERS] UPDATE of partition key