Date: Mon, 18 Mar 2019 14:57:10 -0700
From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Jeff Janes <jeff.janes@gmail.com>, Matthias Otterbach <mo@otterbach.eu>,
	pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #15700: PG 10 vs. 11: Large increase in memory usage when
 selecting BYTEA data (maybe memory leak)
Message-ID: <20190318215710.f6balpny456kink2@alap3.anarazel.de>
References: <15700-8c408321a87d56bb@postgresql.org>
 <16717.1552926811@sss.pgh.pa.us>
 <4ef5f38d5710c4d5c289537c588c7335@otterbach.eu>
 <CAMkU=1wW32K8n4ut_7zz+6HH1J068tFtpv5os=eeAgvjiQH8ZQ@mail.gmail.com>
 <CAMkU=1x_XySqXZ-1gCyoaipKLzefQSr1BuaHL12vy_9WKFnjYQ@mail.gmail.com>
 <13467.1552942521@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <13467.1552942521@sss.pgh.pa.us>
Precedence: bulk

Hi,

On 2019-03-18 16:55:21 -0400, Tom Lane wrote:
> Jeff Janes <jeff.janes@gmail.com> writes:
> > The culprit is f2dec34e19d3969ddd6.
> 
> Hmm.  I bet problem is that the buf isn't being freed in
> printtup_shutdown.

That's indeed part of it, and there's very little reason (besides there
not being a convenient function for it, but it's obviously trivial to
open code) to not free it.

This specific issue can trivially resolved by adding something like
	/* free memory allocated by initStringInfo */
	if(myState->buf.data)
		pfree(myState->buf.data);
	myState->buf.data = NULL;

to printtup_shutdown(). I'm inclined to just leave it open coded for
now.


But are we actually prepared to assume that nothing ever leaks in
receivers? That's, as far as I can tell, not a documented
assumption.

What we're getting here is a stream of:

2019-03-18 14:47:46.159 PDT [30035][3/4] LOG:  duration: 0.269 ms  parse <unnamed>: SELECT b FROM public.a
2019-03-18 14:47:46.159 PDT [30035][3/4] LOG:  duration: 0.235 ms  bind <unnamed>/C_1: SELECT b FROM public.a
2019-03-18 14:47:50.705 PDT [30035][3/4] LOG:  duration: 4545.495 ms  execute <unnamed>/C_1: SELECT b FROM public.a
2019-03-18 14:48:01.126 PDT [30035][3/4] LOG:  duration: 9613.291 ms  execute fetch from <unnamed>/C_1: SELECT b FROM public.a
2019-03-18 14:48:06.488 PDT [30035][3/4] LOG:  duration: 4564.857 ms  execute fetch from <unnamed>/C_1: SELECT b FROM public.a
2019-03-18 14:48:11.700 PDT [30035][3/4] LOG:  duration: 4449.060 ms  execute fetch from <unnamed>/C_1: SELECT b FROM public.a

(ignore the timing, I had a debugger attached)

So we're assuming that all dest receivers never leak.

We're also assuming that we don't leak into MessageContext over such
cycles, which seems wrong. At the very least things like
errdetail_params() are happy to leak into MessageContext.

Greetings,

Andres Freund