On Fri, Dec 27, 2024 at 08:13:53AM +0000, Zhijie Hou (Fujitsu) wrote: > Thanks for updating patches ! They look good to me.
Fine by me as well. I had a bit of time today, so I've taken care of all this one down to 15 for now after checking each branch. + cachectx_mcallback = palloc0(sizeof(MemoryContextCallback)); + cachectx_mcallback->func = pgoutput_cachectx_reset_callback; + MemoryContextRegisterResetCallback(ctx->context, cachectx_mcallback); In the version of the patch for 14 and 13, why don't you just use a single reset callback to handle both of cachectx and pubctx at the same time? That would be simpler. +/* + * Private memory context for relation attribute map, created in + * PGOutputData->context when starting pgoutput, and set to NULL when its + * parent context is reset via a dedicated MemoryContextCallback. + */ +static MemoryContext cachectx = NULL; This comment block is a copy-paste of the previous one, let's just stick both declarations together. > Just to confirm, would the other stuff (streamed_txns) that allocated under > CacheMemoryContext also leaks memory ? I think it's OK to change them > separately if it does but just would like to confirm if there is a risk. Yeah, let's tackle this stuff separately and remove more the dependency to CacheMemoryContext. -- Michael
signature.asc
Description: PGP signature
