On Fri, Nov 29, 2024 at 11:05:51AM +0900, Michael Paquier wrote: > This is a follow-up of ea792bfd93ab and this thread where I've noticed > that some memory was still leaking when running sysbench with a > logical replication setup: > https://www.postgresql.org/message-id/zz7srcbxxhoyn...@paquier.xyz
Adding Amit in CC in case, as we've talked about this topic offlist. > This problem exists since the introduction of logical replication, > down to v10. It would be sad to have an extra loop on publications to > free explicitely the publication names, and the issue would still be > hidden to the existing callers of GetPublication(), so I'd suggest to > change the Publication structure to use a NAMEDATALEN string to force > the free to happen, as in the attached. > > That means an ABI break. I've looked around and did not notice any > out-of-core code relying on sizeof(Publication). That does not mean > that nothing would break, of course, but the odds should be pretty > good as this is a rather internal structure. One way would be to just > fix that on HEAD and call it a day, but I'm aware of heavy logirep > users whose workloads would be able to see memory bloating because > they maintain WAL senders around. After sleeping on that, and because the leak is minimal, I'm thinking about just applying the fix only on HEAD and call it a day. This changes the structure of Publication so as we use a char[NAMEDATALEN] rather than a char*, avoiding the pstrdup(), for the publication name and free it in the list_free_deep() when reloading the list of publications. If there are any objections or comments, feel free. I'll revisit that again around the middle of next week. -- Michael
signature.asc
Description: PGP signature
