Re-think predicate locking on GIN indexes.

The principle behind the locking was not very well thought-out, and not
documented. Add a section in the README to explain how it's supposed to
work, and change the code so that it actually works that way.

This fixes two bugs:

1. If fast update was turned on concurrently, subsequent inserts to the
   pending list would not conflict with predicate locks that were acquired
   earlier, on entry pages. The included 'predicate-gin-fastupdate' test
   demonstrates that. To fix, make all scans acquire a predicate lock on
   the metapage. That lock represents a scan of the pending list, whether
   or not there is a pending list at the moment. Forget about the
   optimization to skip locking/checking for locks, when fastupdate=off.
2. If a scan finds no match, it still needs to lock the entry page. The
   point of predicate locks is to lock the gabs between values, whether
   or not there is a match. The included 'predicate-gin-nomatch' test
   tests that case.

In addition to those two bug fixes, this removes some unnecessary locking,
following the principle laid out in the README. Because all items in
a posting tree have the same key value, a lock on the posting tree root is
enough to cover all the items. (With a very large posting tree, it would
possibly be better to lock the posting tree leaf pages instead, so that a
"skip scan" with a query like "A & B", you could avoid unnecessary conflict
if a new tuple is inserted with A but !B. But let's keep this simple.)

Also, some spelling  fixes.

Author: Heikki Linnakangas with some editorization by me
Review: Andrey Borodin, Alexander Korotkov
Discussion: https://www.postgresql.org/message-id/0b3ad2c2-2692-62a9-3a04-5724f2af9114@iki.fi

1 files changed, 2 insertions, 24 deletions

diff --git a/src/backend/access/gin/gininsert.c b/src/backend/access/gin/gininsert.c
index cf218dd75d4..5281eb68238 100644
--- a/src/backend/access/gin/gininsert.c
+++ b/src/backend/access/gin/gininsert.c
@@ -219,7 +219,7 @@ ginEntryInsert(GinState *ginstate,
 			return;
 		}
 
-		GinCheckForSerializableConflictIn(btree.index, NULL, stack->buffer);
+		CheckForSerializableConflictIn(ginstate->index, NULL, stack->buffer);
 		/* modify an existing leaf entry */
 		itup = addItemPointersToLeafTuple(ginstate, itup,
 										  items, nitem, buildStats, stack->buffer);
@@ -228,7 +228,7 @@ ginEntryInsert(GinState *ginstate,
 	}
 	else
 	{
-		GinCheckForSerializableConflictIn(btree.index, NULL, stack->buffer);
+		CheckForSerializableConflictIn(ginstate->index, NULL, stack->buffer);
 		/* no match, so construct a new leaf entry */
 		itup = buildFreshLeafTuple(ginstate, attnum, key, category,
 								   items, nitem, buildStats, stack->buffer);
@@ -517,18 +517,6 @@ gininsert(Relation index, Datum *values, bool *isnull,
 
 		memset(&collector, 0, sizeof(GinTupleCollector));
 
-		/*
-		 * With fastupdate on each scan and each insert begin with access to
-		 * pending list, so it effectively lock entire index. In this case we
-		 * aquire predicate lock and check for conflicts over index relation,
-		 * and hope that it will reduce locking overhead.
-		 *
-		 * Do not use GinCheckForSerializableConflictIn() here, because it
-		 * will do nothing (it does actual work only with fastupdate off).
-		 * Check for conflicts for entire index.
-		 */
-		CheckForSerializableConflictIn(index, NULL, InvalidBuffer);
-
 		for (i = 0; i < ginstate->origTupdesc->natts; i++)
 			ginHeapTupleFastCollect(ginstate, &collector,
 									(OffsetNumber) (i + 1),
@@ -539,16 +527,6 @@ gininsert(Relation index, Datum *values, bool *isnull,
 	}
 	else
 	{
-		GinStatsData stats;
-
-		/*
-		 * Fastupdate is off but if pending list isn't empty then we need to
-		 * check conflicts with PredicateLockRelation in scanPendingInsert().
-		 */
-		ginGetStats(index, &stats);
-		if (stats.nPendingPages > 0)
-			CheckForSerializableConflictIn(index, NULL, InvalidBuffer);
-
 		for (i = 0; i < ginstate->origTupdesc->natts; i++)
 			ginHeapTupleInsert(ginstate, (OffsetNumber) (i + 1),
 							   values[i], isnull[i],