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

Commit727c155

Browse files
committed
Fix reindexing of pg_class indexes some more.
Commits3dbb317 et al failed under CLOBBER_CACHE_ALWAYS testing.Investigation showed that to reindex pg_class_oid_index, we mustsuppress accesses to the index (via SetReindexProcessing) before we callRelationSetNewRelfilenode, or at least before we do CommandCounterIncrementtherein; otherwise, relcache reloads happening within the CCI may try tofetch pg_class rows using the index's new relfilenode value, which is asyet an empty file.Of course, the point of3dbb317 was that that ordering didn't workeither, because then RelationSetNewRelfilenode's own update of the index'spg_class row cannot access the index, should it need to.There are various ways we might have got around that, but Andres Freundcame up with a brilliant solution: for a mapped index, we can really justskip the pg_class update altogether. The only fields it was actuallychanging were relpages etc, but it was just setting them to zeroes whichis useless make-work. (Correct new values will be installed at the endof index build.) All pg_class indexes are mapped and probably always willbe, so this eliminates the problem by removing work rather than adding it,always a pleasant outcome. Having taught RelationSetNewRelfilenode to doit that way, we can revert the code reordering in reindex_index. (ButI left the moved setup code where it was; there seems no reason why ithas to run without use of the old index. If you're trying to fix abusted pg_class index, you'll have had to disable system index usealtogether to get this far.)Moreover, this means we don't need RelationSetIndexList at all, becausereindex_relation's hacking to make "REINDEX TABLE pg_class" work islikewise now unnecessary. We'll leave that code in place in the backbranches, but a follow-on patch will remove it in HEAD.In passing, do some minor cleanup for commit5c15606 (in HEAD only),notably removing a duplicate newrnode assignment.Patch by me, using a core idea due to Andres Freund. Back-patch to allsupported branches, as3dbb317 was.Discussion:https://postgr.es/m/28926.1556664156@sss.pgh.pa.us
1 parent63c6a24 commit727c155

File tree

2 files changed

+49
-65
lines changed

2 files changed

+49
-65
lines changed

‎src/backend/catalog/index.c

Lines changed: 4 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -3713,21 +3713,16 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
37133713
indexInfo->ii_ExclusionStrats=NULL;
37143714
}
37153715

