Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 834aa56

Browse files
committed
Fix the logic for putting relations into the relcache init file.
Commit f3b5565 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.
1 parent 7e5859c commit 834aa56

File tree

3 files changed

+53
-11
lines changed

3 files changed

+53
-11
lines changed

src/backend/utils/cache/inval.c

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -463,11 +463,8 @@ RegisterRelcacheInvalidation(Oid dbId, Oid relId)
463463
/*
464464
* If the relation being invalidated is one of those cached in the local
465465
* relcache init file, mark that we need to zap that file at commit.
466-
* (Note: perhaps it would be better if this code were a bit more
467-
* decoupled from the knowledge that the init file contains exactly those
468-
* non-shared rels used in catalog caches.)
469466
*/
470-
if (OidIsValid(dbId) && RelationSupportsSysCache(relId))
467+
if (OidIsValid(dbId) && RelationIdIsInInitFile(relId))
471468
transInvalInfo->RelcacheInitFileInval = true;
472469
}
473470

src/backend/utils/cache/relcache.c

Lines changed: 51 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4441,21 +4441,32 @@ load_relcache_init_file(bool shared)
44414441
}
44424442

44434443
/*
4444-
* We reached the end of the init file without apparent problem. Did we
4445-
* get the right number of nailed items? (This is a useful crosscheck in
4446-
* case the set of critical rels or indexes changes.)
4444+
* We reached the end of the init file without apparent problem. Did we
4445+
* get the right number of nailed items? This is a useful crosscheck in
4446+
* case the set of critical rels or indexes changes. However, that should
4447+
* not happen in a normally-running system, so let's bleat if it does.
44474448
*/
44484449
if (shared)
44494450
{
44504451
if (nailed_rels != NUM_CRITICAL_SHARED_RELS ||
44514452
nailed_indexes != NUM_CRITICAL_SHARED_INDEXES)
4453+
{
4454+
elog(WARNING, "found %d nailed shared rels and %d nailed shared indexes in init file, but expected %d and %d respectively",
4455+
nailed_rels, nailed_indexes,
4456+
NUM_CRITICAL_SHARED_RELS, NUM_CRITICAL_SHARED_INDEXES);
44524457
goto read_failed;
4458+
}
44534459
}
44544460
else
44554461
{
44564462
if (nailed_rels != NUM_CRITICAL_LOCAL_RELS ||
44574463
nailed_indexes != NUM_CRITICAL_LOCAL_INDEXES)
4464+
{
4465+
elog(WARNING, "found %d nailed rels and %d nailed indexes in init file, but expected %d and %d respectively",
4466+
nailed_rels, nailed_indexes,
4467+
NUM_CRITICAL_LOCAL_RELS, NUM_CRITICAL_LOCAL_INDEXES);
44584468
goto read_failed;
4469+
}
44594470
}
44604471

44614472
/*
@@ -4573,12 +4584,19 @@ write_relcache_init_file(bool shared)
45734584
/*
45744585
* Ignore if not supposed to be in init file. We can allow any shared
45754586
* relation that's been loaded so far to be in the shared init file,
4576-
* but unshared relations must be used for catalog caches. (Note: if
4577-
* you want to change the criterion for rels to be kept in the init
4578-
* file, see also inval.c.)
4587+
* but unshared relations must be ones that should be in the local
4588+
* file per RelationIdIsInInitFile. (Note: if you want to change the
4589+
* criterion for rels to be kept in the init file, see also inval.c.
4590+
* The reason for filtering here is to be sure that we don't put
4591+
* anything into the local init file for which a relcache inval would
4592+
* not cause invalidation of that init file.)
45794593
*/
4580-
if (!shared && !RelationSupportsSysCache(RelationGetRelid(rel)))
4594+
if (!shared && !RelationIdIsInInitFile(RelationGetRelid(rel)))
4595+
{
4596+
/* Nailed rels had better get stored. */
4597+
Assert(!rel->rd_isnailed);
45814598
continue;
4599+
}
45824600

45834601
/* first write the relcache entry proper */
45844602
write_item(rel, sizeof(RelationData), fp);
@@ -4694,6 +4712,32 @@ write_item(const void *data, Size len, FILE *fp)
46944712
elog(FATAL, "could not write init file");
46954713
}
46964714

4715+
/*
4716+
* Determine whether a given relation (identified by OID) is one of the ones
4717+
* we should store in the local relcache init file.
4718+
*
4719+
* We must cache all nailed rels, and for efficiency we should cache every rel
4720+
* that supports a syscache. The former set is almost but not quite a subset
4721+
* of the latter. Currently, we must special-case TriggerRelidNameIndexId,
4722+
* which RelationCacheInitializePhase3 chooses to nail for efficiency reasons,
4723+
* but which does not support any syscache.
4724+
*
4725+
* Note: this function is currently never called for shared rels. If it were,
4726+
* we'd probably also need a special case for DatabaseNameIndexId, which is
4727+
* critical but does not support a syscache.
4728+
*/
4729+
bool
4730+
RelationIdIsInInitFile(Oid relationId)
4731+
{
4732+
if (relationId == TriggerRelidNameIndexId)
4733+
{
4734+
/* If this Assert fails, we don't need this special case anymore. */
4735+
Assert(!RelationSupportsSysCache(relationId));
4736+
return true;
4737+
}
4738+
return RelationSupportsSysCache(relationId);
4739+
}
4740+
46974741
/*
46984742
* Invalidate (remove) the init file during commit of a transaction that
46994743
* changed one or more of the relation cache entries that are kept in the

src/include/utils/relcache.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ extern void AtEOSubXact_RelationCache(bool isCommit, SubTransactionId mySubid,
105105
/*
106106
* Routines to help manage rebuilding of relcache init files
107107
*/
108+
extern bool RelationIdIsInInitFile(Oid relationId);
108109
extern void RelationCacheInitFilePreInvalidate(void);
109110
extern void RelationCacheInitFilePostInvalidate(void);
110111
extern void RelationCacheInitFileRemove(void);

0 commit comments

Comments
 (0)