Re: Memory leak in incremental sort re-scan

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>
Cc: Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Memory leak in incremental sort re-scan
Date: 2023-06-15 20:11:23
Message-ID: 2267955.1686859883@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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. 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.
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.)

regards, tom lane

[1] https://www.postgresql.org/message-id/db03c582-086d-e7cd-d4a1-3bc722f81765%40inf.ethz.ch

Attachment Content-Type Size
stop-incremental-sort-rescan-leaks.patch text/x-diff 1.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2023-06-15 20:30:58 Re: Memory leak in incremental sort re-scan
Previous Message Konstantin Knizhnik 2023-06-15 19:49:23 Re: Let's make PostgreSQL multi-threaded