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-19 08:43:31
Message-ID: CAM2+6=WirPrYX-QOHouDQfi+DzqCGSuvmaTATP2Qwcpq4UEr1A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Jun 18, 2018 at 9:27 PM, Andres Freund <andres(at)anarazel(dot)de> wrote:

> On 2018-06-18 17:10:12 +0530, Jeevan Chalke wrote:
> > On Mon, Jun 18, 2018 at 5:02 PM, Rajkumar Raghuwanshi <
> > rajkumar(dot)raghuwanshi(at)enterprisedb(dot)com> wrote:
> >
> > > Hi,
> > >
> > > Below test case crashed, when set enable_partitionwise_aggregate to
> true.
> > >
> >
> > I will have a look over this.
>

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.

Thanks

> >
> > Thanks for reporting.
>
> I've added an v11 open-items entry.
>
> - Andres
>

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Korotkov 2018-06-19 08:43:38 Re: [HACKERS] GUC for cleanup indexes threshold.
Previous Message Masahiko Sawada 2018-06-19 08:33:49 Re: [HACKERS] GUC for cleanup indexes threshold.