Horton

Re: Decoding speculative insert with toast leaks memory

Amit Kapila <amit.kapila16@gmail.com>

View thread View on postgr.es
From: Amit Kapila <amit.kapila16@gmail.com>
To: Amit Langote <amitlangote09@gmail.com>
Cc: Dilip Kumar <dilipbalaut@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, osumi.takamichi@fujitsu.com, Alvaro Herrera <alvherre@alvh.no-ip.org>, 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-06-17T09:25:08Z
Lists: pgsql-hackers 
On Thu, Jun 17, 2021 at 1:35 PM Amit Langote <amitlangote09@gmail.com> wrote:
>
> Hi Dilip,
>
> On Thu, Jun 17, 2021 at 4:45 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > On Thu, Jun 17, 2021 at 12:52 PM Amit Langote <amitlangote09@gmail.com> wrote:
> >
> > > Oh I missed that the problem report is for the PG13 branch.
> > >
> > > How about the attached patch then?
> > >
> > Looks good,
>
> Thanks for checking.
>
> > one minor comment, how about making the below comment,
> > same as on the head?
> >
> > - if (!found || !entry->replicate_valid)
> > + if (!found)
> > + {
> > + /*
> > + * Make the new entry valid enough for the callbacks to look at, in
> > + * case any of them get invoked during the more complicated
> > + * initialization steps below.
> > + */
> >
> > On head:
> > if (!found)
> > {
> > /* immediately make a new entry valid enough to satisfy callbacks */
>
> Agree it's better to have the same comment in both branches.
>
> Though, I think it should be "the new entry", not "a new entry".  I
> find the sentence I wrote a bit more enlightening, but I am fine with
> just fixing the aforementioned problem with the existing comment.
>
> I've updated the patch.  Also, attaching a patch for HEAD for the
> s/a/the change.  While at it, I also capitalized "immediately".
>

Your patch looks good to me as well. I would like to retain the
comment as it is from master for now. I'll do some testing and push it
tomorrow unless there are additional comments.

-- 
With Regards,
Amit Kapila.

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.