Re: Server crashed with TRAP: FailedAssertion("!(parallel_workers > 0)" when partitionwise_aggregate true.

From: Jeevan Chalke <jeevan(dot)chalke(at)enterprisedb(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Rajkumar Raghuwanshi <rajkumar(dot)raghuwanshi(at)enterprisedb(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Server crashed with TRAP: FailedAssertion("!(parallel_workers > 0)" when partitionwise_aggregate true.
Date: 2018-06-20 13:25:34
Message-ID: CAM2+6=UYAAho8JWE5h9UWtGa0ByU4RDZJkgeONiCHqFaRpunWg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jun 19, 2018 at 7:14 PM, Jeevan Chalke <
jeevan(dot)chalke(at)enterprisedb(dot)com> wrote:

>
> In the reported testcase, parallel_workers is set to 0 for all partition
>> (child) relations. Which means partial parallel paths are not possible for
>> child rels. However, the parent can have partial parallel paths. Thus, when
>> we have a full partitionwise aggregate possible (as the group by clause
>> matches with the partition key), we end-up in a situation where we do
>> create a partially_grouped_rel for the parent but there won't be any
>> partially_grouped_live_children.
>>
>> The current code was calling add_paths_to_append_rel() without making
>> sure of any live children present or not (sorry, it was my fault). This
>> causes an Assertion failure in add_paths_to_append_rel() as this function
>> assumes that it will have non-NIL live_childrels list.
>>
>> I have fixed partitionwise aggregate code which is calling
>> add_paths_to_append_rel() by checking the live children list correctly. And
>> for robustness, I have also added an Assert() in add_paths_to_append_rel().
>>
>> I have verified the callers of add_paths_to_append_rel() and except one,
>> all are calling it by making sure that they have a non-NIL live children
>> list. The one which is calling add_paths_to_append_rel() directly is
>> set_append_rel_pathlist(). And I think, at that place, we will never have
>> an empty live children list, I may be wrong though. And if that's the case
>> then newly added Assert() in add_paths_to_append_rel() will fail anyway to
>> catch any programming error in that code path.
>>
>> Attached patch fixing the crash and also added a simple test-coverage for
>> that.
>>
>> Let me know if I missed any.
>>
>
> Rajkumar offlist reported another issue related to data-loss. If few of
> the partitions has parallel_workers = 0, not all, then PWA plan ended up
> with a plan having children which has parallel_workers != 0. So the
> partitions with parallel_workers = 0; were not scanned.
>
> Fixed this in attached version of the patch.
>

There were few commits in this area due to which patch is not cleanly
applying.

Attached rebased patch.

Thanks

>
>>
>> --
Jeevan Chalke
Technical Architect, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company

Attachment Content-Type Size
0001-Make-sure-that-we-have-live-children-before-we-appen.patch text/x-patch 9.4 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Ashutosh Bapat 2018-06-20 13:41:36 Re: Server crashed with TRAP: FailedAssertion("!(parallel_workers > 0)" when partitionwise_aggregate true.
Previous Message Amit Kapila 2018-06-20 13:24:21 Re: Concurrency bug in UPDATE of partition-key