Re: Xact end leaves CurrentMemoryContext = TopMemoryContext

Hi,

On 2024-06-18 15:28:03 -0400, Tom Lane wrote:
> After awhile I had an epiphany: what we should do is make
> CommitTransactionCommand restore the memory context that was active
> before StartTransactionCommand.  That's what we want in every place
> that was cognizant of this issue, and it seems to be the case in every
> place that wasn't doing anything explicit about it, either.

I like it.

I wonder if there's an argument the "persistent" TopTransactionContext should
live under a different name outside of transactions, to avoid references to it
working in a context where it's not valid?   It's probably not worth it, but
not sure.


> The 0001 patch attached does that, and seems to work nicely.
> I made it implement the idea of recycling TopTransactionContext,
> too

Nice.

I think there might be some benefit to doing that for some more things,
later/separately. E.g. the allocation of TopTransactionResourceOwner shows up
in profiles for workloads with small transactions. Which got a bit worse with
17 (largely bought back in other places by the advantages of the new resowner
system).



> At this point I'd be inclined to wait for the branch to be made,
> and then apply 0001 in HEAD/v18 only and 0002 in v17 and before.
> While 0001 seems fairly straightforward, it's still a nontrivial
> change and I'm hesitant to shove it in at this late stage of the
> v17 cycle.

Seems reasonable.


Greetings,

Andres Freund