From c8df4831ef212b9028824d1b5de558973c110c77 Mon Sep 17 00:00:00 2001
From: Robert Haas <rhaas@postgresql.org>
Date: Thu, 16 Nov 2017 14:19:27 -0500
Subject: [PATCH] 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
---
 src/backend/access/gin/ginfast.c   | 14 +++++++++-----
 src/backend/access/gin/ginvacuum.c |  6 +++---
 src/include/access/gin_private.h   |  2 +-
 3 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/src/backend/access/gin/ginfast.c b/src/backend/access/gin/ginfast.c
index 59e435465ac..ff610ef2184 100644
--- a/src/backend/access/gin/ginfast.c
+++ b/src/backend/access/gin/ginfast.c
@@ -440,8 +440,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);
 }
 
 /*
@@ -727,7 +731,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,
@@ -744,7 +749,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
@@ -753,7 +757,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()
@@ -1016,7 +1020,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 31425e9963e..d3e4a177670 100644
--- 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 adfdb0c6d9c..f57ba5594c2 100644
--- 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 */
 
-- 
GitLab