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 00348891a26b583858a969f170f11a0175b16935..95c8bd7b43937c7b94a227f11376be21f55c63be 100644 (file)
--- a/src/backend/access/gin/ginfast.c
+++ b/src/backend/access/gin/ginfast.c
@@ -450,8 +450,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);
 }
 
 /*
@@ -748,7 +752,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,
@@ -765,7 +770,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
@@ -774,7 +778,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()
@@ -1036,7 +1040,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 a20a99c81404a79fba884c222b9846d20b5726ad..394bc832a409e85a8b160e84fc5b2083482cab51 100644 (file)
--- a/src/backend/access/gin/ginvacuum.c
+++ b/src/backend/access/gin/ginvacuum.c
@@ -570,7 +570,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 */
@@ -683,7 +683,7 @@ ginvacuumcleanup(IndexVacuumInfo *info, IndexBulkDeleteResult *stats)
                if (IsAutoVacuumWorkerProcess())
                {
                        initGinState(&ginstate, index);
-                       ginInsertCleanup(&ginstate, false, true, stats);
+                       ginInsertCleanup(&ginstate, false, true, true, stats);
                }
                return stats;
        }
@@ -697,7 +697,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 7b5c845b83fe2290554cdcabdbbed586e9e46160..dc49b6f17d24ede4866f59095daf547be81bd96a 100644 (file)
--- a/src/include/access/gin_private.h
+++ b/src/include/access/gin_private.h
@@ -439,7 +439,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 */