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