From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Richard Guo <guofenglinux(at)gmail(dot)com> |
Cc: | Andres Freund <andres(at)anarazel(dot)de>, Álvaro Herrera <alvherre(at)kurilemu(dot)de>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: Why our Valgrind reports suck |
Date: | 2025-07-10 18:16:04 |
Message-ID: | 1609690.1752171364@sss.pgh.pa.us |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Richard Guo <guofenglinux(at)gmail(dot)com> writes:
> I'm just skimming through the changes and happened to spot two minor
> things.
> In 0008:
> if (pq_mq_handle != NULL)
> + {
> shm_mq_detach(pq_mq_handle);
> + pfree(pq_mq_handle);
> + }
> pq_mq_handle = NULL;
> Maybe we could move "pq_mq_handle = NULL;" inside the if branch?
> Though I see we're doing it in your way on master.
Yeah, we could make it be like that. I was just trying to do the
minimal change from master.
> In 0015:
> I noticed that we're freeing the list returned from
> logicalrep_workers_find(). Should we do the same for the "workers"
> list in AtEOXact_LogicalRepWorkers()?
Seems probably unnecessary. AtEOXact functions should run in the
transaction's CurTransactionContext, which will be reset or deleted
once the transaction is done. If we were talking about a large amount
of memory it might be worth reclaiming sooner, but surely we are not.
> This is very useful work; I hope we can get it in soon.
Thanks for looking at it!
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Jeff Davis | 2025-07-10 18:22:50 | Re: Remaining dependency on setlocale() |
Previous Message | David E. Wheeler | 2025-07-10 18:13:13 | Re: PATCH: jsonpath string methods: lower, upper, initcap, l/r/btrim, replace, split_part |