Fix the logic for putting relations into the relcache init file.

Commit f3b5565dd4e59576be4c772da364704863e6a835 was a couple of bricks shy
of a load; specifically, it missed putting pg_trigger_tgrelid_tgname_index
into the relcache init file, because that index is not used by any
syscache.  However, we have historically nailed that index into cache for
performance reasons.  The upshot was that load_relcache_init_file always
decided that the init file was busted and silently ignored it, resulting
in a significant hit to backend startup speed.

To fix, reinstantiate RelationIdIsInInitFile() as a wrapper around
RelationSupportsSysCache(), which can know about additional relations
that should be in the init file despite being unknown to syscache.c.

Also install some guards against future mistakes of this type: make
write_relcache_init_file Assert that all nailed relations get written to
the init file, and make load_relcache_init_file emit a WARNING if it takes
the "wrong number of nailed relations" exit path.  Now that we remove the
init files during postmaster startup, that case should never occur in the
field, even if we are starting a minor-version update that added or removed
rels from the nailed set.  So the warning shouldn't ever be seen by end
users, but it will show up in the regression tests if somebody breaks this
logic.

Back-patch to all supported branches, like the previous commit.

diff --git a/src/backend/utils/cache/inval.c b/src/backend/utils/cache/inval.c
index 730a2949214eec3df62320320f30a81b289e1916..587a2cd248b079a122c60e8c183521cd95d4bd9c 100644 (file)
--- a/src/backend/utils/cache/inval.c
+++ b/src/backend/utils/cache/inval.c
@@ -509,11 +509,8 @@ RegisterRelcacheInvalidation(Oid dbId, Oid relId)
        /*
         * If the relation being invalidated is one of those cached in the local
         * relcache init file, mark that we need to zap that file at commit.
-        * (Note: perhaps it would be better if this code were a bit more
-        * decoupled from the knowledge that the init file contains exactly those
-        * non-shared rels used in catalog caches.)
         */
-       if (OidIsValid(dbId) && RelationSupportsSysCache(relId))
+       if (OidIsValid(dbId) && RelationIdIsInInitFile(relId))
                transInvalInfo->RelcacheInitFileInval = true;
 }
 

diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index a5521ca40a4f4c82db77eb7995c04b8675a54fbb..44e95098acbf20fc5b2843e92ce22a2d7550f6c2 100644 (file)
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -4879,21 +4879,32 @@ load_relcache_init_file(bool shared)
        }
 
        /*
-        * We reached the end of the init file without apparent problem. Did we
-        * get the right number of nailed items?  (This is a useful crosscheck in
-        * case the set of critical rels or indexes changes.)
+        * We reached the end of the init file without apparent problem.  Did we
+        * get the right number of nailed items?  This is a useful crosscheck in
+        * case the set of critical rels or indexes changes.  However, that should
+        * not happen in a normally-running system, so let's bleat if it does.
         */
        if (shared)
        {
                if (nailed_rels != NUM_CRITICAL_SHARED_RELS ||
                        nailed_indexes != NUM_CRITICAL_SHARED_INDEXES)
+               {
+                       elog(WARNING, "found %d nailed shared rels and %d nailed shared indexes in init file, but expected %d and %d respectively",
+                                nailed_rels, nailed_indexes,
+                                NUM_CRITICAL_SHARED_RELS, NUM_CRITICAL_SHARED_INDEXES);
                        goto read_failed;
+               }
        }
        else
        {
                if (nailed_rels != NUM_CRITICAL_LOCAL_RELS ||
                        nailed_indexes != NUM_CRITICAL_LOCAL_INDEXES)
+               {
+                       elog(WARNING, "found %d nailed rels and %d nailed indexes in init file, but expected %d and %d respectively",
+                                nailed_rels, nailed_indexes,
+                                NUM_CRITICAL_LOCAL_RELS, NUM_CRITICAL_LOCAL_INDEXES);
                        goto read_failed;
+               }
        }
 
        /*
@@ -5011,12 +5022,19 @@ write_relcache_init_file(bool shared)
                /*
                 * Ignore if not supposed to be in init file.  We can allow any shared
                 * relation that's been loaded so far to be in the shared init file,
-                * but unshared relations must be used for catalog caches.  (Note: if
-                * you want to change the criterion for rels to be kept in the init
-                * file, see also inval.c.)
+                * but unshared relations must be ones that should be in the local
+                * file per RelationIdIsInInitFile.  (Note: if you want to change the
+                * criterion for rels to be kept in the init file, see also inval.c.
+                * The reason for filtering here is to be sure that we don't put
+                * anything into the local init file for which a relcache inval would
+                * not cause invalidation of that init file.)
                 */
-               if (!shared && !RelationSupportsSysCache(RelationGetRelid(rel)))
+               if (!shared && !RelationIdIsInInitFile(RelationGetRelid(rel)))
+               {
+                       /* Nailed rels had better get stored. */
+                       Assert(!rel->rd_isnailed);
                        continue;
+               }
 
                /* first write the relcache entry proper */
                write_item(rel, sizeof(RelationData), fp);
@@ -5132,6 +5150,32 @@ write_item(const void *data, Size len, FILE *fp)
                elog(FATAL, "could not write init file");
 }
 
+/*
+ * Determine whether a given relation (identified by OID) is one of the ones
+ * we should store in the local relcache init file.
+ *
+ * We must cache all nailed rels, and for efficiency we should cache every rel
+ * that supports a syscache.  The former set is almost but not quite a subset
+ * of the latter.  Currently, we must special-case TriggerRelidNameIndexId,
+ * which RelationCacheInitializePhase3 chooses to nail for efficiency reasons,
+ * but which does not support any syscache.
+ *
+ * Note: this function is currently never called for shared rels.  If it were,
+ * we'd probably also need a special case for DatabaseNameIndexId, which is
+ * critical but does not support a syscache.
+ */
+bool
+RelationIdIsInInitFile(Oid relationId)
+{
+       if (relationId == TriggerRelidNameIndexId)
+       {
+               /* If this Assert fails, we don't need this special case anymore. */
+               Assert(!RelationSupportsSysCache(relationId));
+               return true;
+       }
+       return RelationSupportsSysCache(relationId);
+}
+
 /*
  * Invalidate (remove) the init file during commit of a transaction that
  * changed one or more of the relation cache entries that are kept in the

diff --git a/src/include/utils/relcache.h b/src/include/utils/relcache.h
index 9bea572f2cacb98376f74b015416a78eec001512..6953281b7419e76d747447645a91a8f87ee47c0d 100644 (file)
--- a/src/include/utils/relcache.h
+++ b/src/include/utils/relcache.h
@@ -116,6 +116,7 @@ extern void AtEOSubXact_RelationCache(bool isCommit, SubTransactionId mySubid,
 /*
  * Routines to help manage rebuilding of relcache init files
  */
+extern bool RelationIdIsInInitFile(Oid relationId);
 extern void RelationCacheInitFilePreInvalidate(void);
 extern void RelationCacheInitFilePostInvalidate(void);
 extern void RelationCacheInitFileRemove(void);