Re: Memory leak in WAL sender with pgoutput (v10~)

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/Zz7SRcBXxhOYngOr@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