Reuse BrinDesc and BrinRevmap in brininsert

The brininsert code used to initialize (and destroy) BrinDesc and
BrinRevmap for each tuple, which is not free. This patch initializes
these structures only once, and reuses them for all inserts in the same
command. The data is passed through indexInfo->ii_AmCache.

This also introduces an optional AM callback "aminsertcleanup" that
allows performing custom cleanup in case simply pfree-ing ii_AmCache is
not sufficient (which is the case when the cache contains TupleDesc,
Buffers, and so on).

Author: Soumyadeep Chakraborty
Reviewed-by: Alvaro Herrera, Matthias van de Meent, Tomas Vondra
Discussion: https://postgr.es/m/CAE-ML%2B9r2%3DaO1wwji1sBN9gvPz2xRAtFUGfnffpd0ZqyuzjamA%40mail.gmail.com

diff --git a/contrib/bloom/blutils.c b/contrib/bloom/blutils.c
index f23fbb1d9e0a6ef4c5a1705326079f87bdc90bc8..4830cb3fee640597865d4eace75f3189669d59b4 100644 (file)
--- a/contrib/bloom/blutils.c
+++ b/contrib/bloom/blutils.c
@@ -131,6 +131,7 @@ blhandler(PG_FUNCTION_ARGS)
        amroutine->ambuild = blbuild;
        amroutine->ambuildempty = blbuildempty;
        amroutine->aminsert = blinsert;
+       amroutine->aminsertcleanup = NULL;
        amroutine->ambulkdelete = blbulkdelete;
        amroutine->amvacuumcleanup = blvacuumcleanup;
        amroutine->amcanreturn = NULL;

diff --git a/doc/src/sgml/indexam.sgml b/doc/src/sgml/indexam.sgml
index 30eda37afa8e895ce00a2a41d147fd404efa7933..f107c43d6a69c0fda64790272e4182d2f168207a 100644 (file)
--- a/doc/src/sgml/indexam.sgml
+++ b/doc/src/sgml/indexam.sgml
@@ -139,6 +139,7 @@ typedef struct IndexAmRoutine
     ambuild_function ambuild;
     ambuildempty_function ambuildempty;
     aminsert_function aminsert;
+    aminsertcleanup_function aminsertcleanup;
     ambulkdelete_function ambulkdelete;
     amvacuumcleanup_function amvacuumcleanup;
     amcanreturn_function amcanreturn;   /* can be NULL */
@@ -359,7 +360,21 @@ aminsert (Relation indexRelation,
    within an SQL statement, it can allocate space
    in <literal>indexInfo-&gt;ii_Context</literal> and store a pointer to the
    data in <literal>indexInfo-&gt;ii_AmCache</literal> (which will be NULL
-   initially).
+   initially). After the index insertions complete, the memory will be freed
+   automatically. If additional cleanup is required (e.g. if the cache contains
+   buffers and tuple descriptors), the AM may define an optional function
+   <literal>indexinsertcleanup</literal>, called before the memory is released.
+  </para>
+
+  <para>
+<programlisting>
+void
+aminsertcleanup (IndexInfo *indexInfo);
+</programlisting>
+   Clean up state that was maintained across successive inserts in
+   <literal>indexInfo-&gt;ii_AmCache</literal>. This is useful if the data
+   requires additional cleanup steps, and simply releasing the memory is not
+   sufficient.
   </para>
 
   <para>

diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index f0eac078e0b2effae7441494b30e9f0c5d74d884..a5f68f67b74fa72c9308af5fe4c43ac3888c0eab 100644 (file)
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -58,6 +58,17 @@ typedef struct BrinBuildState
        BrinMemTuple *bs_dtuple;
 } BrinBuildState;
 
+/*
+ * We use a BrinInsertState to capture running state spanning multiple
+ * brininsert invocations, within the same command.
+ */
+typedef struct BrinInsertState
+{
+       BrinRevmap *bis_rmAccess;
+       BrinDesc   *bis_desc;
+       BlockNumber     bis_pages_per_range;
+} BrinInsertState;
+
 /*
  * Struct used as "opaque" during index scans
  */
@@ -72,6 +83,7 @@ typedef struct BrinOpaque
 
 static BrinBuildState *initialize_brin_buildstate(Relation idxRel,
                                                                                                  BrinRevmap *revmap, BlockNumber pagesPerRange);
+static BrinInsertState *initialize_brin_insertstate(Relation idxRel, IndexInfo *indexInfo);
 static void terminate_brin_buildstate(BrinBuildState *state);
 static void brinsummarize(Relation index, Relation heapRel, BlockNumber pageRange,
                                                  bool include_partial, double *numSummarized, double *numExisting);
