Re: Memory consumed by paths during partitionwise join planning

From: David Rowley <dgrowleyml(at)gmail(dot)com>
To: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Memory consumed by paths during partitionwise join planning
Date: 2023-11-28 19:40:02
Message-ID: CAApHDvoDc4QwxMH4h_WRjJO4NK47L_gafv0oTpyjAkXKyXFz_A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, 28 Jul 2023 at 02:06, Ashutosh Bapat
<ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
> Table 1: Join between unpartitioned tables
> Number of tables | without patch | with patch | % reduction |
> being joined | | | |
> --------------------------------------------------------------
> 2 | 29.0 KiB | 28.9 KiB | 0.66% |
> 3 | 79.1 KiB | 76.7 KiB | 3.03% |
> 4 | 208.6 KiB | 198.2 KiB | 4.97% |
> 5 | 561.6 KiB | 526.3 KiB | 6.28% |

I have mostly just random thoughts and some questions...

Have you done any analysis of the node types that are consuming all
that memory? Are Path and subclasses of Path really that dominant in
this? The memory savings aren't that impressive and it makes me
wonder if this is worth the trouble.

What's the goal of the memory reduction work? If it's for
performance, does this increase performance? Tracking refcounts of
Paths isn't free.

Why do your refcounts start at 1? This seems weird:

+ /* Free the path if none references it. */
+ if (path->ref_count == 1)

Does ref_count really need to be an int? There's a 16-bit hole you
could use between param_info and parallel_aware. I doubt you'd need
32 bits of space for this.

I agree that it would be nice to get rid of the "if (!IsA(old_path,
IndexPath))" hack in add_path() and it seems like this idea could
allow that. However, I think if we were to have something like this
then you'd want all the logic to be neatly contained inside pathnode.c
without adding any burden in other areas to track or check Path
refcounts.

I imagine the patch would be neater if you added some kind of Path
traversal function akin to expression_tree_walker() that allows you to
visit each subpath recursively and run a callback function on each.

David

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2023-11-28 19:43:54 Re: Catalog domain not-null constraints
Previous Message Masahiko Sawada 2023-11-28 19:30:37 Re: [PoC] pg_upgrade: allow to upgrade publisher node