Re: Memory leak in incremental sort re-scan

On 6/15/23 22:11, Tom Lane wrote:
> Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com> writes:
>> On 6/15/23 13:48, Laurenz Albe wrote:
>>> ExecIncrementalSort() calls tuplesort_begin_common(), which creates the "TupleSort main"
>>> and "TupleSort sort" memory contexts, and ExecEndIncrementalSort() calls tuplesort_end(),
>>> which destroys them.
>>> But ExecReScanIncrementalSort() only resets the memory contexts.
>
>> I think it's correct, but I need to look at the code more closely - it's
>> been a while. The code is a bit silly, as it resets the tuplesort and
>> then throws away all the pointers - so what could the _end() break?
>
> The report at [1] seems to be the same issue of ExecReScanIncrementalSort
> leaking memory.

Funny how these reports often come in pairs ...

> I applied Laurenz's fix, and that greatly reduces the
> speed of leak but doesn't remove the problem entirely. It looks like
> the remaining issue is that the data computed by preparePresortedCols() is
> recomputed each time we rescan the node. This seems entirely gratuitous,
> because there's nothing in that that could change across rescans.

Yeah, I was wondering about that too when I skimmed over that code
earlier today.

> I see zero leakage in that example after applying the attached quick
> hack. (It might be better to make the check in the caller, or to just
> move the call to ExecInitIncrementalSort.)
>

Thanks for looking. Are you planning to work on this and push the fix,
or do you want me to finish this up?

regards

--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company