memory leak when serializing TRUNCATE in reorderbuffer

Hi,

While rebasing the logical replication patches on top of PG11, I've 
noticed that ReorderBufferSerializeChange claims this:

     case REORDER_BUFFER_CHANGE_TRUNCATE:
         ...
         /* ReorderBufferChange contains everything important */

That is not quite correct, though - the OIDs of truncated relations is 
stored in a separately palloc-ed array. So we only serialize the pointer 
to that array (which is part of ReorderBufferChange) and then read it 
back when restoring the change from disk.

Now, this can't cause crashes, because the 'relids' array won't go away 
(more on this later), and so the pointer remains valid. But it's a 
memory leak - a quite small and not very common one, because people 
don't do TRUNCATE very often, particularly not with many tables.

So I think we should fix and serialize/restore the OID array, just like 
we do for tuples, snapshots etc. See the attached fix.

Another thing we should probably reconsider is where the relids is 
allocated - the pointer remains valid because we happen to allocate it 
in TopMemoryContext. It's not that bad because we don't free the other 
reorderbuffer contexts until the walsender exits anyway, but still.

So I propose to allocate it in rb->context just like the other bits of 
data (snapshots, ...). Replacing the palloc() in DecodeTruncate() with 
something like:

    MemoryContextAlloc(ctx->reorder->context,
                       xlrec->nrelids * sizeof(Oid));

should do the trick. The other places in decode.c don't allocate memory 
directly but call ReorderBufferGetTupleBuf() instead - perhaps we should 
introduce a similar wrapper here too.


regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services