Re: Decoding speculative insert with toast leaks memory

On Tue, Jun 1, 2021 at 11:44 AM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
> On Tue, Jun 1, 2021 at 11:00 AM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> >
> > On Tue, Jun 1, 2021 at 10:21 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > >
> > >
> > > Right, I think you can remove the change related to stream xact and
> > > probably write some comments on why we don't need it for streamed
> > > transactions. But, now I have another question related to fixing the
> > > non-streamed case. What if after the missing spec_confirm we get the
> > > delete operation in the transaction? It seems
> > > ReorderBufferToastReplace always expects Insert/Update if we have
> > > toast hash active in the transaction.
> >
> > Yeah, that looks like a problem, I will test this case.
>
> I am able to hit that Assert after slight modification in the original
> test case, basically, I added an extra delete in the spec abort
> transaction and I got this assertion.
>

Can we try with other Insert/Update after spec abort to check if there
can be other problems due to active toast_hash?

>
> 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.

--
With Regards,
Amit Kapila.