Use a safer method for determining whether relcache init file is stale.

When we invalidate the relcache entry for a system catalog or index, we
must also delete the relcache "init file" if the init file contains a copy
of that rel's entry.  The old way of doing this relied on a specially
maintained list of the OIDs of relations present in the init file: we made
the list either when reading the file in, or when writing the file out.
The problem is that when writing the file out, we included only rels
present in our local relcache, which might have already suffered some
deletions due to relcache inval events.  In such cases we correctly decided
not to overwrite the real init file with incomplete data --- but we still
used the incomplete initFileRelationIds list for the rest of the current
session.  This could result in wrong decisions about whether the session's
own actions require deletion of the init file, potentially allowing an init
file created by some other concurrent session to be left around even though
it's been made stale.

Since we don't support changing the schema of a system catalog at runtime,
the only likely scenario in which this would cause a problem in the field
involves a "vacuum full" on a catalog concurrently with other activity, and
even then it's far from easy to provoke.  Remarkably, this has been broken
since 2002 (in commit 786340441706ac1957a031f11ad1c2e5b6e18314), but we had
never seen a reproducible test case until recently.  If it did happen in
the field, the symptoms would probably involve unexpected "cache lookup
failed" errors to begin with, then "could not open file" failures after the
next checkpoint, as all accesses to the affected catalog stopped working.
Recovery would require manually removing the stale "pg_internal.init" file.

To fix, get rid of the initFileRelationIds list, and instead consult
syscache.c's list of relations used in catalog caches to decide whether a
relation is included in the init file.  This should be a tad more efficient
anyway, since we're replacing linear search of a list with ~100 entries
with a binary search.  It's a bit ugly that the init file contents are now
so directly tied to the catalog caches, but in practice that won't make
much difference.

Back-patch to all supported branches.

1 files changed, 61 insertions, 11 deletions

diff --git a/src/backend/utils/cache/syscache.c b/src/backend/utils/cache/syscache.c
index 94d951ce056..81cde1295f3 100644
--- a/src/backend/utils/cache/syscache.c
+++ b/src/backend/utils/cache/syscache.c
@@ -798,17 +798,23 @@ static const struct cachedesc cacheinfo[] = {
 	}
 };
 
-static CatCache *SysCache[
-						  lengthof(cacheinfo)];
-static int	SysCacheSize = lengthof(cacheinfo);
+#define SysCacheSize	((int) lengthof(cacheinfo))
+
+static CatCache *SysCache[SysCacheSize];
+
 static bool CacheInitialized = false;
 
-static Oid	SysCacheRelationOid[
-								lengthof(cacheinfo)];
+/* Sorted array of OIDs of tables that have caches on them */
+static Oid	SysCacheRelationOid[SysCacheSize];
 static int	SysCacheRelationOidSize;
 
+/* Sorted array of OIDs of tables and indexes used by caches */
+static Oid	SysCacheSupportingRelOid[SysCacheSize * 2];
+static int	SysCacheSupportingRelOidSize;
+
 static int	oid_compare(const void *a, const void *b);
 
+
 /*
  * InitCatalogCache - initialize the caches
  *
@@ -822,11 +828,11 @@ InitCatalogCache(void)
 {
 	int			cacheId;
 	int			i,
-				j = 0;
+				j;
 
 	Assert(!CacheInitialized);
 
-	MemSet(SysCache, 0, sizeof(SysCache));
+	SysCacheRelationOidSize = SysCacheSupportingRelOidSize = 0;
 
 	for (cacheId = 0; cacheId < SysCacheSize; cacheId++)
 	{
@@ -839,20 +845,39 @@ InitCatalogCache(void)
 		if (!PointerIsValid(SysCache[cacheId]))
 			elog(ERROR, "could not initialize cache %u (%d)",
 				 cacheinfo[cacheId].reloid, cacheId);
+		/* Accumulate data for OID lists, too */
 		SysCacheRelationOid[SysCacheRelationOidSize++] =
 			cacheinfo[cacheId].reloid;
+		SysCacheSupportingRelOid[SysCacheSupportingRelOidSize++] =
+			cacheinfo[cacheId].reloid;
+		SysCacheSupportingRelOid[SysCacheSupportingRelOidSize++] =
+			cacheinfo[cacheId].indoid;
 		/* see comments for RelationInvalidatesSnapshotsOnly */
 		Assert(!RelationInvalidatesSnapshotsOnly(cacheinfo[cacheId].reloid));
 	}
 
-	/* Sort and dedup OIDs. */
+	Assert(SysCacheRelationOidSize <= lengthof(SysCacheRelationOid));
+	Assert(SysCacheSupportingRelOidSize <= lengthof(SysCacheSupportingRelOid));
+
+	/* Sort and de-dup OID arrays, so we can use binary search. */
 	pg_qsort(SysCacheRelationOid, SysCacheRelationOidSize,
 			 sizeof(Oid), oid_compare);
-	for (i = 1; i < SysCacheRelationOidSize; ++i)
+	for (i = 1, j = 0; i < SysCacheRelationOidSize; i++)
+	{
 		if (SysCacheRelationOid[i] != SysCacheRelationOid[j])
 			SysCacheRelationOid[++j] = SysCacheRelationOid[i];
+	}
 	SysCacheRelationOidSize = j + 1;
 
+	pg_qsort(SysCacheSupportingRelOid, SysCacheSupportingRelOidSize,
+			 sizeof(Oid), oid_compare);
+	for (i = 1, j = 0; i < SysCacheSupportingRelOidSize; i++)
+	{
+		if (SysCacheSupportingRelOid[i] != SysCacheSupportingRelOid[j])
+			SysCacheSupportingRelOid[++j] = SysCacheSupportingRelOid[i];
+	}
+	SysCacheSupportingRelOidSize = j + 1;
+
 	CacheInitialized = true;
 }
 
@@ -1195,6 +1220,31 @@ RelationHasSysCache(Oid relid)
 	return false;
 }
 
+/*
+ * Test whether a relation supports a system cache, ie it is either a
+ * cached table or the index used for a cache.
+ */
+bool
+RelationSupportsSysCache(Oid relid)
+{
+	int			low = 0,
+				high = SysCacheSupportingRelOidSize - 1;
+
+	while (low <= high)
+	{
+		int			middle = low + (high - low) / 2;
+
+		if (SysCacheSupportingRelOid[middle] == relid)
+			return true;
+		if (SysCacheSupportingRelOid[middle] < relid)
+			low = middle + 1;
+		else
+			high = middle - 1;
+	}
+
+	return false;
+}
+
 
 /*
  * OID comparator for pg_qsort
@@ -1202,8 +1252,8 @@ RelationHasSysCache(Oid relid)
 static int
 oid_compare(const void *a, const void *b)
 {
-	Oid			oa = *((Oid *) a);
-	Oid			ob = *((Oid *) b);
+	Oid			oa = *((const Oid *) a);
+	Oid			ob = *((const Oid *) b);
 
 	if (oa == ob)
 		return 0;