Re: Decoding speculative insert with toast leaks memory

On Tue, Jun 1, 2021 at 5:23 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Tue, Jun 1, 2021 at 12:25 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> > >
> > > IMHO, as I stated earlier one way to fix this problem is that we add
> > > the spec abort operation (DELETE + XLH_DELETE_IS_SUPER flag) to the
> > > queue, maybe with action name
> > > "REORDER_BUFFER_CHANGE_INTERNAL_SPEC_ABORT" and as part of processing
> > > that just cleans up the toast and specinsert tuple and nothing else.
> > > If we think that makes sense then I can work on that patch?
> > >
> >
> > I think this should solve the problem but let's first try to see if we
> > have any other problems. Because, say, if we don't have any other
> > problem, then maybe removing Assert might work but I guess we still
> > need to process the tuple to find that we don't need to assemble toast
> > for it which again seems like overkill.
>
> Yeah, other operation will also fail, basically, if txn->toast_hash is
> not NULL then we assume that we need to assemble the tuple using
> toast, but if there is next insert in another relation and if that
> relation doesn't have a toast table then it will fail with below
> error.  And otherwise also, if it is the same relation, then the toast
> chunks of previous tuple will be used for constructing this new tuple.
>

I think the same relation case might not create a problem because it
won't find the entry for it in the toast_hash, so it will return from
there but the other two problems will be there. So, one idea could be
to just avoid those two cases (by simply adding return for those
cases) and still we can rely on toast clean up on the next
insert/update/delete. However, your approach looks more natural to me
as that will allow us to clean up memory in all cases instead of
waiting till the transaction end. So, I still vote for going with your
patch's idea of cleaning at spec_abort but I am fine if you and others
decide not to process spec_abort message. What do you think? Tomas, do
you have any opinion on this matter?

-- 
With Regards,
Amit Kapila.