Date: Mon, 15 May 2023 16:15:26 +0200
From: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: pgsql-hackers@lists.postgresql.org, Konstantin Knizhnik
 <knizhnik@garret.ru>, Thomas Munro <thomas.munro@gmail.com>
Subject: Re: Memory leak from ExecutorState context?
Message-ID: <20230515161526.5d05708d@karst>
In-Reply-To: <bd5d089a-c10c-8780-b0cb-33318227fe32@enterprisedb.com>
References: <20230327231323.08277083@karst>
	<f548a8d6-9ac2-46a9-1197-585ad79c1797@enterprisedb.com>
	<20230328151745.0f6061f8@karst>
	<f532d33d-e4c1-ed00-e047-94f3aafa49db@enterprisedb.com>
	<20230331140611.3695d30d@karst>
	<20230408020119.32a0841b@karst>
	<CAAKRu_ZxT9K9FuUCKV6SwZZnC9ExAiELa1oUYyiPk4eGHh12GQ@mail.gmail.com>
	<20230504193006.1b5b9622@karst>
	<20230508155648.lzddwywo6emote2b@liskov>
	<20230510142419.6185a492@karst>
	<20230512213606.isxoy2s57majc4ji@liskov>
	<bd5d089a-c10c-8780-b0cb-33318227fe32@enterprisedb.com>
Organization: Dalibo
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/20230515161526.5d05708d%40karst>
Precedence: bulk

On Sun, 14 May 2023 00:10:00 +0200
Tomas Vondra <tomas.vondra@enterprisedb.com> wrote:

> On 5/12/23 23:36, Melanie Plageman wrote:
> > Thanks for continuing to work on this.
> >=20
> > Are you planning to modify what is displayed for memory usage in
> > EXPLAIN ANALYZE?

Yes, I already start to work on this. Tracking spilling memory in
spaceUsed/spacePeak change the current behavior of the serialized HJ becaus=
e it
will increase the number of batch much faster, so this is a no go for v16.

I'll try to accumulate the total allocated (used+not used) spill context me=
mory
in instrumentation. This is gross, but it avoids to track the spilling memo=
ry
in its own structure entry.

> We could do that, but we can do that separately - it's a separate and
> independent improvement, I think.

+1

> Also, do you have a proposal how to change the explain output? In
> principle we already have the number of batches, so people can calculate
> the "peak" amount of memory (assuming they realize what it means).

We could add the batch memory consumption with the number of batches. Eg.:

=C2=A0=C2=A0Buckets: 4096 (originally 4096)=C2=A0=20
  Batches: 32768 (originally 8192) using 256MB
=C2=A0=C2=A0Memory Usage: 192kB

> I think the main problem with adding this info to EXPLAIN is that I'm
> not sure it's very useful in practice. I've only really heard about this
> memory explosion issue when the query dies with OOM or takes forever,
> but EXPLAIN ANALYZE requires the query to complete.

It could be useful to help admins tuning their queries realize that the cur=
rent
number of batches is consuming much more memory than the join itself.

This could help them fix the issue before OOM happen.

Regards,