Re: surprisingly expensive join planning query

From: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: surprisingly expensive join planning query
Date: 2019-12-01 19:05:56
Message-ID: 20191201190556.3br6aszb442b7iec@development
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sun, Dec 01, 2019 at 01:27:04PM -0500, Tom Lane wrote:
>Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> writes:
>> It seems most of this comesfrom find_mergeclauses_for_outer_pathkeys()
>> which builds matched_restrictinfos and then just leaves it allocated.
>> After pfreeing this (see attached patch), the memory usage gets way down
>> and the query completes.
>
>Interesting. The memory leak was probably much less bad before
>commit 1cff1b95a, since in the old List implementation this code
>would have leaked only a list header. It makes sense to me to
>add the list_free.
>

I forgot to mention I tried on older releases, up to 9.5 (I suspected it
might be related to parallel queries), and I get OOM crashes there too.
I can't say if the memory is leaking slower/faster, though.

I tried fixing 9.5 - a simple pfree(matched_restrictinfos) triggers some
sort of list_concat error for me, seemed a bit weird TBH.

>Alternatively, it'd be possible to get rid of the separate List
>altogether, and just add the rinfo's to "mergeclauses" immediately.
>The functionality of the separate list could be replaced by a
>bool variable remembering whether we found any matches in this
>pass through the loop. I think the code would be a little less
>clear that way, but this report makes it clear that it's a
>performance bottleneck, so maybe we should just change it.
>

Yes, that might be an option. And it works even on 9.5 for me (per the
attached patch). I don't think it's much less clear compared to just
doing an explicit free at the end.

It does fix cases with up to join_collapse_limit = 10, but with 11 it
still OOM-crashes. That definitely depends on available memory, of
course.

regards

--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachment Content-Type Size
oom-fix-95.patch text/plain 1.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2019-12-01 19:08:58 Re: Using multiple extended statistics for estimates
Previous Message Tom Lane 2019-12-01 18:27:04 Re: surprisingly expensive join planning query