@@ -117,6 +129,7 @@ brinhandler(PG_FUNCTION_ARGS)
        amroutine->ambuild = brinbuild;
        amroutine->ambuildempty = brinbuildempty;
        amroutine->aminsert = brininsert;
+       amroutine->aminsertcleanup = brininsertcleanup;
        amroutine->ambulkdelete = brinbulkdelete;
        amroutine->amvacuumcleanup = brinvacuumcleanup;
        amroutine->amcanreturn = NULL;
@@ -140,6 +153,27 @@ brinhandler(PG_FUNCTION_ARGS)
        PG_RETURN_POINTER(amroutine);
 }
 
+/*
+ * Initialize a BrinInsertState to maintain state to be used across multiple
+ * tuple inserts, within the same command.
+ */
+static BrinInsertState *
+initialize_brin_insertstate(Relation idxRel, IndexInfo *indexInfo)
+{
+       BrinInsertState *bistate;
+       MemoryContext oldcxt;
+
+       oldcxt = MemoryContextSwitchTo(indexInfo->ii_Context);
+       bistate = palloc0(sizeof(BrinInsertState));
+       bistate->bis_desc = brin_build_desc(idxRel);
+       bistate->bis_rmAccess = brinRevmapInitialize(idxRel,
+                                                                                                &bistate->bis_pages_per_range);
+       indexInfo->ii_AmCache = bistate;
+       MemoryContextSwitchTo(oldcxt);
+
+       return bistate;
+}
+
 /*
  * A tuple in the heap is being inserted.  To keep a brin index up to date,
  * we need to obtain the relevant index tuple and compare its stored values
@@ -162,14 +196,24 @@ brininsert(Relation idxRel, Datum *values, bool *nulls,
        BlockNumber pagesPerRange;
        BlockNumber origHeapBlk;
        BlockNumber heapBlk;
-       BrinDesc   *bdesc = (BrinDesc *) indexInfo->ii_AmCache;
+       BrinInsertState *bistate = (BrinInsertState *) indexInfo->ii_AmCache;
        BrinRevmap *revmap;
+       BrinDesc   *bdesc;
        Buffer          buf = InvalidBuffer;
        MemoryContext tupcxt = NULL;
        MemoryContext oldcxt = CurrentMemoryContext;
        bool            autosummarize = BrinGetAutoSummarize(idxRel);
 
-       revmap = brinRevmapInitialize(idxRel, &pagesPerRange);
+       /*
+        * If firt time through in this statement, initialize the insert state
+        * that we keep for all the inserts in the command.
+        */
+       if (!bistate)
+               bistate = initialize_brin_insertstate(idxRel, indexInfo);
+
+       revmap = bistate->bis_rmAccess;
+       bdesc = bistate->bis_desc;
+       pagesPerRange = bistate->bis_pages_per_range;
 
        /*
         * origHeapBlk is the block number where the insertion occurred.  heapBlk
@@ -228,14 +272,6 @@ brininsert(Relation idxRel, Datum *values, bool *nulls,
                if (!brtup)
                        break;
 
-               /* First time through in this statement? */
-               if (bdesc == NULL)
-               {
-                       MemoryContextSwitchTo(indexInfo->ii_Context);
-                       bdesc = brin_build_desc(idxRel);
-                       indexInfo->ii_AmCache = (void *) bdesc;
-                       MemoryContextSwitchTo(oldcxt);
-               }
                /* First time through in this brininsert call? */
                if (tupcxt == NULL)
                {
@@ -306,7 +342,6 @@ brininsert(Relation idxRel, Datum *values, bool *nulls,
                break;
        }
 
-       brinRevmapTerminate(revmap);
        if (BufferIsValid(buf))
                ReleaseBuffer(buf);
        MemoryContextSwitchTo(oldcxt);
@@ -316,6 +351,24 @@ brininsert(Relation idxRel, Datum *values, bool *nulls,
        return false;
 }
 
+/*
+ * Callback to clean up the BrinInsertState once all tuple inserts are done.
+ */
+void
+brininsertcleanup(IndexInfo *indexInfo)
+{
+       BrinInsertState *bistate = (BrinInsertState *) indexInfo->ii_AmCache;
+
+       Assert(bistate);
+       /*
+        * Clean up the revmap. Note that the brinDesc has already been cleaned up
+        * as part of its own memory context.
+        */
+       brinRevmapTerminate(bistate->bis_rmAccess);
+       bistate->bis_rmAccess = NULL;
+       bistate->bis_desc = NULL;
+}
+
 /*
  * Initialize state for a BRIN index scan.
  *

diff --git a/src/backend/access/gin/ginutil.c b/src/backend/access/gin/ginutil.c
index 7a4cd93f30185cd1b607bfc33f7fdf52559c89ed..a875c5d3d7a8b510bc83c9e6231b0e701cb42b7c 100644 (file)
--- a/src/backend/access/gin/ginutil.c
+++ b/src/backend/access/gin/ginutil.c
@@ -64,6 +64,7 @@ ginhandler(PG_FUNCTION_ARGS)
        amroutine->ambuild = ginbuild;
        amroutine->ambuildempty = ginbuildempty;
        amroutine->aminsert = gininsert;
+       amroutine->aminsertcleanup = NULL;
        amroutine->ambulkdelete = ginbulkdelete;
        amroutine->amvacuumcleanup = ginvacuumcleanup;
        amroutine->amcanreturn = NULL;

diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c
index 8ef5fa032904dc2fb9f824ff9ce571da40ce5e23..9a1bf8f66cbfc97a7966c3f0bcc21854aebe944e 100644 (file)
--- a/src/backend/access/gist/gist.c
+++ b/src/backend/access/gist/gist.c
@@ -86,6 +86,7 @@ gisthandler(PG_FUNCTION_ARGS)
        amroutine->ambuild = gistbuild;
        amroutine->ambuildempty = gistbuildempty;
        amroutine->aminsert = gistinsert;
+       amroutine->aminsertcleanup = NULL;
        amroutine->ambulkdelete = gistbulkdelete;
        amroutine->amvacuumcleanup = gistvacuumcleanup;
        amroutine->amcanreturn = gistcanreturn;

diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c
index 7a025f33cfeab51d7971af7b6a4cd05d879dda7d..6443ff21bda3629bfbacfccf142bb9cdc1fa6354 100644 (file)
--- a/src/backend/access/hash/hash.c
+++ b/src/backend/access/hash/hash.c
@@ -83,6 +83,7 @@ hashhandler(PG_FUNCTION_ARGS)
        amroutine->ambuild = hashbuild;
        amroutine->ambuildempty = hashbuildempty;
        amroutine->aminsert = hashinsert;
+       amroutine->aminsertcleanup = NULL;
        amroutine->ambulkdelete = hashbulkdelete;
        amroutine->amvacuumcleanup = hashvacuumcleanup;
        amroutine->amcanreturn = NULL;

diff --git a/src/backend/access/index/indexam.c b/src/backend/access/index/indexam.c
index b25b03f7abc3299f511a839c847083ebf349eb21..597b763d1f68ed1926bcd27921244b9053b00997 100644 (file)
--- a/src/backend/access/index/indexam.c
+++ b/src/backend/access/index/indexam.c
@@ -196,6 +196,21 @@ index_insert(Relation indexRelation,
                                                                                         indexInfo);
 }
 
+/* -------------------------
+ *             index_insert_cleanup - clean up after all index inserts are done
+ * -------------------------
+ */
+void
+index_insert_cleanup(Relation indexRelation,
+                                        IndexInfo *indexInfo)
+{
+       RELATION_CHECKS;
+       Assert(indexInfo);
+
+       if (indexRelation->rd_indam->aminsertcleanup)
+               indexRelation->rd_indam->aminsertcleanup(indexInfo);
+}
+
 /*
  * index_beginscan - start a scan of an index with amgettuple
  *

diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index a88b36a589ab091fa240395fd13aafbf2b9bdfee..0930f9b37e326a18c514e3ad525de1db1fd6d2d5 100644 (file)
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -122,6 +122,7 @@ bthandler(PG_FUNCTION_ARGS)
        amroutine->ambuild = btbuild;
        amroutine->ambuildempty = btbuildempty;
        amroutine->aminsert = btinsert;
+       amroutine->aminsertcleanup = NULL;
        amroutine->ambulkdelete = btbulkdelete;
        amroutine->amvacuumcleanup = btvacuumcleanup;
        amroutine->amcanreturn = btcanreturn;

diff --git a/src/backend/access/spgist/spgutils.c b/src/backend/access/spgist/spgutils.c
index c112e1e5dd4a7a4eb12d1a283e5001f799759273..30c00876a561fb7a927e091ec8ece9f10e5d2f81 100644 (file)
--- a/src/backend/access/spgist/spgutils.c
+++ b/src/backend/access/spgist/spgutils.c
@@ -70,6 +70,7 @@ spghandler(PG_FUNCTION_ARGS)
        amroutine->ambuild = spgbuild;
        amroutine->ambuildempty = spgbuildempty;
        amroutine->aminsert = spginsert;
+       amroutine->aminsertcleanup = NULL;
        amroutine->ambulkdelete = spgbulkdelete;
        amroutine->amvacuumcleanup = spgvacuumcleanup;
        amroutine->amcanreturn = spgcanreturn;

diff --git a/src/backend/executor/execIndexing.c b/src/backend/executor/execIndexing.c
index 384b39839a0aa6151428eae2617d25d02439d4dc..2fa2118f3c2cc110333c0ebd03fd176de0e1e3dd 100644 (file)
--- a/src/backend/executor/execIndexing.c
+++ b/src/backend/executor/execIndexing.c
@@ -233,15 +233,20 @@ ExecCloseIndices(ResultRelInfo *resultRelInfo)
        int                     i;
        int                     numIndices;
        RelationPtr indexDescs;
+       IndexInfo **indexInfos;
 
        numIndices = resultRelInfo->ri_NumIndices;
        indexDescs = resultRelInfo->ri_IndexRelationDescs;
+       indexInfos = resultRelInfo->ri_IndexRelationInfo;
 
        for (i = 0; i < numIndices; i++)
        {
                if (indexDescs[i] == NULL)
                        continue;                       /* shouldn't happen? */
 
+               /* Give the index a chance to do some post-insert cleanup */
+               index_insert_cleanup(indexDescs[i], indexInfos[i]);
+
                /* Drop lock acquired by ExecOpenIndices */
                index_close(indexDescs[i], RowExclusiveLock);
        }

