Re: Some memory not freed at the exit of RelationBuildPartitionDesc()

On Thu, Aug 8, 2019 at 1:27 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:

> Hi Amul,
>
> On Thu, Aug 8, 2019 at 4:15 PM amul sul <sulamul(at)gmail(dot)com> wrote:
> >
> > Hi,
> >
> > In RelationBuildPartitionDesc(), a memory space that use to gather
> partitioning
> > bound info wasn't free at the end. This might not a problem because this
> > allocated memory will eventually be recovered when the top-level context
> is
> > freed, but the case when a partitioned table having 1000s or more
> partitions and
> > this partitioned relation open & close, and its cached entry invalidated
> in loop
> > then we'll have too may call to RelationBuildPartitionDesc() which will
> keep
> > wasting some space with every loop.
> >
> > For a demonstration purpose, I did the following changes to
> > heap_drop_with_catalog() and tried to drop a partitioned table having
> 5000
> > partitions(attached create script) which hit OOM on a machine in no time:
> >
> > diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
> > index b7bcdd9d0f..6b7bc0d7ae 100644
> > --- a/src/backend/catalog/heap.c
> > +++ b/src/backend/catalog/heap.c
> > @@ -1842,6 +1842,8 @@ heap_drop_with_catalog(Oid relid)
> > parentOid = get_partition_parent(relid);
> > LockRelationOid(parentOid, AccessExclusiveLock);
> >
> > + rel = relation_open(parentOid, NoLock);
> > + relation_close(rel, NoLock);
> > /*
> > * If this is not the default partition, dropping it will change
> the
> > * default partition's partition constraint, so we must lock it.
> >
> >
> > I think we should do all partitioned bound information gathering and
> > calculation in temporary memory context which can be released at the end
> of
> > RelationBuildPartitionDesc(), thoughts/Comments?
> >
> > I did the same in the attached patch and the aforesaid OOM issue is
> disappeared.
>
> Thanks for the patch. This was discussed recently in the "hyrax vs.
> RelationBuildPartitionDesc()" thread [1] and I think Alvaro proposed
> an approach that's similar to yours. Not sure why it wasn't pursued
> though. Maybe the reason is buried somewhere in that discussion.
>
> Thanks,
> Amit
>
> [1]
> https://www.postgresql.org/message-id/flat/CA%2BTgmoY3bRmGB6-DUnoVy5fJoreiBJ43rwMrQRCdPXuKt4Ykaw%40mail.gmail.com

Oh, quite similar, thanks Amit for pointing that out.

Look like "hyrax vs.RelationBuildPartitionDesc()" is in discussion for the
master
branch only, not sure though, but we need the similar fix for the back
branches as well.

Regards,
Amul