Date: Sun, 15 Aug 2021 22:13:17 -0700
From: Noah Misch <noah@leadboat.com>
To: Andrey Borodin <x4mmm@yandex-team.ru>
Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>,
	Michael Paquier <michael@paquier.xyz>, Tom Lane <tgl@sss.pgh.pa.us>,
	Peter Geoghegan <pg@bowt.ie>
Subject: Re: CREATE INDEX CONCURRENTLY does not index prepared xact's data
Message-ID: <20210816051317.GA1409586@rfd.leadboat.com>
References: 
 <CAH2-Wz=Et8WLvS6d7-BH_yT9b5=cV89fXBmY0T-KgsT7R0=R_A@mail.gmail.com>
 <85D32B53-FB25-4F2F-A253-0D7B26F72881@yandex-team.ru>
 <DBD361BB-4327-4143-A849-36A262B22CEE@yandex-team.ru>
 <341252AE-B633-471E-9FF4-B09FC5D036E2@yandex-team.ru>
 <20210807221957.GA169359@rfd.leadboat.com>
 <5E041A70-4946-489C-9B6D-764DF627A92D@yandex-team.ru>
 <20210808163752.GB23176@rfd.leadboat.com>
 <20210814221328.GA146778@rfd.leadboat.com>
 <20210815084535.GA1227376@rfd.leadboat.com>
 <E86CBC3F-1FCF-44BF-BAD8-1301F66D8767@yandex-team.ru>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="DocE+STaALJfprDB"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <E86CBC3F-1FCF-44BF-BAD8-1301F66D8767@yandex-team.ru>
User-Agent: Mutt/1.5.24 (2015-08-30)
Archived-At: 
 <https://www.postgresql.org/message-id/20210816051317.GA1409586%40rfd.leadboat.com>
Precedence: bulk


--DocE+STaALJfprDB
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On Sun, Aug 15, 2021 at 04:09:37PM +0500, Andrey Borodin wrote:
> > 15 авг. 2021 г., в 13:45, Noah Misch <noah@leadboat.com> написал(а):
> >> Do you see failures with that loop?  If so, can you diagnose them?

> I do not observe failure on my laptop, though reproduced it on a linux server.
> I've fixed one bug in TwoPhaseGetXidByVXid(). Also rebased on actual master.

> > Just one 1PC failure in six hours of 1PC test runtime, though.
> I've attached a patch that reproduces the problem in 30sec on my server.

Having studied the broader inval situation, I found just one additional gap
that seemed potentially relevant.  It didn't stop the failures under current
tests, however.  The attached patch replaces my last patch on this thread, so
it should replace
v12-0002-PoC-fix-for-race-in-RelationBuildDesc-and-relcac.patch in your
series.  (Like its predecessor, it's an unfinished proof-of-concept.)

With v12, on my machine, the same loop took 2000s to get three failures, both
of which were in the 1PC tests.  I ran out of time to study the failure
mechanism.  Would you diagnose what happens when it fails on your server?
Also see the larger review from Andres.

--DocE+STaALJfprDB
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="inval-build-race-v0.1.patch"

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

    
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 13d9994..da6d8d6 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -997,9 +997,16 @@ equalRSDesc(RowSecurityDesc *rsdesc1, RowSecurityDesc *rsdesc2)
  *		(suggesting we are trying to access a just-deleted relation).
  *		Any other error is reported via elog.
  */
+typedef struct InProgressRels {
+	Oid relid;
+	bool invalidated;
+} InProgressRels;
+static InProgressRels inProgress[100];
+
 static Relation
 RelationBuildDesc(Oid targetRelId, bool insertIt)
 {
+	int in_progress_offset;
 	Relation	relation;
 	Oid			relid;
 	HeapTuple	pg_class_tuple;
@@ -1033,6 +1040,14 @@ RelationBuildDesc(Oid targetRelId, bool insertIt)
 	}
 #endif
 
+	for (in_progress_offset = 0;
+		 OidIsValid(inProgress[in_progress_offset].relid);
+		 in_progress_offset++)
+		;
+	inProgress[in_progress_offset].relid = targetRelId;
+retry:
+	inProgress[in_progress_offset].invalidated = false;
+
 	/*
 	 * find the tuple in pg_class corresponding to the given relation id
 	 */
@@ -1213,6 +1228,10 @@ RelationBuildDesc(Oid targetRelId, bool insertIt)
 	 */
 	heap_freetuple(pg_class_tuple);
 
+	if (inProgress[in_progress_offset].invalidated)
+		goto retry;				/* TODO free old one */
+	inProgress[in_progress_offset].relid = InvalidOid;
+
 	/*
 	 * Insert newly created relation into relcache hash table, if requested.
 	 *
@@ -2805,6 +2824,13 @@ RelationCacheInvalidateEntry(Oid relationId)
 		relcacheInvalsReceived++;
 		RelationFlushRelation(relation);
 	}
+	else
+	{
+		int i;
+		for (i = 0; OidIsValid(inProgress[i].relid); i++)
+			if (inProgress[i].relid == relationId)
+				inProgress[i].invalidated = true;
+	}
 }
 
 /*
@@ -2845,6 +2871,7 @@ RelationCacheInvalidate(void)
 	List	   *rebuildFirstList = NIL;
 	List	   *rebuildList = NIL;
 	ListCell   *l;
+	int i;
 
 	/*
 	 * Reload relation mapping data before starting to reconstruct cache.
@@ -2931,6 +2958,10 @@ RelationCacheInvalidate(void)
 		RelationClearRelation(relation, true);
 	}
 	list_free(rebuildList);
+
+	/* Any RelationBuildDesc() on the stack must start over. */
+	for (i = 0; OidIsValid(inProgress[i].relid); i++)
+		inProgress[i].invalidated = true;
 }
 
 /*

--DocE+STaALJfprDB--