repro-ItemIdIsNormal-v1.patch

Author:     Noah Misch <noah@leadboat.com>
Commit:     Noah Misch <noah@leadboat.com>

    

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index d00300c..787a995 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -49,6 +49,7 @@
 #include "storage/predicate.h"
 #include "storage/procarray.h"
 #include "utils/datum.h"
+#include "utils/injection_point.h"
 #include "utils/inval.h"
 #include "utils/spccache.h"
 
@@ -3233,6 +3234,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
 	interesting_attrs = bms_add_members(interesting_attrs, id_attrs);
 
 	block = ItemPointerGetBlockNumber(otid);
+	INJECTION_POINT("heap_update-before-pin");
 	buffer = ReadBuffer(relation, block);
 	page = BufferGetPage(buffer);
 
@@ -6372,6 +6374,9 @@ heap_inplace_update_and_unlock(Relation relation,
 	 */
 	PreInplace_Inval();
 
+	INJECTION_POINT_LOAD("SIInsertDataEntries2");
+	INJECTION_POINT_LOAD("SIInsertDataEntries3");
+
 	/*----------
 	 * NO EREPORT(ERROR) from here till changes are complete
 	 *
diff --git a/src/backend/storage/ipc/sinvaladt.c b/src/backend/storage/ipc/sinvaladt.c
index ff81744..d837f41 100644
--- a/src/backend/storage/ipc/sinvaladt.c
+++ b/src/backend/storage/ipc/sinvaladt.c
@@ -25,6 +25,7 @@
 #include "storage/shmem.h"
 #include "storage/sinvaladt.h"
 #include "storage/spin.h"
+#include "utils/injection_point.h"
 
 /*
  * Conceptually, the shared cache invalidation messages are stored in an
@@ -386,6 +387,17 @@ SIInsertDataEntries(const SharedInvalidationMessage *data, int n)
 		int			max;
 		int			i;
 
+		/*
+		 * CacheInvalidateSmgr skips the injection point, since this
+		 * facilitates a test that needs this only for delaying pg_class
+		 * syscache invals and relcache invals.
+		 */
+		if (n != 1 || data->sm.id != SHAREDINVALSMGR_ID)
+		{
+			INJECTION_POINT("SIInsertDataEntries2");
+			INJECTION_POINT("SIInsertDataEntries3");
+		}
+
 		n -= nthistime;
 
 		LWLockAcquire(SInvalWriteLock, LW_EXCLUSIVE);
diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile
index 0753a9d..013c68e 100644
--- a/src/test/modules/injection_points/Makefile
+++ b/src/test/modules/injection_points/Makefile
@@ -13,7 +13,7 @@ PGFILEDESC = "injection_points - facility for injection points"
 REGRESS = injection_points reindex_conc
 REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress
 
-ISOLATION = basic inplace
+ISOLATION = basic syscache-update-pruned inplace
 
 TAP_TESTS = 1
 
diff --git a/src/test/modules/injection_points/expected/syscache-update-pruned.out b/src/test/modules/injection_points/expected/syscache-update-pruned.out
new file mode 100644
index 0000000..9caba9a
--- /dev/null
+++ b/src/test/modules/injection_points/expected/syscache-update-pruned.out
@@ -0,0 +1,133 @@
+Parsed test spec with 4 sessions
+
+starting permutation: cachefill1 at2 vac3 grant1 mkrels4 wakegrant4 wakeinval4
+mkrels
+------
+      
+(1 row)
+
+injection_points_attach
+-----------------------
+                       
+(1 row)
+
+injection_points_attach
+-----------------------
+                       
+(1 row)
+
+injection_points_attach
+-----------------------
+                       
+(1 row)
+
+step cachefill1: SELECT pg_relation_filenode('vactest.orig50') > 0;
+?column?
+--------
+t       
+(1 row)
+
+step at2: ALTER TABLE vactest.orig50 FORCE ROW LEVEL SECURITY; <waiting ...>
+step vac3: VACUUM pg_class; <waiting ...>
+step grant1: GRANT SELECT ON vactest.orig50 TO PUBLIC; <waiting ...>
+step mkrels4: 
+	SELECT vactest.mkrels('intruder', 1, 100);  -- repopulate LP_UNUSED
+
+mkrels
+------
+      
+(1 row)
+
+step wakegrant4: 
+	SELECT injection_points_detach('heap_update-before-pin');
+	SELECT injection_points_wakeup('heap_update-before-pin');
+
+injection_points_detach
+-----------------------
+                       
+(1 row)
+
+injection_points_wakeup
+-----------------------
+                       
+(1 row)
+
+step grant1: <... completed>
+ERROR:  duplicate key value violates unique constraint "pg_class_oid_index"
+step wakeinval4: 
+	SELECT injection_points_detach('SIInsertDataEntries2');
+	SELECT injection_points_detach('SIInsertDataEntries3');
+	SELECT injection_points_wakeup('SIInsertDataEntries2');
+	SELECT injection_points_wakeup('SIInsertDataEntries3');
+
+injection_points_detach
+-----------------------
+                       
+(1 row)
+
+injection_points_detach
+-----------------------
+                       
+(1 row)
+
+injection_points_wakeup
+-----------------------
+                       
+(1 row)
+
+injection_points_wakeup
+-----------------------
+                       
+(1 row)
+
+step at2: <... completed>
+step vac3: <... completed>
+
+starting permutation: cachefill1 at2 vac3 grant1 wakegrant4 wakeinval4
+mkrels
+------
+      
+(1 row)
+
+injection_points_attach
+-----------------------
+                       
+(1 row)
+
+injection_points_attach
+-----------------------
+                       
+(1 row)
+
+injection_points_attach
+-----------------------
+                       
+(1 row)
+
+step cachefill1: SELECT pg_relation_filenode('vactest.orig50') > 0;
+?column?
+--------
+t       
+(1 row)
+
+step at2: ALTER TABLE vactest.orig50 FORCE ROW LEVEL SECURITY; <waiting ...>
+step vac3: VACUUM pg_class; <waiting ...>
+step grant1: GRANT SELECT ON vactest.orig50 TO PUBLIC; <waiting ...>
+step wakegrant4: 
+	SELECT injection_points_detach('heap_update-before-pin');
+	SELECT injection_points_wakeup('heap_update-before-pin');
+
+injection_points_detach
+-----------------------
+                       
+(1 row)
+
+injection_points_wakeup
+-----------------------
+                       
+(1 row)
+
+PQconsumeInput failed: server closed the connection unexpectedly
+	This probably means the server terminated abnormally
+	before or while processing the request.
+
diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build
index 58f1900..626039e 100644
--- a/src/test/modules/injection_points/meson.build
+++ b/src/test/modules/injection_points/meson.build
@@ -43,6 +43,7 @@ tests += {
   'isolation': {
     'specs': [
       'basic',
+      'syscache-update-pruned',
       'inplace',
     ],
   },
diff --git a/src/test/modules/injection_points/specs/syscache-update-pruned.spec b/src/test/modules/injection_points/specs/syscache-update-pruned.spec
new file mode 100644
index 0000000..50dde9a
--- /dev/null
+++ b/src/test/modules/injection_points/specs/syscache-update-pruned.spec
@@ -0,0 +1,107 @@
+# Test race conditions involving:
+# - s1: heap_update($FROM_SYSCACHE), without a snapshot or pin
+# - s2: ALTER TABLE making $FROM_SYSCACHE a dead tuple
+# - s3: "VACUUM pg_class" making $FROM_SYSCACHE become LP_UNUSED
+
+# This is a derivative work of inplace.spec.  When introduced, that spec
+# reached basically the same race condition, for inplace updates.  Commit
+# a07e03f fixed the inplace update case.
+
+# Need s2 to make a non-HOT update.  Otherwise, "VACUUM pg_class" would leave
+# an LP_REDIRECT that persists.  To get non-HOT, make rels so the pg_class row
+# for vactest.orig50 is on a filled page (assuming BLCKSZ=8192).  Just to save
+# on filesystem syscalls, use relkind=c for every other rel.
+setup
+{
+	CREATE EXTENSION injection_points;
+	CREATE SCHEMA vactest;
+	CREATE FUNCTION vactest.mkrels(text, int, int) RETURNS void
+		LANGUAGE plpgsql SET search_path = vactest AS $$
+	DECLARE
+		tname text;
+	BEGIN
+		FOR i in $2 .. $3 LOOP
+			tname := $1 || i;
+			EXECUTE FORMAT('CREATE TYPE ' || tname || ' AS ()');
+			RAISE DEBUG '% at %', tname, ctid
+				FROM pg_class WHERE oid = tname::regclass;
+		END LOOP;
+	END
+	$$;
+}
+setup	{ VACUUM FULL pg_class;  -- reduce free space }
+setup
+{
+	SELECT vactest.mkrels('orig', 1, 49);
+	CREATE TABLE vactest.orig50 ();
+	SELECT vactest.mkrels('orig', 51, 100);
+}
+teardown
+{
+	DROP SCHEMA vactest CASCADE;
+	DROP EXTENSION injection_points;
+}
+
+# Wait during GRANT.
+session s1
+setup	{
+	SELECT injection_points_set_local();
+	SELECT injection_points_attach('heap_update-before-pin', 'wait');
+}
+step cachefill1	{ SELECT pg_relation_filenode('vactest.orig50') > 0; }
+step grant1	{ GRANT SELECT ON vactest.orig50 TO PUBLIC; }
+
+# Update of the tuple grant1 will update.  It has to be an update that uses a
+# relation lock instead of LockTuple(), else GRANT would block it.  Wait
+# before sending invals, so s1 will not get a cache miss.
+session s2
+setup	{
+	SELECT injection_points_set_local();
+	SELECT injection_points_attach('SIInsertDataEntries2', 'wait');
+}
+step at2		{ ALTER TABLE vactest.orig50 FORCE ROW LEVEL SECURITY; }
+
+# VACUUM to achieve LP_UNUSED.  Again delay any s1 cache miss.
+session s3
+setup	{
+	SELECT injection_points_set_local();
+	SELECT injection_points_attach('SIInsertDataEntries3', 'wait');
+}
+step vac3		{ VACUUM pg_class; }
+
+# Non-blocking actions.
+session s4
+# Reuse the lp that s1 is waiting to change.  I've observed reuse at the 1st
+# or 18th CREATE, so create excess.
+step mkrels4	{
+	SELECT vactest.mkrels('intruder', 1, 100);  -- repopulate LP_UNUSED
+}
+step wakegrant4	{
+	SELECT injection_points_detach('heap_update-before-pin');
+	SELECT injection_points_wakeup('heap_update-before-pin');
+}
+step wakeinval4	{
+	SELECT injection_points_detach('SIInsertDataEntries2');
+	SELECT injection_points_detach('SIInsertDataEntries3');
+	SELECT injection_points_wakeup('SIInsertDataEntries2');
+	SELECT injection_points_wakeup('SIInsertDataEntries3');
+}
+
+# tuple $FROM_SYSCACHE changes from LP_UNUSED to a successor
+permutation
+	cachefill1		# reads pg_class tuple T0 for vactest.orig50, xmax invalid
+	at2(wakeinval4)	# T0 becomes eligible for pruning, T1 is successor
+	vac3(at2)		# T0 becomes LP_UNUSED
+	grant1(wakegrant4)	# pauses at heap_update(T0)
+	mkrels4			# T0 becomes a new rel
+	wakegrant4		# s1 wakes, "duplicate key value violates unique"
+	wakeinval4
+
+# subtract mkrels4: tuple $FROM_SYSCACHE becomes LP_UNUSED
+permutation
+	cachefill1		# reads pg_class tuple T0 for vactest.orig50, xmax invalid
+	at2(wakeinval4)	# T0 becomes eligible for pruning, T1 is successor
+	vac3(at2)		# T0 becomes LP_UNUSED
+	grant1(wakegrant4)	# pauses at heap_update(T0)
+	wakegrant4		# s1 wakes, assertion failure
+	wakeinval4