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 |
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 |