Re: Why our Valgrind reports suck

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