Date: Tue, 11 Apr 2023 19:14:24 +0200
From: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: Melanie Plageman <melanieplageman@gmail.com>,
 pgsql-hackers@lists.postgresql.org
Subject: Re: Memory leak from ExecutorState context?
Message-ID: <20230411191424.56e9acfe@karst>
In-Reply-To: <20230408020119.32a0841b@karst>
References: <3013398b-316c-638f-2a73-3783e8e2ef02@enterprisedb.com>
	<20230302001827.66e95dc3@karst>
	<41c5766d-ed71-b70c-bbbc-d3396c462d62@enterprisedb.com>
	<20230302130838.717e888d@karst>
	<77a96d42-00cb-2448-465a-aa1e92d00cac@enterprisedb.com>
	<20230302191530.781909fe@karst>
	<dbae24d7-0dda-18aa-5e08-8138ac1caef9@enterprisedb.com>
	<20230310195114.6d0c5406@karst>
	<20230317091834.22e97642@karst>
	<ae017eef-79d5-fcd6-b865-b7e55ac5290b@enterprisedb.com>
	<ZBdjJ8l3CNmBZUg0@telsasoft.com>
	<455abe0e-91b2-f428-6f4c-b95c7c8dfb52@enterprisedb.com>
	<20230320151234.38b2235e@karst>
	<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>
Organization: Dalibo
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/20230411191424.56e9acfe%40karst>
Precedence: bulk

On Sat, 8 Apr 2023 02:01:19 +0200
Jehan-Guillaume de Rorthais <jgdr@dalibo.com> wrote:

> On Fri, 31 Mar 2023 14:06:11 +0200
> Jehan-Guillaume de Rorthais <jgdr@dalibo.com> wrote:
> 
>  [...]  
> 
> After rebasing Tomas' memory balancing patch, I did some memory measures
> to answer some of my questions. Please, find in attachment the resulting
> charts "HJ-HEAD.png" and "balancing-v3.png" to compare memory consumption
> between HEAD and Tomas' patch. They shows an alternance of numbers
> before/after calling ExecHashIncreaseNumBatches (see the debug patch). I
> didn't try to find the exact last total peak of memory consumption during the
> join phase and before all the BufFiles are destroyed. So the last number
> might be underestimated.

I did some more analysis about the total memory consumption in filecxt of HEAD,
v3 and v4 patches. My previous debug numbers only prints memory metrics during
batch increments or hash table destruction. That means:

* for HEAD: we miss the batches consumed during the outer scan
* for v3: adds twice nbatch in spaceUsed, which is a rough estimation
* for v4: batches are tracked in spaceUsed, so they are reflected in spacePeak

Using a breakpoint in ExecHashJoinSaveTuple to print "filecxt->mem_allocated"
from there, here are the maximum allocated memory for bufFile context for each
branch:

        batches   max bufFiles   total  spaceAllowed rise
  HEAD    16384      199966960  ~194MB
  v3       4096       65419456   ~78MB
  v4(*3)   2048       34273280    48MB  nbatch*sizeof(PGAlignedBlock)*3
  v4(*4)   1024       17170160  60.6MB  nbatch*sizeof(PGAlignedBlock)*4
  v4(*5)   2048       34273280  42.5MB  nbatch*sizeof(PGAlignedBlock)*5

It seems account for bufFile in spaceUsed allows a better memory balancing and
management. The precise factor to rise spaceAllowed is yet to be defined. *3 or
*4 looks good, but this is based on a single artificial test case.

Also, note that HEAD is currently reporting ~4MB of memory usage. This is by
far wrong with the reality. So even if we don't commit the balancing memory
patch in v16, maybe we could account for filecxt in spaceUsed as a bugfix?

Regards,