Fix broken cleanup interlock for GIN pending list.

The pending list must (for correctness) always be cleaned up by vacuum, and
should (for the avoidance of surprising behavior) always be cleaned up
by an explicit call to gin_clean_pending_list, but cleanup is optional
when inserting.  The old logic got this backward: cleanup was forced
if (stats == NULL), but that's going to be *false* when vacuuming and
*true* for inserts.

Masahiko Sawada, reviewed by me.

Discussion: http://postgr.es/m/CAD21AoBLUSyiYKnTYtSAbC+F=XDjiaBrOUEGK+zUXdQ8owfPKw@mail.gmail.com

diff --git a/src/backend/access/gin/ginfast.c b/src/backend/access/gin/ginfast.c
index 6b709dbdb3a1b4981cbb0a7ca6d1da8df4f5ad7b..56a3ff590a45f00088ff9f0fd9fece0809bbe2fe 100644 (file)
--- a/src/backend/access/gin/ginfast.c
+++ b/src/backend/access/gin/ginfast.c
@@ -438,8 +438,12 @@ ginHeapTupleFastInsert(GinState *ginstate, GinTupleCollector *collector)
 
        END_CRIT_SECTION();
 
+       /*
+        * Since it could contend with concurrent cleanup process we cleanup
+        * pending list not forcibly.
+        */
        if (needCleanup)
-               ginInsertCleanup(ginstate, false, true, NULL);
+               ginInsertCleanup(ginstate, false, true, false, NULL);
 }
 
 /*
@@ -725,7 +729,8 @@ processPendingPage(BuildAccumulator *accum, KeyArray *ka,
  */
 void
 ginInsertCleanup(GinState *ginstate, bool full_clean,
-                                bool fill_fsm, IndexBulkDeleteResult *stats)
+                                bool fill_fsm, bool forceCleanup,
+                                IndexBulkDeleteResult *stats)
 {
        Relation        index = ginstate->index;
        Buffer          metabuffer,
@@ -742,7 +747,6 @@ ginInsertCleanup(GinState *ginstate, bool full_clean,
        bool            cleanupFinish = false;
        bool            fsm_vac = false;
        Size            workMemory;
-       bool            inVacuum = (stats == NULL);
 
        /*
         * We would like to prevent concurrent cleanup process. For that we will
@@ -751,7 +755,7 @@ ginInsertCleanup(GinState *ginstate, bool full_clean,
         * insertion into pending list
         */
 
-       if (inVacuum)
+       if (forceCleanup)
        {
                /*
                 * We are called from [auto]vacuum/analyze or gin_clean_pending_list()
@@ -1014,7 +1018,7 @@ gin_clean_pending_list(PG_FUNCTION_ARGS)
 
        memset(&stats, 0, sizeof(stats));
        initGinState(&ginstate, indexRel);
-       ginInsertCleanup(&ginstate, true, true, &stats);
+       ginInsertCleanup(&ginstate, true, true, true, &stats);
 
        index_close(indexRel, AccessShareLock);
 

diff --git a/src/backend/access/gin/ginvacuum.c b/src/backend/access/gin/ginvacuum.c
index 2685a1c373196612a613e6495e895967a8bd21f7..3d23cebf6283b78e20e9cbd7213d242fe0b63834 100644 (file)
--- a/src/backend/access/gin/ginvacuum.c
+++ b/src/backend/access/gin/ginvacuum.c
@@ -543,7 +543,7 @@ ginbulkdelete(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
                 * and cleanup any pending inserts
                 */
                ginInsertCleanup(&gvs.ginstate, !IsAutoVacuumWorkerProcess(),
-                                                false, stats);
+                                                false, true, stats);
        }
 
        /* we'll re-count the tuples each time */
@@ -656,7 +656,7 @@ ginvacuumcleanup(IndexVacuumInfo *info, IndexBulkDeleteResult *stats)
                if (IsAutoVacuumWorkerProcess())
                {
                        initGinState(&ginstate, index);
-                       ginInsertCleanup(&ginstate, false, true, stats);
+                       ginInsertCleanup(&ginstate, false, true, true, stats);
                }
                return stats;
        }
@@ -670,7 +670,7 @@ ginvacuumcleanup(IndexVacuumInfo *info, IndexBulkDeleteResult *stats)
                stats = (IndexBulkDeleteResult *) palloc0(sizeof(IndexBulkDeleteResult));
                initGinState(&ginstate, index);
                ginInsertCleanup(&ginstate, !IsAutoVacuumWorkerProcess(),
-                                                false, stats);
+                                                false, true, stats);
        }
 
        memset(&idxStat, 0, sizeof(idxStat));

diff --git a/src/include/access/gin_private.h b/src/include/access/gin_private.h
index 68cfe0c1ac0cf349da6f98801a3aa513c323d8bb..0b6006f0ad49d7e73005fa2d3ca0e4c2e8d6375a 100644 (file)
--- a/src/include/access/gin_private.h
+++ b/src/include/access/gin_private.h
@@ -948,7 +948,7 @@ extern void ginHeapTupleFastCollect(GinState *ginstate,
                                                OffsetNumber attnum, Datum value, bool isNull,
                                                ItemPointer ht_ctid);
 extern void ginInsertCleanup(GinState *ginstate, bool full_clean,
-                                bool fill_fsm, IndexBulkDeleteResult *stats);
+                                bool fill_fsm, bool forceCleanup, IndexBulkDeleteResult *stats);
 
 /* ginpostinglist.c */