Re: Missing pfree in logical_heap_rewrite_flush_mappings()

* Andres Freund (andres(at)2ndquadrant(dot)com) wrote:
> On 2014-03-26 12:49:41 -0400, Stephen Frost wrote:
> > * Ants Aasma (ants(at)cybertec(dot)at) wrote:
> > > It seems to me that when flushing logical mappings to disk, each
> > > mapping file leaks the buffer used to pass the mappings to XLogInsert.
> > > Also, it seems consistent to allocate that buffer in the RewriteState
> > > memory context. Patch attached.
>
> Good catch. There's actually no need for explicitly using the context,
> we're in the appropriate one. The only other MemoryContextAlloc() caller
> in there should be converted to a palloc as well.

Hrm..? I don't think that's right when it's called from
end_heap_rewrite(). Perhaps we should be switching to state->rs_cxt
while in end_heap_rewrite() also though?

> > Hmm, yeah, it does look that way. Why bother pfree'ing it here though
> > instead of letting it be cleaned up with state->rs_cxt in
> > end_heap_rewrite()?
>
> For a somewhat large relation (say a pg_attribute in a db with lots of
> tables), this can actually get to be a somewhat significant amount of
> memory. It *will* currently already get cleaned up with the context, but
> we can easily do better.

Ok, so perhaps we should go ahead and pfree() this as we go.

Thanks,

Stephen