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

Commitf912d7d

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 parent2bf372a commitf912d7d

File tree

2 files changed

+60
-75
lines changed

2 files changed

+60
-75
lines changed

‎src/backend/catalog/index.c

Lines changed: 3 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -3330,20 +3330,15 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
33303330
indexInfo->ii_ExclusionStrats=NULL;
33313331
}
33323332

3333-
/*
3334-
* Build a new physical relation for the index. Need to do that before
3335-
* "officially" starting the reindexing with SetReindexProcessing -
3336-
* otherwise the necessary pg_class changes cannot be made with
3337-
* encountering assertions.
3338-
*/
3339-
RelationSetNewRelfilenode(iRel,persistence);
3340-
33413333
/* ensure SetReindexProcessing state isn't leaked */
33423334
PG_TRY();
33433335
{
33443336
/* Suppress use of the target index while rebuilding it */
33453337
SetReindexProcessing(heapId,indexId);
33463338

3339+
/* Create a new physical relation for the index */
3340+
RelationSetNewRelfilenode(iRel,persistence);
3341+
33473342
/* Initialize the index and rebuild */
33483343
/* Note: we do not need to re-establish pkey setting */
33493344
index_build(heapRelation,iRel,indexInfo, true, true);
@@ -3491,7 +3486,6 @@ reindex_relation(Oid relid, int flags, int options)
34913486
Relationrel;
34923487
Oidtoast_relid;
34933488
List*indexIds;
3494-
boolis_pg_class;
34953489
boolresult;
34963490
inti;
34973491

@@ -3527,32 +3521,8 @@ reindex_relation(Oid relid, int flags, int options)
35273521
*/
35283522
indexIds=RelationGetIndexList(rel);
35293523

3530-
/*
3531-
* reindex_index will attempt to update the pg_class rows for the relation
3532-
* and index. If we are processing pg_class itself, we want to make sure
3533-
* that the updates do not try to insert index entries into indexes we
3534-
* have not processed yet. (When we are trying to recover from corrupted
3535-
* indexes, that could easily cause a crash.) We can accomplish this
3536-
* because CatalogTupleInsert/CatalogTupleUpdate will use the relcache's
3537-
* index list to know which indexes to update. We just force the index
3538-
* list to be only the stuff we've processed.
3539-
*
3540-
* It is okay to not insert entries into the indexes we have not processed
3541-
* yet because all of this is transaction-safe. If we fail partway
3542-
* through, the updated rows are dead and it doesn't matter whether they
3543-
* have index entries. Also, a new pg_class index will be created with a
3544-
* correct entry for its own pg_class row because we do
3545-
* RelationSetNewRelfilenode() before we do index_build().
3546-
*/
3547-
is_pg_class= (RelationGetRelid(rel)==RelationRelationId);
3548-
3549-
/* Ensure rd_indexattr is valid; see comments for RelationSetIndexList */
3550-
if (is_pg_class)
3551-
(void)RelationGetIndexAttrBitmap(rel,INDEX_ATTR_BITMAP_ALL);
3552-
35533524
PG_TRY();
35543525
{
3555-
List*doneIndexes;
35563526
ListCell*indexId;
35573527
charpersistence;
35583528

@@ -3580,15 +3550,11 @@ reindex_relation(Oid relid, int flags, int options)
35803550
persistence=rel->rd_rel->relpersistence;
35813551

35823552
/* Reindex all the indexes. */
3583-
doneIndexes=NIL;
35843553
i=1;
35853554
foreach(indexId,indexIds)
35863555
{
35873556
OidindexOid=lfirst_oid(indexId);
35883557

3589-
if (is_pg_class)
3590-
RelationSetIndexList(rel,doneIndexes);
3591-
35923558
reindex_index(indexOid, !(flags&REINDEX_REL_CHECK_CONSTRAINTS),
35933559
persistence,options);
35943560

@@ -3597,9 +3563,6 @@ reindex_relation(Oid relid, int flags, int options)
35973563
/* Index should no longer be in the pending list */
35983564
Assert(!ReindexIsProcessingIndex(indexOid));
35993565

3600-
if (is_pg_class)
3601-
doneIndexes=lappend_oid(doneIndexes,indexOid);
3602-
36033566
/* Set index rebuild count */
36043567
pgstat_progress_update_param(PROGRESS_CLUSTER_INDEX_REBUILD_COUNT,
36053568
i);
@@ -3615,9 +3578,6 @@ reindex_relation(Oid relid, int flags, int options)
36153578
PG_END_TRY();
36163579
ResetReindexPending();
36173580

3618-
if (is_pg_class)
3619-
RelationSetIndexList(rel,indexIds);
3620-
36213581
/*
36223582
* Close rel, but continue to hold the lock.
36233583
*/

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

Lines changed: 57 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -3421,7 +3421,8 @@ RelationBuildLocalRelation(const char *relname,
34213421
/*
34223422
* RelationSetNewRelfilenode
34233423
*
3424-
* Assign a new relfilenode (physical file name) to the relation.
3424+
* Assign a new relfilenode (physical file name), and possibly a new
3425+
* persistence setting, to the relation.
34253426
*
34263427
* This allows a full rewrite of the relation to be done with transactional
34273428
* safety (since the filenode assignment can be rolled back). Note however
@@ -3463,13 +3464,10 @@ RelationSetNewRelfilenode(Relation relation, char persistence)
34633464
*/
34643465
RelationDropStorage(relation);
34653466

3466-
/* initialize new relfilenode from old relfilenode */
3467-
newrnode=relation->rd_node;
3468-
34693467
/*
3470-
* Create storage for the main fork of the new relfilenode. If it's
3471-
* table-like object, call into table AM to do so, which'll also create
3472-
* the table's init fork.
3468+
* Create storage for the main fork of the new relfilenode.If it's a
3469+
* table-like object, call intothetable AM to do so, which'll also
3470+
*createthe table's init fork if needed.
34733471
*
34743472
* NOTE: If relevant for the AM, any conflict in relfilenode value will be
34753473
* caught here, if GetNewRelFileNode messes up for any reason.
@@ -3479,18 +3477,10 @@ RelationSetNewRelfilenode(Relation relation, char persistence)
34793477

34803478
switch (relation->rd_rel->relkind)
34813479
{
3482-
/* shouldn't be called for these */
3483-
caseRELKIND_VIEW:
3484-
caseRELKIND_COMPOSITE_TYPE:
3485-
caseRELKIND_FOREIGN_TABLE:
3486-
caseRELKIND_PARTITIONED_TABLE:
3487-
caseRELKIND_PARTITIONED_INDEX:
3488-
elog(ERROR,"should not have storage");
3489-
break;
3490-
34913480
caseRELKIND_INDEX:
34923481
caseRELKIND_SEQUENCE:
34933482
{
3483+
/* handle these directly, at least for now */
34943484
SMgrRelationsrel;
34953485

34963486
srel=RelationCreateStorage(newrnode,persistence);
@@ -3505,41 +3495,76 @@ RelationSetNewRelfilenode(Relation relation, char persistence)
35053495
persistence,
35063496
&freezeXid,&minmulti);
35073497
break;
3498+
3499+
default:
3500+
/* we shouldn't be called for anything else */
3501+
elog(ERROR,"relation \"%s\" does not have storage",
3502+
RelationGetRelationName(relation));
3503+
break;
35083504
}
35093505

35103506
/*
3511-
* However, if we're dealing with a mapped index, pg_class.relfilenode
3512-
* doesn't change; instead we have to send the update to the relation
3513-
* mapper.
3507+
* If we're dealing with a mapped index, pg_class.relfilenode doesn't
3508+
* change; instead we have to send the update to the relation mapper.
3509+
*
3510+
* For mapped indexes, we don't actually change the pg_class entry at all;
3511+
* this is essential when reindexing pg_class itself. That leaves us with
3512+
* possibly-inaccurate values of relpages etc, but those will be fixed up
3513+
* later.
35143514
*/
35153515
if (RelationIsMapped(relation))
3516+
{
3517+
/* This case is only supported for indexes */
3518+
Assert(relation->rd_rel->relkind==RELKIND_INDEX);
3519+
3520+
/* Since we're not updating pg_class, these had better not change */
3521+
Assert(classform->relfrozenxid==freezeXid);
3522+
Assert(classform->relminmxid==minmulti);
3523+
Assert(classform->relpersistence==persistence);
3524+
3525+
/*
3526+
* In some code paths it's possible that the tuple update we'd
3527+
* otherwise do here is the only thing that would assign an XID for
3528+
* the current transaction. However, we must have an XID to delete
3529+
* files, so make sure one is assigned.
3530+
*/
3531+
(void)GetCurrentTransactionId();
3532+
3533+
/* Do the deed */
35163534
RelationMapUpdateMap(RelationGetRelid(relation),
35173535
newrelfilenode,
35183536
relation->rd_rel->relisshared,
35193537
false);
3538+
3539+
/* Since we're not updating pg_class, must trigger inval manually */
3540+
CacheInvalidateRelcache(relation);
3541+
}
35203542
else
3543+
{
3544+
/* Normal case, update the pg_class entry */
35213545
classform->relfilenode=newrelfilenode;
35223546

3523-
/*These changes are safe evenfora mapped relation */
3524-
if (relation->rd_rel->relkind!=RELKIND_SEQUENCE)
3525-
{
3526-
classform->relpages=0;/* it's empty until further notice */
3527-
classform->reltuples=0;
3528-
classform->relallvisible=0;
3529-
}
3530-
classform->relfrozenxid=freezeXid;
3531-
classform->relminmxid=minmulti;
3532-
classform->relpersistence=persistence;
3547+
/*relpages etc. never changeforsequences */
3548+
if (relation->rd_rel->relkind!=RELKIND_SEQUENCE)
3549+
{
3550+
classform->relpages=0;/* it's empty until further notice */
3551+
classform->reltuples=0;
3552+
classform->relallvisible=0;
3553+
}
3554+
classform->relfrozenxid=freezeXid;
3555+
classform->relminmxid=minmulti;
3556+
classform->relpersistence=persistence;
35333557

3534-
CatalogTupleUpdate(pg_class,&tuple->t_self,tuple);
3558+
CatalogTupleUpdate(pg_class,&tuple->t_self,tuple);
3559+
}
35353560

35363561
heap_freetuple(tuple);
35373562

35383563
table_close(pg_class,RowExclusiveLock);
35393564

35403565
/*
3541-
* Make the pg_class row changevisible, as well as the relation map
3542-
*change if any. This willcause the relcache entry to get updated, too.
3566+
* Make the pg_class row changeor relation map change visible. This will
3567+
* cause the relcache entry to get updated, too.
35433568
*/
35443569
CommandCounterIncrement();
35453570

0 commit comments

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