Hi,
On 2017-10-05 17:43:39 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2017-10-05 11:38:40 -0400, Tom Lane wrote:
> >> I wonder why ReorderBufferCommit does this:
> >> ...
>
> > I'm not sure what the problem is that you're seeing?
>
> Nah, I take that back. The AbortCurrentTransaction call looks funny
> (and is sadly underdocumented) but it's not invalid to call it and
> then call RollbackAndReleaseCurrentSubTransaction.
It's not perfectly documented, but there's some:/* * Decoding needs access to syscaches et al., which in turn use *
heavyweightlocks and such. Thus we need to have enough state around to * keep track of those. The easiest way is to
simplyuse a transaction * internally. That also allows us to easily enforce that nothing writes * to the database by
checkingfor xid assignments. * * When we're called via the SQL SRF there's already a transaction * started, so start an
explicitsubtransaction there. */
... /* * Aborting the current (sub-)transaction as a whole has the right * semantics. We want all locks
acquiredin here to be released, not * reassigned to the parent and we do not want any database access * have
persistenteffects. */
> 2. The "MemoryContextResetAndDeleteChildren(_SPI_current->execCxt)"
> call, further up, is deleting a still-live executor state tree,
> as well as the logical-decoding context that is a child of that
> executor query context. So if you get past the Assert you'll still
> crash later on.
Right. That's why just adding found && to the entire if "works", as it
avoids that part as well.
> I'm not sure about a good way to fix #2 without re-introducing the memory
> leaks that call was added to fix (cf 7ec1c5a86). It's slightly annoying
> at this point that we got rid of the SPI_push/SPI_pop mechanism, because
> we could perhaps have used a check of the _SPI_curid value to distinguish
> a SPI context we should clean up from one we shouldn't. But that ship has
> sailed, and even if we wanted to undo that change there'd still be the
> matter of how to fix the bugs that prompted removing SPI_push/SPI_pop.
I think I don't fully understand what 7ec1c5a86 is trying to
achieve. Unfortunately reading the commit message and comment hasn't
cleared it up much so far. Why do we want to clean up memory in a
subtransaction that's above the one aborted? I can't yet meaningfully
comment on your proposals before fully understanding, sorry.
Greetings,
Andres Freund
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs