Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commita54e1f1

Browse files
committed
Fix bugs in vacuum of shared rels, by keeping their relcache entries current.
When vacuum processes a relation it uses the corresponding relcacheentry's relfrozenxid / relminmxid as a cutoff for when to removetuples etc. Unfortunately for nailed relations (i.e. critical systemcatalogs) bugs could frequently lead to the corresponding relcacheentry being stale.This set of bugs could cause actual data corruption as vacuum wouldpotentially not remove the correct row versions, potentially revivingthem at a later point. After699bf7d some corruptions in this veinwere prevented, but the additional error checks could also triggerspuriously. Examples of such errors are: ERROR: found xmin ... from before relfrozenxid ...and ERROR: found multixact ... from before relminmxid ...To be caused by this bug the errors have to occur on system catalogtables.The two bugs are:1) Invalidations for nailed relations were ignored, based on the theory that the relcache entry for such tables doesn't change. Which is largely true, except for fields like relfrozenxid etc. This means that changes to relations vacuumed in other sessions weren't picked up by already existing sessions. Luckily autovacuum doesn't have particularly longrunning sessions.2) For shared *and* nailed relations, the shared relcache init file was never invalidated while running. That means that for such tables (e.g. pg_authid, pg_database) it's not just already existing sessions that are affected, but even new connections are as well. That explains why the reports usually were about pg_authid et. al.To fix 1), revalidate the rd_rel portion of a relcache entry wheninvalid. This implies a bit of extra complexity to deal withbootstrapping, but it's not too bad. The fix for 2) is simpler,simply always remove both the shared and local init files.Author: Andres FreundReviewed-By: Alvaro HerreraDiscussion:https://postgr.es/m/20180525203736.crkbg36muzxrjj5e@alap3.anarazel.dehttps://postgr.es/m/CAMa1XUhKSJd98JW4o9StWPrfS=11bPgG+_GDMxe25TvUY4Sugg@mail.gmail.comhttps://postgr.es/m/CAKMFJucqbuoDRfxPDX39WhA3vJyxweRg_zDVXzncr6+5wOguWA@mail.gmail.comhttps://postgr.es/m/CAGewt-ujGpMLQ09gXcUFMZaZsGJC98VXHEFbF-tpPB0fB13K+A@mail.gmail.comBackpatch: 9.3-
1 parent8a07ebb commita54e1f1

File tree

3 files changed

+146
-75
lines changed

3 files changed

+146
-75
lines changed

‎src/backend/utils/cache/inval.c

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -521,12 +521,12 @@ RegisterRelcacheInvalidation(Oid dbId, Oid relId)
521521
(void)GetCurrentCommandId(true);
522522

523523
/*
524-
* If the relation being invalidated is one of those cached in the local
525-
* relcache init file, mark that we need to zap that file at commit. Same
526-
* is true when we are invalidating whole relcache.
524+
* If the relation being invalidated is one of those cached in a relcache
525+
* init file, mark that we need to zap that file at commit. For simplicity
526+
* invalidations for a specific database always invalidate the shared file
527+
* as well. Also zap when we are invalidating whole relcache.
527528
*/
528-
if (OidIsValid(dbId)&&
529-
(RelationIdIsInInitFile(relId)||relId==InvalidOid))
529+
if (relId==InvalidOid||RelationIdIsInInitFile(relId))
530530
transInvalInfo->RelcacheInitFileInval= true;
531531
}
532532

@@ -881,18 +881,26 @@ ProcessCommittedInvalidationMessages(SharedInvalidationMessage *msgs,
881881

882882
if (RelcacheInitFileInval)
883883
{
884+
elog(trace_recovery(DEBUG4),"removing relcache init files for database %u",
885+
dbid);
886+
884887
/*
885-
* RelationCacheInitFilePreInvalidate requires DatabasePath to be set,
886-
* but we should not use SetDatabasePath during recovery, since it is
888+
* RelationCacheInitFilePreInvalidate, when the invalidation message
889+
* is for a specific database, requires DatabasePath to be set, but we
890+
* should not use SetDatabasePath during recovery, since it is
887891
* intended to be used only once by normal backends. Hence, a quick
888892
* hack: set DatabasePath directly then unset after use.
889893
*/
890-
DatabasePath=GetDatabasePath(dbid,tsid);
891-
elog(trace_recovery(DEBUG4),"removing relcache init file in \"%s\"",
892-
DatabasePath);
894+
if (OidIsValid(dbid))
895+
DatabasePath=GetDatabasePath(dbid,tsid);
896+
893897
RelationCacheInitFilePreInvalidate();
894-
pfree(DatabasePath);
895-
DatabasePath=NULL;
898+
899+
if (OidIsValid(dbid))
900+
{
901+
pfree(DatabasePath);
902+
DatabasePath=NULL;
903+
}
896904
}
897905

898906
SendSharedInvalidMessages(msgs,nmsgs);

‎src/backend/utils/cache/relcache.c

Lines changed: 125 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,7 @@ static void RelationDestroyRelation(Relation relation, bool remember_tupdesc);
250250
staticvoidRelationClearRelation(Relationrelation,boolrebuild);
251251

252252
staticvoidRelationReloadIndexInfo(Relationrelation);
253+
staticvoidRelationReloadNailed(Relationrelation);
253254
staticvoidRelationFlushRelation(Relationrelation);
254255
staticvoidRememberToFreeTupleDescAtEOX(TupleDesctd);
255256
staticvoidAtEOXact_cleanup(Relationrelation,boolisCommit);
@@ -286,7 +287,7 @@ static void IndexSupportInitialize(oidvector *indclass,
286287
staticOpClassCacheEnt*LookupOpclassInfo(OidoperatorClassOid,
287288
StrategyNumbernumSupport);
288289
staticvoidRelationCacheInitFileRemoveInDir(constchar*tblspcpath);
289-
staticvoidunlink_initfile(constchar*initfilename);
290+
staticvoidunlink_initfile(constchar*initfilename,intelevel);
290291
staticboolequalPartitionDescs(PartitionKeykey,PartitionDescpartdesc1,
291292
PartitionDescpartdesc2);
292293

@@ -1931,7 +1932,16 @@ RelationIdGetRelation(Oid relationId)
19311932
RelationReloadIndexInfo(rd);
19321933
else
19331934
RelationClearRelation(rd, true);
1934-
Assert(rd->rd_isvalid);
1935+
1936+
/*
1937+
* Normally entries need to be valid here, but before the relcache
1938+
* has been initialized, not enough infrastructure exists to
1939+
* perform pg_class lookups. The structure of such entries doesn't
1940+
* change, but we still want to update the rd_rel entry. So
1941+
* rd_isvalid = false is left in place for a later lookup.
1942+
*/
1943+
Assert(rd->rd_isvalid||
1944+
(rd->rd_isnailed&& !criticalRelcachesBuilt));
19351945
}
19361946
returnrd;
19371947
}
@@ -2135,6 +2145,81 @@ RelationReloadIndexInfo(Relation relation)
21352145
relation->rd_isvalid= true;
21362146
}
21372147

2148+
/*
2149+
* RelationReloadNailed - reload minimal information for nailed relations.
2150+
*
2151+
* The structure of a nailed relation can never change (which is good, because
2152+
* we rely on knowing their structure to be able to read catalog content). But
2153+
* some parts, e.g. pg_class.relfrozenxid, are still important to have
2154+
* accurate content for. Therefore those need to be reloaded after the arrival
2155+
* of invalidations.
2156+
*/
2157+
staticvoid
2158+
RelationReloadNailed(Relationrelation)
2159+
{
2160+
Assert(relation->rd_isnailed);
2161+
2162+
/*
2163+
* Redo RelationInitPhysicalAddr in case it is a mapped relation whose
2164+
* mapping changed.
2165+
*/
2166+
RelationInitPhysicalAddr(relation);
2167+
2168+
/* flag as needing to be revalidated */
2169+
relation->rd_isvalid= false;
2170+
2171+
/*
2172+
* Can only reread catalog contents if in a transaction. If the relation
2173+
* is currently open (not counting the nailed refcount), do so
2174+
* immediately. Otherwise we've already marked the entry as possibly
2175+
* invalid, and it'll be fixed when next opened.
2176+
*/
2177+
if (!IsTransactionState()||relation->rd_refcnt <=1)
2178+
return;
2179+
2180+
if (relation->rd_rel->relkind==RELKIND_INDEX)
2181+
{
2182+
/*
2183+
* If it's a nailed-but-not-mapped index, then we need to re-read the
2184+
* pg_class row to see if its relfilenode changed.
2185+
*/
2186+
RelationReloadIndexInfo(relation);
2187+
}
2188+
else
2189+
{
2190+
/*
2191+
* Reload a non-index entry. We can't easily do so if relcaches
2192+
* aren't yet built, but that's fine because at that stage the
2193+
* attributes that need to be current (like relfrozenxid) aren't yet
2194+
* accessed. To ensure the entry will later be revalidated, we leave
2195+
* it in invalid state, but allow use (cf. RelationIdGetRelation()).
2196+
*/
2197+
if (criticalRelcachesBuilt)
2198+
{
2199+
HeapTuplepg_class_tuple;
2200+
Form_pg_classrelp;
2201+
2202+
/*
2203+
* NB: Mark the entry as valid before starting to scan, to avoid
2204+
* self-recursion when re-building pg_class.
2205+
*/
2206+
relation->rd_isvalid= true;
2207+
2208+
pg_class_tuple=ScanPgRelation(RelationGetRelid(relation),
2209+
true, false);
2210+
relp= (Form_pg_class)GETSTRUCT(pg_class_tuple);
2211+
memcpy(relation->rd_rel,relp,CLASS_TUPLE_SIZE);
2212+
heap_freetuple(pg_class_tuple);
2213+
2214+
/*
2215+
* Again mark as valid, to protect against concurrently arriving
2216+
* invalidations.
2217+
*/
2218+
relation->rd_isvalid= true;
2219+
}
2220+
}
2221+
}
2222+
21382223
/*
21392224
* RelationDestroyRelation
21402225
*
@@ -2250,27 +2335,12 @@ RelationClearRelation(Relation relation, bool rebuild)
22502335
RelationCloseSmgr(relation);
22512336

22522337
/*
2253-
* Never, never ever blow away a nailed-in system relation, because we'd
2254-
* be unable to recover. However, we must redo RelationInitPhysicalAddr
2255-
* in case it is a mapped relation whose mapping changed.
2256-
*
2257-
* If it's a nailed-but-not-mapped index, then we need to re-read the
2258-
* pg_class row to see if its relfilenode changed. We do that immediately
2259-
* if we're inside a valid transaction and the relation is open (not
2260-
* counting the nailed refcount). Otherwise just mark the entry as
2261-
* possibly invalid, and it'll be fixed when next opened.
2338+
* Treat nailed-in system relations separately, they always need to be
2339+
* accessible, so we can't blow them away.
22622340
*/
22632341
if (relation->rd_isnailed)
22642342
{
2265-
RelationInitPhysicalAddr(relation);
2266-
2267-
if (relation->rd_rel->relkind==RELKIND_INDEX||
2268-
relation->rd_rel->relkind==RELKIND_PARTITIONED_INDEX)
2269-
{
2270-
relation->rd_isvalid= false;/* needs to be revalidated */
2271-
if (relation->rd_refcnt>1&&IsTransactionState())
2272-
RelationReloadIndexInfo(relation);
2273-
}
2343+
RelationReloadNailed(relation);
22742344
return;
22752345
}
22762346

@@ -5907,24 +5977,26 @@ write_item(const void *data, Size len, FILE *fp)
59075977

59085978
/*
59095979
* Determine whether a given relation (identified by OID) is one of the ones
5910-
* we should store inthe local relcache init file.
5980+
* we should store ina relcache init file.
59115981
*
59125982
* We must cache all nailed rels, and for efficiency we should cache every rel
59135983
* that supports a syscache. The former set is almost but not quite a subset
5914-
* of the latter. Currently, we must special-case TriggerRelidNameIndexId,
5915-
* which RelationCacheInitializePhase3 chooses to nail for efficiency reasons,
5916-
* but which does not support any syscache.
5917-
*
5918-
* Note: this function is currently never called for shared rels. If it were,
5919-
* we'd probably also need a special case for DatabaseNameIndexId, which is
5920-
* critical but does not support a syscache.
5984+
* of the latter. The special cases are relations where
5985+
* RelationCacheInitializePhase2/3 chooses to nail for efficiency reasons, but
5986+
* which do not support any syscache.
59215987
*/
59225988
bool
59235989
RelationIdIsInInitFile(OidrelationId)
59245990
{
5925-
if (relationId==TriggerRelidNameIndexId)
5991+
if (relationId==SharedSecLabelRelationId||
5992+
relationId==TriggerRelidNameIndexId||
5993+
relationId==DatabaseNameIndexId||
5994+
relationId==SharedSecLabelObjectIndexId)
59265995
{
5927-
/* If this Assert fails, we don't need this special case anymore. */
5996+
/*
5997+
* If this Assert fails, we don't need the applicable special case
5998+
* anymore.
5999+
*/
59286000
Assert(!RelationSupportsSysCache(relationId));
59296001
return true;
59306002
}
@@ -5994,38 +6066,30 @@ RelationHasUnloggedIndex(Relation rel)
59946066
* We take the lock and do the unlink in RelationCacheInitFilePreInvalidate,
59956067
* then release the lock in RelationCacheInitFilePostInvalidate. Caller must
59966068
* send any pending SI messages between those calls.
5997-
*
5998-
* Notice this deals only with the local init file, not the shared init file.
5999-
* The reason is that there can never be a "significant" change to the
6000-
* relcache entry of a shared relation; the most that could happen is
6001-
* updates of noncritical fields such as relpages/reltuples. So, while
6002-
* it's worth updating the shared init file from time to time, it can never
6003-
* be invalid enough to make it necessary to remove it.
60046069
*/
60056070
void
60066071
RelationCacheInitFilePreInvalidate(void)
60076072
{
6008-
charinitfilename[MAXPGPATH];
6073+
charlocalinitfname[MAXPGPATH];
6074+
charsharedinitfname[MAXPGPATH];
60096075

6010-
snprintf(initfilename,sizeof(initfilename),"%s/%s",
6011-
DatabasePath,RELCACHE_INIT_FILENAME);
6076+
if (DatabasePath)
6077+
snprintf(localinitfname,sizeof(localinitfname),"%s/%s",
6078+
DatabasePath,RELCACHE_INIT_FILENAME);
6079+
snprintf(sharedinitfname,sizeof(sharedinitfname),"global/%s",
6080+
RELCACHE_INIT_FILENAME);
60126081

60136082
LWLockAcquire(RelCacheInitLock,LW_EXCLUSIVE);
60146083

6015-
if (unlink(initfilename)<0)
6016-
{
6017-
/*
6018-
* The file might not be there if no backend has been started since
6019-
* the last removal. But complain about failures other than ENOENT.
6020-
* Fortunately, it's not too late to abort the transaction if we can't
6021-
* get rid of the would-be-obsolete init file.
6022-
*/
6023-
if (errno!=ENOENT)
6024-
ereport(ERROR,
6025-
(errcode_for_file_access(),
6026-
errmsg("could not remove cache file \"%s\": %m",
6027-
initfilename)));
6028-
}
6084+
/*
6085+
* The files might not be there if no backend has been started since the
6086+
* last removal. But complain about failures other than ENOENT with
6087+
* ERROR. Fortunately, it's not too late to abort the transaction if we
6088+
* can't get rid of the would-be-obsolete init file.
6089+
*/
6090+
if (DatabasePath)
6091+
unlink_initfile(localinitfname,ERROR);
6092+
unlink_initfile(sharedinitfname,ERROR);
60296093
}
60306094

60316095
void
@@ -6051,13 +6115,9 @@ RelationCacheInitFileRemove(void)
60516115
structdirent*de;
60526116
charpath[MAXPGPATH+10+sizeof(TABLESPACE_VERSION_DIRECTORY)];
60536117

6054-
/*
6055-
* We zap the shared cache file too. In theory it can't get out of sync
6056-
* enough to be a problem, but in data-corruption cases, who knows ...
6057-
*/
60586118
snprintf(path,sizeof(path),"global/%s",
60596119
RELCACHE_INIT_FILENAME);
6060-
unlink_initfile(path);
6120+
unlink_initfile(path,LOG);
60616121

60626122
/* Scan everything in the default tablespace */
60636123
RelationCacheInitFileRemoveInDir("base");
@@ -6097,20 +6157,23 @@ RelationCacheInitFileRemoveInDir(const char *tblspcpath)
60976157
/* Try to remove the init file in each database */
60986158
snprintf(initfilename,sizeof(initfilename),"%s/%s/%s",
60996159
tblspcpath,de->d_name,RELCACHE_INIT_FILENAME);
6100-
unlink_initfile(initfilename);
6160+
unlink_initfile(initfilename,LOG);
61016161
}
61026162
}
61036163

61046164
FreeDir(dir);
61056165
}
61066166

61076167
staticvoid
6108-
unlink_initfile(constchar*initfilename)
6168+
unlink_initfile(constchar*initfilename,intelevel)
61096169
{
61106170
if (unlink(initfilename)<0)
61116171
{
61126172
/* It might not be there, but log any error other than ENOENT */
61136173
if (errno!=ENOENT)
6114-
elog(LOG,"could not remove cache file \"%s\": %m",initfilename);
6174+
ereport(elevel,
6175+
(errcode_for_file_access(),
6176+
errmsg("could not remove cache file \"%s\": %m",
6177+
initfilename)));
61156178
}
61166179
}

‎src/include/storage/standbydefs.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ typedef struct xl_invalidations
6464
{
6565
OiddbId;/* MyDatabaseId */
6666
OidtsId;/* MyDatabaseTableSpace */
67-
boolrelcacheInitFileInval;/* invalidate relcache initfile */
67+
boolrelcacheInitFileInval;/* invalidate relcache initfiles */
6868
intnmsgs;/* number of shared inval msgs */
6969
SharedInvalidationMessagemsgs[FLEXIBLE_ARRAY_MEMBER];
7070
}xl_invalidations;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp