On Wed, Sep 28, 2022 at 5:30 AM Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.com> wrote: > I'm attaching the v2 patch designed as described above. Please review it. > > I've added an entry in CF - https://commitfest.postgresql.org/40/3915/
This looks odd to me. In the case of a regular backend, the sigsetjmp() handler in src/backend/tcop/postgres.c is responsible for cleaning up memory. It calls AbortCurrentTransaction() which will call CleanupTransaction() which will call AtCleanup_Memory() which will block away TopTransactionContext. I think we ought to do something analogous here, and we almost do already. Some walsender commands are going to be SQL commands and some aren't. For those that aren't, the same block calls WalSndErrorCleanup() which does similar kinds of cleanup, including in some situations calling WalSndResourceCleanup() which cleans up the resource owner in cases where we have a resource owner without a transaction. I feel like we ought to be trying to tie the cleanup into WalSndErrorCleanup() or WalSndResourceCleanup() based on having the memory context that we ought to be blowing away stored in a global variable, rather than using a try/catch block. Like, maybe there's a function EstablishWalSenderMemoryContext() that commands can call before allocating memory that shouldn't survive an error. And it's deleted after each command if it exists, or if an error occurs then WalSndErrorCleanup() deletes it. -- Robert Haas EDB: http://www.enterprisedb.com
