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-12-07 12:49:15
Message-ID: CAApHDvrc4UoP=E+zzVdQRgC0gJ37+W_zw=3wjnLkXJhvToZ50A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, 1 Dec 2023 at 19:59, Ashutosh Bapat
<ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
> I am fine to work on this further if the community thinks it is
> acceptable. It seems from your point of view the worth of this work is
> as follows
> a. memory savings - not worth it
> b. getting rid of !IsA(old_path, IndexPath) - worth it
> c. problem of same path being added to multiple relations - not worth
> it since you have other solution
>
> Can't judge whether that means it's acceptable or not.

I think it's worth trying to get this working and we can run some
performance tests to see if it's cheap enough to use.

I've not had enough time to fully process your patches, but on a quick
look, I don't quite understand why link_path() does not have to
recursively increment ref_counts in each of its subpaths. It seems
you're doing the opposite in free_path(), so it seems like this would
break for cases like a SortPath on say, a LimitPath over a SeqScan.
When you call create_sort_path you'll only increment the refcount on
the LimitPath. The SeqScan path then has a refcount 1 lower than it
should. If something calls unlink_path on the SeqScan path that could
put the refcount to 0 and break the SortPath by freeing a Path
indirectly referenced by the sort.

Recursively incrementing the refcounts would slow the patch down a
bit, so we'd need to test the performance of this. I think at the
rate we create and free join paths in complex join problems we could
end up bumping refcounts quite often.

Another way to fix the pfree issues was to add infrastructure to
shallow copy paths. We could shallow copy paths whenever we borrow a
Path from another RelOptInfo and then just reset the parent to the new
RelOptInfo. That unfortunately makes your memory issue a bit worse.
We discussed this a bit in [1]. It also seems there was some
discussion about wrapping a Path up in a ProjectionPath in there. That
unfortunately also takes us in the opposite direction in terms of your
memory reduction goals.

Maybe we can try to move forward with your refcount idea and see how
the performance looks. If that's intolerable then that might help us
decide on the next best alternative solution.

David

[1] https://postgr.es/m/CAApHDvo8tiB5xbJa94f4Mo8Of2fPJ2zG+zQQQTGr-ThjSsymQQ@mail.gmail.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Zhijie Hou (Fujitsu) 2023-12-07 13:00:35 RE: Forbid the use of invalidated physical slots in streaming replication.
Previous Message Andrey M. Borodin 2023-12-07 12:40:34 Re: Transaction timeout