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