Re: Decoding speculative insert with toast leaks memory

On Tue, Jun 1, 2021 at 5:22 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
> On Tue, Jun 1, 2021 at 12:25 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)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 we must have to clean the toast before processing the next
> tuple so I think we can go with the solution I mentioned above.
>
> static void
> ReorderBufferToastReplace
> {
> ...
> toast_rel = RelationIdGetRelation(relation->rd_rel->reltoastrelid);
> if (!RelationIsValid(toast_rel))
> elog(ERROR, "could not open relation with OID %u",
> relation->rd_rel->reltoastrelid);

The attached patch fixes by queuing the spec abort change and cleaning
up the toast hash on spec abort. Currently, in this patch I am
queuing up all the spec abort changes, but as an optimization we can
avoid
queuing the spec abort for toast tables but for that we need to log
that as a flag in WAL. that this XLH_DELETE_IS_SUPER is for a toast
relation.

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