Horton

Re: Decoding speculative insert with toast leaks memory

Dilip Kumar <dilipbalaut@gmail.com>

View thread View on postgr.es
From: Dilip Kumar <dilipbalaut@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Andres Freund <andres@anarazel.de>, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>, Peter Eisentraut <peter.eisentraut@2ndquadrant.com>, Petr Jelinek <petr.jelinek@enterprisedb.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Tomas Vondra <tomas.vondra@enterprisedb.com>
Date: 2021-05-31T03:20:16Z
Lists: pgsql-hackers 
On Mon, 31 May 2021 at 8:21 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:

> On Sat, May 29, 2021 at 5:45 PM Tomas Vondra
> <tomas.vondra@enterprisedb.com> wrote:
> >
> > On 5/29/21 6:29 AM, Amit Kapila wrote:
> > > On Fri, May 28, 2021 at 5:16 PM Tomas Vondra
> > > <tomas.vondra@enterprisedb.com> wrote:
> > >>
> > >> I wonder if there's a way to free the TOASTed data earlier, instead of
> > >> waiting until the end of the transaction (as this patch does).
> > >>
> > >
> > > IIUC we are anyway freeing the toasted data at the next
> > > insert/update/delete. We can try to free at other change message types
> > > like REORDER_BUFFER_CHANGE_MESSAGE but as you said that may make the
> > > patch more complex, so it seems better to do the fix on the lines of
> > > what is proposed in the patch.
> > >
> >
> > +1
> >
> > Even if we started doing what you mention (freeing the hash for other
> > change types), we'd still need to do what the patch proposes because the
> > speculative insert may be the last change in the transaction. For the
> > other cases it works as a mitigation, so that we don't leak the memory
> > forever.
> >
>
> Right.
>
> > So let's get this committed, perhaps with a comment explaining that it
> > might be possible to reset earlier if needed.
> >
>
> Okay, I think it would be better if we can test this once for the
> streaming case as well. Dilip, would you like to do that and send the
> updated patch as per one of the comments by Tomas?


I will do that in sometime.

> --
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

Commits

  1. Fix ABI break introduced by commit 4daa140a2f.

  2. Fix valgrind issue in pgoutput.c.

  3. Fix decoding of speculative aborts.

  4. pgoutput: Fix memory leak due to RelationSyncEntry.map.

  5. Fix initialization of RelationSyncEntry for streaming transactions.

  6. Implement streaming mode in ReorderBuffer.