From: Tom Lane <tgl@sss.pgh.pa.us>
To: feichanghong <feichanghong@qq.com>
Cc: ocean_li_996 <ocean_li_996@163.com>
Cc: mohen.lhy@alibaba-inc.com <mohen.lhy@alibaba-inc.com>
Cc: pgsql-bugs@lists.postgresql.org <pgsql-bugs@lists.postgresql.org>
Cc: jdavis@postgresql.org
Subject: Re: BUG #19040: Memory leak in hashed subplan node due to missing hashtempcxt reset
Date: Wed, 03 Sep 2025 11:35:26 -0400
Message-ID: <808767.1756913726@sss.pgh.pa.us> (raw)
In-Reply-To: <tencent_8E54DE49323C3A293EE44DFE385011189509@qq.com>
References: <19040-c9b6073ef814f48c@postgresql.org>
<58062871.112.1990d3e06ea.Coremail.ocean_li_996@163.com>
<tencent_8E54DE49323C3A293EE44DFE385011189509@qq.com>
feichanghong <feichanghong@qq.com> writes:
> It seems this issue has been around for many years. I took a quick
> look at the patch for fixing it. Why don't we reset the temp context
> in the LookupTupleHashEntry, TupleHashTableHash, LookupTupleHashEntryHash,
> and FindTupleHashEntry functions? This seems more robust.
No, I disagree with that. MemoryContextReset is not free. The
existing code seems to expect that the hash tempcxt will be a
per-tuple context or similar, which will be reset once per executor
cycle by existing mechanisms. I wonder why ExecInitSubPlan is
making a separate context for this at all, rather than using some
surrounding short-lived context.
If we do keep a separate context, I agree with the idea of one
MemoryContextReset in the exit of ExecHashSubPlan, but the proposed
patch seems like a mess. I think what we ought to do is refactor
ExecHashSubPlan so that there's exactly one "ExecClearTuple(slot)"
down at the bottom, and then we can add a MemoryContextReset after it.
> Furthermore,
> the added test case doesn't seem to detect whether there's a memory leak.
Yeah, test cases for memory leaks are problematic. The only way they
can really "detect" one is if they run so long as to be pretty much
guaranteed to hit OOM, which is (a) impossible to quantify across
a range of platforms and (b) not something we'd care to commit anyway.
It's good if we have an example that one can watch to confirm
it-leaks-or-not by monitoring the process's memory consumption,
but I don't foresee committing it.
regards, tom lane
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: tgl@sss.pgh.pa.us, feichanghong@qq.com, ocean_li_996@163.com, mohen.lhy@alibaba-inc.com, pgsql-bugs@lists.postgresql.org, jdavis@postgresql.org
Subject: Re: BUG #19040: Memory leak in hashed subplan node due to missing hashtempcxt reset
In-Reply-To: <808767.1756913726@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox