On Wednesday, December 4, 2024 8:55 AM Michael Paquier <mich...@paquier.xyz> wrote: > > On Tue, Dec 03, 2024 at 09:46:06PM -0300, Euler Taveira wrote: > > Although, Debian code search [1] says this data structure is not used > outside > > PostgreSQL, I wouldn't risk breaking third-party extensions during a minor > > upgrade (even if it is known that such data structure is from that > > particular > > output plugin -- pgoutput -- and other output plugins generally have its own > > data structure). +1 from Alvaro's proposal. > > A lookup of the public repos of github did not show fancy with the > manipulation of the structure for peoject related to Postgres, either. > > FWIW, I'm OK with the memory context reset solution as much as the > direct free calls as we are sure that they will be safe. And at the > end of the day, the problem would be solved with any of these > solutions. My votes would be +0.6 for the free and +0.5 for the mcxt > manipulation, so let's say that they are tied. > > As Alvaro and yourself are in favor of the mcxt approach, then let's > go for it.
+1 > Amit has concerns with other code paths that could be > similarly leaking. I'm not sure if this is worth waiting too long > based on how local the fix for the existing leak is with any of these > solutions. It appears there is an additional memory leak caused by allocating publication names within the CacheMemoryContext, as noted in [1]. And it can also be fixed by creating a separate memctx for publication names under the logical decoding context. I think the approach makes sense since the lifespan of publication names should ideally align with that of the logical decoding context. [1] https://www.postgresql.org/message-id/OS0PR01MB57166A4DA0ABBB94F2FBB28694362%40OS0PR01MB5716.jpnprd01.prod.outlook.com Best Regards, Hou zj
