Re: Decoding speculative insert with toast leaks memory
Dilip Kumar <dilipbalaut@gmail.com>
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-06-01T14:30:42Z
Lists: pgsql-hackers
Attachments
- v2-0001-Bug-fix-for-speculative-abort.patch (text/x-patch) patch v2-0001
On Tue, Jun 1, 2021 at 5:22 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 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
Commits
-
Fix ABI break introduced by commit 4daa140a2f.
- e95f617acc48 11.13 landed
- 72b51e214a2c 12.8 landed
- 56e366f6757d 13.4 landed
-
Fix valgrind issue in pgoutput.c.
- 357cb8f07f95 13.4 landed
-
Fix decoding of speculative aborts.
- 43acadfcebec 9.6.23 landed
- 35f56c124a7a 10.18 landed
- 1f8a934e0a86 11.13 landed
- 40ad7ebff6ca 12.8 landed
- 602a32a685e4 13.4 landed
- 4daa140a2f50 14.0 landed
-
pgoutput: Fix memory leak due to RelationSyncEntry.map.
- d2505681211b 13.4 cited
-
Fix initialization of RelationSyncEntry for streaming transactions.
- 69bd60672af6 14.0 cited
-
Implement streaming mode in ReorderBuffer.
- 7259736a6e5b 14.0 cited