On Fri, Oct 14, 2022 at 09:56:31PM +0000, Cary Huang wrote: > I applied your v5 patch on the current master and run valgrind on it > while doing a basebackup with simulated error. No memory leak > related to backup is observed. Regression is also passing.
Echoing with what I mentioned upthread in [1], I don't quite understand why this patch needs to touch basebackup.c, walsender.c and postgres.c. In the case of a replication command processed by a WAL sender, memory allocations happen in the memory context created for replication commands, which is itself, as far as I understand, the message memory context when we get a 'Q' message for a simple query. Why do we need more code for a cleanup that should be already happening? Am I missing something obvious? xlogfuncs.c, by storing stuff in the TopMemoryContext of the process running the SQL commands pg_backup_start/stop() is different, of course. Perhaps the point of centralizing the base backup context in xlogbackup.c makes sense, but my guess that it makes more sense to keep that with the SQL functions as these are the only ones in need of a cleanup, coming down to the fact that the start and stop functions happen in different queries, aka these are not bind to a message context. [1]: https://www.postgresql.org/message-id/YzPKpKEk/jmjh...@paquier.xyz -- Michael
signature.asc
Description: PGP signature
