From: | Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com> |
---|---|
To: | David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, Amit Langote <amitlangote09(at)gmail(dot)com>, Thomas Reiss <thomas(dot)reiss(at)dalibo(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Performance regression with PostgreSQL 11 and partitioning |
Date: | 2018-06-04 13:35:16 |
Message-ID: | CAFjFpRdFJdOQkNP44XZ7q8OkCt3iR=Bcr9s_O2LvewWa=CRWhQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, May 30, 2018 at 7:27 AM, David Rowley
<david(dot)rowley(at)2ndquadrant(dot)com> wrote:
>
> In the end I just made an array to store AppendRelInfo's by their
> child_relid which is created and populated during
> setup_simple_rel_arrays.
May be we want to change the name of the function or plural "arrays" conveys it?
>
> Probably the patch could go a bit further and skip array allocation
> when the append_rel_list is empty, but I've been busy working on
> another bunch of stuff to improve planning time. I'd planned to give
> this another look before submitting in the PG12 cycle, so state ==
> WIP.
I was wondering if we can get rid of append_rel_list altogether. In
your patch, you have saved AppendRelInfos by child_relid. So all the
slots indexed by parent relid are empty. We could place AppendRelInfos
by parent relid. Thus a given AppendRelInfo would be places at two
places, one at the index of child relid and second at the index
pointed by parent relid. That's fine even in case of multilevel
inheritance since an intermediate parent has two relids one as a
parent and other as a child.
One problem with that we do not know how long the array would be to
start with. But we could keep on repallocing the array to increase its
size.
It then helps us in places where we iterate over append_rel_list
looking for a parent id. It does not help in places like the loop
below in inheritance_planner().
foreach(lc, root->append_rel_list)
{
AppendRelInfo *appinfo = lfirst_node(AppendRelInfo, lc);
if (bms_is_member(appinfo->parent_relid, subqueryRTindexes) ||
bms_is_member(appinfo->child_relid, subqueryRTindexes) ||
bms_overlap(pull_varnos((Node *) appinfo->translated_vars),
subqueryRTindexes))
modifiableARIindexes = bms_add_member(modifiableARIindexes,
appinfo->child_relid);
}
>
> A quick test of the attached on Thomas' 4k part test, I get:
>
> Unpatched
> tps = 5.957508 (excluding connections establishing)
>
> Patched:
> tps = 15.368806 (excluding connections establishing)
That's really good.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
From | Date | Subject | |
---|---|---|---|
Next Message | Konstantin Knizhnik | 2018-06-04 13:47:29 | Re: [HACKERS] Moving relation extension locks out of heavyweight lock manager |
Previous Message | Benjamin Scherrey | 2018-06-04 13:33:12 | Re: Code of Conduct plan |