Message-ID: <bd5d089a-c10c-8780-b0cb-33318227fe32@enterprisedb.com>
Date: Sun, 14 May 2023 00:10:00 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: Memory leak from ExecutorState context?
Content-Language: en-US
To: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>,
 pgsql-hackers@lists.postgresql.org, Konstantin Knizhnik
 <knizhnik@garret.ru>, Thomas Munro <thomas.munro@gmail.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>
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
In-Reply-To: <20230512213606.isxoy2s57majc4ji@liskov>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/bd5d089a-c10c-8780-b0cb-33318227fe32%40enterprisedb.com>
Precedence: bulk

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

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

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

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.

A separate memory context (which  pg_log_backend_memory_contexts can
dump to server log) is more valuable, I think.

> Also, since that won't help a user who OOMs, I wondered if the spillCxt
> is helpful on its own or if we need some kind of logging message for
> users to discover that this is what led them to running out of memory.
> 

I think the separate memory context is definitely an improvement,
helpful on it's own it makes it clear *what* allocated the memory. It
requires having the memory context stats, but we may already dump them
into the server log if malloc returns NULL. Granted, it depends on how
the system is configured and it won't help when OOM killer hits :-(

I wouldn't object to having some sort of log message, but when exactly
would we emit it? Presumably after exceeding some amount of memory, but
what would it be? It can't be too low (not to trigger it too often) or
too high (failing to report the issue). Also, do you think it should go
to the user or just to the server log?


regards

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