3716-
/*
3717-
* Build a new physical relation for the index. Need to do that before
3718-
* "officially" starting the reindexing with SetReindexProcessing -
3719-
* otherwise the necessary pg_class changes cannot be made with
3720-
* encountering assertions.
3721-
*/
3722-
RelationSetNewRelfilenode(iRel,persistence,InvalidTransactionId,
3723-
InvalidMultiXactId);
3724-
37253716
/* ensure SetReindexProcessing state isn't leaked */
37263717
PG_TRY();
37273718
{
37283719
/* Suppress use of the target index while rebuilding it */
37293720
SetReindexProcessing(heapId,indexId);
37303721

3722+
/* Create a new physical relation for the index */
3723+
RelationSetNewRelfilenode(iRel,persistence,InvalidTransactionId,
3724+
InvalidMultiXactId);
3725+
37313726
/* Initialize the index and rebuild */
37323727
/* Note: we do not need to re-establish pkey setting */
37333728
index_build(heapRelation,iRel,indexInfo, false, true, true);
@@ -3873,7 +3868,6 @@ reindex_relation(Oid relid, int flags, int options)
38733868
Relationrel;
38743869
Oidtoast_relid;
38753870
List*indexIds;
3876-
boolis_pg_class;
38773871
boolresult;
38783872

38793873
/*
@@ -3908,37 +3902,8 @@ reindex_relation(Oid relid, int flags, int options)
39083902
*/
39093903
indexIds=RelationGetIndexList(rel);
39103904

3911-
/*
3912-
* reindex_index will attempt to update the pg_class rows for the relation
3913-
* and index. If we are processing pg_class itself, we want to make sure
3914-
* that the updates do not try to insert index entries into indexes we
3915-
* have not processed yet. (When we are trying to recover from corrupted
3916-
* indexes, that could easily cause a crash.) We can accomplish this
3917-
* because CatalogTupleInsert/CatalogTupleUpdate will use the relcache's
3918-
* index list to know which indexes to update. We just force the index
3919-
* list to be only the stuff we've processed.
3920-
*
3921-
* It is okay to not insert entries into the indexes we have not processed
3922-
* yet because all of this is transaction-safe. If we fail partway
3923-
* through, the updated rows are dead and it doesn't matter whether they
3924-
* have index entries. Also, a new pg_class index will be created with a
3925-
* correct entry for its own pg_class row because we do
3926-
* RelationSetNewRelfilenode() before we do index_build().
3927-
*
3928-
* Note that we also clear pg_class's rd_oidindex until the loop is done,
3929-
* so that that index can't be accessed either. This means we cannot
3930-
* safely generate new relation OIDs while in the loop; shouldn't be a
3931-
* problem.
3932-
*/
3933-
is_pg_class= (RelationGetRelid(rel)==RelationRelationId);
3934-
3935-
/* Ensure rd_indexattr is valid; see comments for RelationSetIndexList */
3936-
if (is_pg_class)
3937-
(void)RelationGetIndexAttrBitmap(rel,INDEX_ATTR_BITMAP_HOT);
3938-
39393905
PG_TRY();
39403906
{
3941-
List*doneIndexes;
39423907
ListCell*indexId;
39433908
charpersistence;
39443909

@@ -3966,24 +3931,17 @@ reindex_relation(Oid relid, int flags, int options)
39663931
persistence=rel->rd_rel->relpersistence;
39673932

39683933
/* Reindex all the indexes. */
3969-
doneIndexes=NIL;
39703934
foreach(indexId,indexIds)
39713935
{
39723936
OidindexOid=lfirst_oid(indexId);
39733937

3974-
if (is_pg_class)
3975-
RelationSetIndexList(rel,doneIndexes,InvalidOid);
3976-
39773938
reindex_index(indexOid, !(flags&REINDEX_REL_CHECK_CONSTRAINTS),
39783939
persistence,options);
39793940

39803941
CommandCounterIncrement();
39813942

39823943
/* Index should no longer be in the pending list */
39833944
Assert(!ReindexIsProcessingIndex(indexOid));
3984-
3985-
if (is_pg_class)
3986-
doneIndexes=lappend_oid(doneIndexes,indexOid);
39873945
}
39883946
}
39893947
PG_CATCH();
@@ -3995,9 +3953,6 @@ reindex_relation(Oid relid, int flags, int options)
39953953
PG_END_TRY();
39963954
ResetReindexPending();
39973955

3998-
if (is_pg_class)
3999-
RelationSetIndexList(rel,indexIds,ClassOidIndexId);
4000-
40013956
/*
40023957
* Close rel, but continue to hold the lock.
40033958
*/

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

Lines changed: 45 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3384,38 +3384,67 @@ RelationSetNewRelfilenode(Relation relation, char persistence,
33843384
RelationDropStorage(relation);
33853385

33863386
/*
3387-
* Now update the pg_class row. However, if we're dealing with a mapped
3388-
* index, pg_class.relfilenode doesn't change; instead we have to send the
3389-
* update to the relation mapper.
3387+
* If we're dealing with a mapped index, pg_class.relfilenode doesn't
3388+
* change; instead we have to send the update to the relation mapper.
3389+
*
3390+
* For mapped indexes, we don't actually change the pg_class entry at all;
3391+
* this is essential when reindexing pg_class itself. That leaves us with
3392+
* possibly-inaccurate values of relpages etc, but those will be fixed up
3393+
* later.
33903394
*/
33913395
if (RelationIsMapped(relation))
3396+
{
3397+
/* This case is only supported for indexes */
3398+
Assert(relation->rd_rel->relkind==RELKIND_INDEX);
3399+
3400+
/* Since we're not updating pg_class, these had better not change */
3401+
Assert(classform->relfrozenxid==freezeXid);
3402+
Assert(classform->relminmxid==minmulti);
3403+
Assert(classform->relpersistence==persistence);
3404+
3405+
/*
3406+
* In some code paths it's possible that the tuple update we'd
3407+
* otherwise do here is the only thing that would assign an XID for
3408+
* the current transaction. However, we must have an XID to delete
3409+
* files, so make sure one is assigned.
3410+
*/
3411+
(void)GetCurrentTransactionId();
3412+
3413+
/* Do the deed */
33923414
RelationMapUpdateMap(RelationGetRelid(relation),
33933415
newrelfilenode,
33943416
relation->rd_rel->relisshared,
33953417
false);
3418+
3419+
/* Since we're not updating pg_class, must trigger inval manually */
3420+
CacheInvalidateRelcache(relation);
3421+
}
33963422
else
3423+
{
3424+
/* Normal case, update the pg_class entry */
33973425
classform->relfilenode=newrelfilenode;
33983426

3399-
/*These changes are safe evenfora mapped relation */
3400-
if (relation->rd_rel->relkind!=RELKIND_SEQUENCE)
3401-
{
3402-
classform->relpages=0;/* it's empty until further notice */
3403-
classform->reltuples=0;
3404-
classform->relallvisible=0;
3405-
}
3406-
classform->relfrozenxid=freezeXid;
3407-
classform->relminmxid=minmulti;
3408-
classform->relpersistence=persistence;
3427+
/*relpages etc. never changeforsequences */
3428+
if (relation->rd_rel->relkind!=RELKIND_SEQUENCE)
3429+
{
3430+
classform->relpages=0;/* it's empty until further notice */
3431+
classform->reltuples=0;
3432+
classform->relallvisible=0;
3433+
}
3434+
classform->relfrozenxid=freezeXid;
3435+
classform->relminmxid=minmulti;
3436+
classform->relpersistence=persistence;
34093437

3410-
CatalogTupleUpdate(pg_class,&tuple->t_self,tuple);
3438+
CatalogTupleUpdate(pg_class,&tuple->t_self,tuple);
3439+
}
34113440

34123441
heap_freetuple(tuple);
34133442

34143443
heap_close(pg_class,RowExclusiveLock);
34153444

34163445
/*
3417-
* Make the pg_class row changevisible, as well as the relation map
3418-
*change if any. This willcause the relcache entry to get updated, too.
3446+
* Make the pg_class row changeor relation map change visible. This will
3447+
* cause the relcache entry to get updated, too.
34193448
*/
34203449
CommandCounterIncrement();
34213450

0 commit comments

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