diff --git a/src/include/access/amapi.h b/src/include/access/amapi.h
index 995725502a6e43df7953c7569d6f3fbbf5642e5d..244459587fc26cbb233c8cb560bd472c2413f359 100644 (file)
--- a/src/include/access/amapi.h
+++ b/src/include/access/amapi.h
@@ -113,6 +113,9 @@ typedef bool (*aminsert_function) (Relation indexRelation,
                                                                   bool indexUnchanged,
                                                                   struct IndexInfo *indexInfo);
 
+/* cleanup after insert */
+typedef void (*aminsertcleanup_function) (struct IndexInfo *indexInfo);
+
 /* bulk delete */
 typedef IndexBulkDeleteResult *(*ambulkdelete_function) (IndexVacuumInfo *info,
                                                                                                                 IndexBulkDeleteResult *stats,
@@ -261,6 +264,7 @@ typedef struct IndexAmRoutine
        ambuild_function ambuild;
        ambuildempty_function ambuildempty;
        aminsert_function aminsert;
+       aminsertcleanup_function aminsertcleanup;
        ambulkdelete_function ambulkdelete;
        amvacuumcleanup_function amvacuumcleanup;
        amcanreturn_function amcanreturn;       /* can be NULL */

diff --git a/src/include/access/brin_internal.h b/src/include/access/brin_internal.h
index 97ddc925b27fbf170615e3efbc297587ac3f336a..ed231e208eb57c62f504eb5220f046e02bf25b5c 100644 (file)
--- a/src/include/access/brin_internal.h
+++ b/src/include/access/brin_internal.h
@@ -96,6 +96,7 @@ extern bool brininsert(Relation idxRel, Datum *values, bool *nulls,
                                           IndexUniqueCheck checkUnique,
                                           bool indexUnchanged,
                                           struct IndexInfo *indexInfo);
+extern void brininsertcleanup(struct IndexInfo *indexInfo);
 extern IndexScanDesc brinbeginscan(Relation r, int nkeys, int norderbys);
 extern int64 bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm);
 extern void brinrescan(IndexScanDesc scan, ScanKey scankey, int nscankeys,

diff --git a/src/include/access/genam.h b/src/include/access/genam.h
index f31dec6ee0f6fef8ae81d11b7cd7b21bf93f1197..80dc8d54066dc50aedb2c469f1c3236913cd4727 100644 (file)
--- a/src/include/access/genam.h
+++ b/src/include/access/genam.h
@@ -148,6 +148,8 @@ extern bool index_insert(Relation indexRelation,
                                                 IndexUniqueCheck checkUnique,
                                                 bool indexUnchanged,
                                                 struct IndexInfo *indexInfo);
+extern void index_insert_cleanup(Relation indexRelation,
+                                                                struct IndexInfo *indexInfo);
 
 extern IndexScanDesc index_beginscan(Relation heapRelation,
                                                                         Relation indexRelation,