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/JMjhWEz@paquier.xyz
--
Michael