Asymmetry in opening and closing indices for partition routing - Mailing list pgsql-hackers

From Ashutosh Bapat
Subject Asymmetry in opening and closing indices for partition routing
Date
Msg-id CAG-ACPVwmEZTVK3e7Ho1oxpX4EtP21Bc5HOiOGMsnYC771TtLA@mail.gmail.com
Whole thread Raw
Responses Re: Asymmetry in opening and closing indices for partition routing  (Alvaro Herrera <alvherre@2ndquadrant.com>)
List pgsql-hackers
Hi All,
ExecInitPartitionInfo() has code
 536     /*
 537      * Open partition indices.  The user may have asked to check for conflicts
 538      * within this leaf partition and do "nothing" instead of throwing an
 539      * error.  Be prepared in that case by initializing the index information
 540      * needed by ExecInsert() to perform speculative insertions.
 541      */
 542     if (partrel->rd_rel->relhasindex &&
 543         leaf_part_rri->ri_IndexRelationDescs == NULL)
 544         ExecOpenIndices(leaf_part_rri,
 545                         (node != NULL &&
 546                          node->onConflictAction != ONCONFLICT_NONE));

This calls ExecOpenIndices only when ri_indexRelationDescs has not been initialized which is fine. I think this is done so that we don't open indices again and again when multiple tuples are routed to the same partition. But as part of opening indices, we also open corresponding index relations using index_open. 

The indices opened here are closed in ExecCleanupTupleRouting(), but it does this unconditionally. This means that for any reason we had called ExecOpenIndices on a partition before ExecInitPartitionInfo() following things will happen
1. ExecOpenIndices will overwrite the old arrays for index descriptors leaking memory.
2. ExecCleanupTupleRouting will close index relations that were opened second time cleaning up memory. But the relcache references corresponding to the first call to ExecOpenIndices will leak.

Similar situation can happen if ExecOpenIndices is called on a partition after it has been called in ExecInitPartitionInfo.

I couldn't find code where this can happen but I don't see any code which prevents this. This looks like a recipe for memory and reference leaks.

We could fix this by
1. Make ExecOpenIndices and ExecCloseIndices so that they can be called multiple times on the same relation similar to heap_open. The second time onwards ExecOpenIndices doesn't allocate memory and open indexes but increases a refcount. ExecCloseIndices releases memory and closes the index relations when the refcount drops to 0. Then we don't need to check leaf_part_rri->ri_IndexRelationDescs == NULL in ExecInitPartitionInfo(). 

2. Throw an error in ExecOpenIndices if all the arrays are present. We will need to check leaf_part_rri->ri_IndexRelationDescs == NULL in ExecInitPartitionInfo().

--
Best Wishes,
Ashutosh

pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: suggest to rename enable_incrementalsort
Next
From: Tomas Vondra
Date:
Subject: Re: [PATCH] Initial progress reporting for COPY command