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

Commitb23b0f5

Browse files
committed
Code review for recent changes in relcache.c.
rd_replidindex should be managed the same as rd_oidindex, and rd_keyattrand rd_idattr should be managed like rd_indexattr. Omissions in this areameant that the bitmapsets computed for rd_keyattr and rd_idattr would beleaked during any relcache flush, resulting in a slow but permanent leak inCacheMemoryContext. There was also a tiny probability of relcache entrycorruption if we ran out of memory at just the wrong point inRelationGetIndexAttrBitmap. Otherwise, the fields were not zeroed whereexpected, which would not bother the code any AFAICS but could greatlyconfuse anyone examining the relcache entry while debugging.Also, create an API function RelationGetReplicaIndex rather than lettingnon-relcache code be intimate with the mechanisms underlying caching ofthat value (we won't even mention the memory leak there).Also, fix a relcache flush hazard identified by Andres Freund:RelationGetIndexAttrBitmap must not assume that rd_replidindex stays validacross index_open.The aspects of this involving rd_keyattr date back to 9.3, so back-patchthose changes.
1 parentac53295 commitb23b0f5

File tree

4 files changed

+98
-54
lines changed

4 files changed

+98
-54
lines changed

‎src/backend/access/heap/heapam.c‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7113,6 +7113,7 @@ static HeapTuple
71137113
ExtractReplicaIdentity(Relationrelation,HeapTupletp,boolkey_changed,bool*copy)
71147114
{
71157115
TupleDescdesc=RelationGetDescr(relation);
7116+
Oidreplidindex;
71167117
Relationidx_rel;
71177118
TupleDescidx_desc;
71187119
charreplident=relation->rd_rel->relreplident;
@@ -7147,18 +7148,16 @@ ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_changed, bool *
71477148
if (!key_changed)
71487149
returnNULL;
71497150

7150-
/* needs to already have been fetched? */
7151-
if (relation->rd_indexvalid==0)
7152-
RelationGetIndexList(relation);
7153-
7154-
if (!OidIsValid(relation->rd_replidindex))
7151+
/* find the replica identity index */
7152+
replidindex=RelationGetReplicaIndex(relation);
7153+
if (!OidIsValid(replidindex))
71557154
{
7156-
elog(DEBUG4,"Could not find configured replica identity for table \"%s\"",
7155+
elog(DEBUG4,"could not find configured replica identity for table \"%s\"",
71577156
RelationGetRelationName(relation));
71587157
returnNULL;
71597158
}
71607159

7161-
idx_rel=RelationIdGetRelation(relation->rd_replidindex);
7160+
idx_rel=RelationIdGetRelation(replidindex);
71627161
idx_desc=RelationGetDescr(idx_rel);
71637162

71647163
/* deform tuple, so we have fast access to columns */

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

Lines changed: 82 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1901,6 +1901,8 @@ RelationDestroyRelation(Relation relation, bool remember_tupdesc)
19011901
}
19021902
list_free(relation->rd_indexlist);
19031903
bms_free(relation->rd_indexattr);
1904+
bms_free(relation->rd_keyattr);
1905+
bms_free(relation->rd_idattr);
19041906
FreeTriggerDesc(relation->trigdesc);
19051907
if (relation->rd_options)
19061908
pfree(relation->rd_options);
@@ -2547,6 +2549,7 @@ AtEOXact_cleanup(Relation relation, bool isCommit)
25472549
list_free(relation->rd_indexlist);
25482550
relation->rd_indexlist=NIL;
25492551
relation->rd_oidindex=InvalidOid;
2552+
relation->rd_replidindex=InvalidOid;
25502553
relation->rd_indexvalid=0;
25512554
}
25522555
}
@@ -2645,6 +2648,7 @@ AtEOSubXact_cleanup(Relation relation, bool isCommit,
26452648
list_free(relation->rd_indexlist);
26462649
relation->rd_indexlist=NIL;
26472650
relation->rd_oidindex=InvalidOid;
2651+
relation->rd_replidindex=InvalidOid;
26482652
relation->rd_indexvalid=0;
26492653
}
26502654
}
@@ -3596,6 +3600,10 @@ CheckConstraintFetch(Relation relation)
35963600
* of the index list. rd_oidindex is valid when rd_indexvalid isn't zero;
35973601
* it is the pg_class OID of a unique index on OID when the relation has one,
35983602
* and InvalidOid if there is no such index.
3603+
*
3604+
* In exactly the same way, we update rd_replidindex, which is the pg_class
3605+
* OID of an index to be used as the relation's replication identity index,
3606+
* or InvalidOid if there is no such index.
35993607
*/
36003608
List*
36013609
RelationGetIndexList(Relationrelation)
@@ -3667,8 +3675,8 @@ RelationGetIndexList(Relation relation)
36673675

36683676
/*
36693677
* Invalid, non-unique, non-immediate or predicate indexes aren't
3670-
* interesting forneither oid indexesnor replication identity
3671-
*indexes,so don't check them.
3678+
* interesting foreither oid indexesor replication identity indexes,
3679+
* so don't check them.
36723680
*/
36733681
if (!IndexIsValid(index)|| !index->indisunique||
36743682
!index->indimmediate||
@@ -3681,35 +3689,29 @@ RelationGetIndexList(Relation relation)
36813689
indclass->values[0]==OID_BTREE_OPS_OID)
36823690
oidIndex=index->indexrelid;
36833691

3684-
/*always preferprimarykeys */
3692+
/*rememberprimarykey index if any */
36853693
if (index->indisprimary)
36863694
pkeyIndex=index->indexrelid;
36873695

3688-
/* explicitly chosen index */
3696+
/*rememberexplicitly chosen replica index */
36893697
if (index->indisreplident)
36903698
candidateIndex=index->indexrelid;
36913699
}
36923700

36933701
systable_endscan(indscan);
36943702

3695-
/* primary key */
3696-
if (replident==REPLICA_IDENTITY_DEFAULT&&
3697-
OidIsValid(pkeyIndex))
3698-
relation->rd_replidindex=pkeyIndex;
3699-
/* explicitly chosen index */
3700-
elseif (replident==REPLICA_IDENTITY_INDEX&&
3701-
OidIsValid(candidateIndex))
3702-
relation->rd_replidindex=candidateIndex;
3703-
/* nothing */
3704-
else
3705-
relation->rd_replidindex=InvalidOid;
3706-
37073703
heap_close(indrel,AccessShareLock);
37083704

37093705
/* Now save a copy of the completed list in the relcache entry. */
37103706
oldcxt=MemoryContextSwitchTo(CacheMemoryContext);
37113707
relation->rd_indexlist=list_copy(result);
37123708
relation->rd_oidindex=oidIndex;
3709+
if (replident==REPLICA_IDENTITY_DEFAULT&&OidIsValid(pkeyIndex))
3710+
relation->rd_replidindex=pkeyIndex;
3711+
elseif (replident==REPLICA_IDENTITY_INDEX&&OidIsValid(candidateIndex))
3712+
relation->rd_replidindex=candidateIndex;
3713+
else
3714+
relation->rd_replidindex=InvalidOid;
37133715
relation->rd_indexvalid=1;
37143716
MemoryContextSwitchTo(oldcxt);
37153717

@@ -3767,7 +3769,8 @@ insert_ordered_oid(List *list, Oid datum)
37673769
* correctly with respect to the full index set. It is up to the caller
37683770
* to ensure that a correct rd_indexattr set has been cached before first
37693771
* calling RelationSetIndexList; else a subsequent inquiry might cause a
3770-
* wrong rd_indexattr set to get computed and cached.
3772+
* wrong rd_indexattr set to get computed and cached. Likewise, we do not
3773+
* touch rd_keyattr or rd_idattr.
37713774
*/
37723775
void
37733776
RelationSetIndexList(Relationrelation,List*indexIds,OidoidIndex)
@@ -3783,6 +3786,8 @@ RelationSetIndexList(Relation relation, List *indexIds, Oid oidIndex)
37833786
list_free(relation->rd_indexlist);
37843787
relation->rd_indexlist=indexIds;
37853788
relation->rd_oidindex=oidIndex;
3789+
/* For the moment, assume the target rel hasn't got a replica index */
3790+
relation->rd_replidindex=InvalidOid;
37863791
relation->rd_indexvalid=2;/* mark list as forced */
37873792
/* Flag relation as needing eoxact cleanup (to reset the list) */
37883793
EOXactListAdd(relation);
@@ -3816,6 +3821,27 @@ RelationGetOidIndex(Relation relation)
38163821
returnrelation->rd_oidindex;
38173822
}
38183823

3824+
/*
3825+
* RelationGetReplicaIndex -- get OID of the relation's replica identity index
3826+
*
3827+
* Returns InvalidOid if there is no such index.
3828+
*/
3829+
Oid
3830+
RelationGetReplicaIndex(Relationrelation)
3831+
{
3832+
List*ilist;
3833+
3834+
if (relation->rd_indexvalid==0)
3835+
{
3836+
/* RelationGetIndexList does the heavy lifting. */
3837+
ilist=RelationGetIndexList(relation);
3838+
list_free(ilist);
3839+
Assert(relation->rd_indexvalid!=0);
3840+
}
3841+
3842+
returnrelation->rd_replidindex;
3843+
}
3844+
38193845
/*
38203846
* RelationGetIndexExpressions -- get the index expressions for an index
38213847
*
@@ -3954,8 +3980,8 @@ RelationGetIndexPredicate(Relation relation)
39543980
* predicates.)
39553981
*
39563982
* Depending on attrKind, a bitmap covering the attnums for all index columns,
3957-
* for all key columns or for allthecolumns the configured replica identity
3958-
*are returned.
3983+
* for allpotential foreignkey columns, or for all columnsinthe configured
3984+
*replica identity index is returned.
39593985
*
39603986
* Attribute numbers are offset by FirstLowInvalidHeapAttributeNumber so that
39613987
* we can include system attributes (e.g., OID) in the bitmap representation.
@@ -3974,22 +4000,25 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
39744000
Bitmapset*uindexattrs;/* columns in unique indexes */
39754001
Bitmapset*idindexattrs;/* columns in the replica identity */
39764002
List*indexoidlist;
4003+
Oidrelreplindex;
39774004
ListCell*l;
39784005
MemoryContextoldcxt;
39794006

39804007
/* Quick exit if we already computed the result. */
39814008
if (relation->rd_indexattr!=NULL)
4009+
{
39824010
switch (attrKind)
39834011
{
3984-
caseINDEX_ATTR_BITMAP_IDENTITY_KEY:
3985-
returnbms_copy(relation->rd_idattr);
3986-
caseINDEX_ATTR_BITMAP_KEY:
3987-
returnbms_copy(relation->rd_keyattr);
39884012
caseINDEX_ATTR_BITMAP_ALL:
39894013
returnbms_copy(relation->rd_indexattr);
4014+
caseINDEX_ATTR_BITMAP_KEY:
4015+
returnbms_copy(relation->rd_keyattr);
4016+
caseINDEX_ATTR_BITMAP_IDENTITY_KEY:
4017+
returnbms_copy(relation->rd_idattr);
39904018
default:
39914019
elog(ERROR,"unknown attrKind %u",attrKind);
39924020
}
4021+
}
39934022

39944023
/* Fast path if definitely no indexes */
39954024
if (!RelationGetForm(relation)->relhasindex)
@@ -4004,6 +4033,15 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
40044033
if (indexoidlist==NIL)
40054034
returnNULL;
40064035

4036+
/*
4037+
* Copy the rd_replidindex value computed by RelationGetIndexList before
4038+
* proceeding. This is needed because a relcache flush could occur inside
4039+
* index_open below, resetting the fields managed by RelationGetIndexList.
4040+
* (The values we're computing will still be valid, assuming that caller
4041+
* has a sufficient lock on the relation.)
4042+
*/
4043+
relreplindex=relation->rd_replidindex;
4044+
40074045
/*
40084046
* For each index, add referenced attributes to indexattrs.
40094047
*
@@ -4026,7 +4064,6 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
40264064
boolisKey;/* candidate key */
40274065
boolisIDKey;/* replica identity index */
40284066

4029-
40304067
indexDesc=index_open(indexOid,AccessShareLock);
40314068

40324069
/* Extract index key information from the index's pg_index row */
@@ -4038,7 +4075,7 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
40384075
indexInfo->ii_Predicate==NIL;
40394076

40404077
/* Is this index the configured (or default) replica identity? */
4041-
isIDKey=indexOid==relation->rd_replidindex;
4078+
isIDKey=(indexOid==relreplindex);
40424079

40434080
/* Collect simple attribute references */
40444081
for (i=0;i<indexInfo->ii_NumIndexAttrs;i++)
@@ -4050,13 +4087,13 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
40504087
indexattrs=bms_add_member(indexattrs,
40514088
attrnum-FirstLowInvalidHeapAttributeNumber);
40524089

4053-
if (isIDKey)
4054-
idindexattrs=bms_add_member(idindexattrs,
4055-
attrnum-FirstLowInvalidHeapAttributeNumber);
4056-
40574090
if (isKey)
40584091
uindexattrs=bms_add_member(uindexattrs,
40594092
attrnum-FirstLowInvalidHeapAttributeNumber);
4093+
4094+
if (isIDKey)
4095+
idindexattrs=bms_add_member(idindexattrs,
4096+
attrnum-FirstLowInvalidHeapAttributeNumber);
40604097
}
40614098
}
40624099

@@ -4071,22 +4108,28 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
40714108

40724109
list_free(indexoidlist);
40734110

4074-
/* Now save a copy of the bitmap in the relcache entry. */
4111+
/*
4112+
* Now save copies of the bitmaps in the relcache entry. We intentionally
4113+
* set rd_indexattr last, because that's the one that signals validity of
4114+
* the values; if we run out of memory before making that copy, we won't
4115+
* leave the relcache entry looking like the other ones are valid but
4116+
* empty.
4117+
*/
40754118
oldcxt=MemoryContextSwitchTo(CacheMemoryContext);
4076-
relation->rd_indexattr=bms_copy(indexattrs);
40774119
relation->rd_keyattr=bms_copy(uindexattrs);
40784120
relation->rd_idattr=bms_copy(idindexattrs);
4121+
relation->rd_indexattr=bms_copy(indexattrs);
40794122
MemoryContextSwitchTo(oldcxt);
40804123

40814124
/* We return our original working copy for caller to play with */
40824125
switch (attrKind)
40834126
{
4084-
caseINDEX_ATTR_BITMAP_IDENTITY_KEY:
4085-
returnidindexattrs;
4086-
caseINDEX_ATTR_BITMAP_KEY:
4087-
returnuindexattrs;
40884127
caseINDEX_ATTR_BITMAP_ALL:
40894128
returnindexattrs;
4129+
caseINDEX_ATTR_BITMAP_KEY:
4130+
returnuindexattrs;
4131+
caseINDEX_ATTR_BITMAP_IDENTITY_KEY:
4132+
returnidindexattrs;
40904133
default:
40914134
elog(ERROR,"unknown attrKind %u",attrKind);
40924135
returnNULL;
@@ -4630,8 +4673,11 @@ load_relcache_init_file(bool shared)
46304673
rel->rd_refcnt=0;
46314674
rel->rd_indexvalid=0;
46324675
rel->rd_indexlist=NIL;
4633-
rel->rd_indexattr=NULL;
46344676
rel->rd_oidindex=InvalidOid;
4677+
rel->rd_replidindex=InvalidOid;
4678+
rel->rd_indexattr=NULL;
4679+
rel->rd_keyattr=NULL;
4680+
rel->rd_idattr=NULL;
46354681
rel->rd_createSubid=InvalidSubTransactionId;
46364682
rel->rd_newRelfilenodeSubid=InvalidSubTransactionId;
46374683
rel->rd_amcache=NULL;

‎src/include/utils/rel.h‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -101,22 +101,20 @@ typedef struct RelationData
101101
Form_pg_classrd_rel;/* RELATION tuple */
102102
TupleDescrd_att;/* tuple descriptor */
103103
Oidrd_id;/* relation's object id */
104-
List*rd_indexlist;/* list of OIDs of indexes on relation */
105-
Bitmapset*rd_indexattr;/* identifies columns used in indexes */
106-
Bitmapset*rd_keyattr;/* cols that can be ref'd by foreign keys */
107-
Bitmapset*rd_idattr;/* included in replica identity index */
108-
Oidrd_oidindex;/* OID of unique index on OID, if any */
109104
LockInfoDatard_lockInfo;/* lock mgr's info for locking relation */
110105
RuleLock*rd_rules;/* rewrite rules */
111106
MemoryContextrd_rulescxt;/* private memory cxt for rd_rules, if any */
112107
TriggerDesc*trigdesc;/* Trigger info, or NULL if rel has none */
113108

114-
/*
115-
* The index chosen as the relation's replication identity or InvalidOid.
116-
* Only set correctly if RelationGetIndexList has been
117-
* called/rd_indexvalid > 0.
118-
*/
119-
Oidrd_replidindex;
109+
/* data managed by RelationGetIndexList: */
110+
List*rd_indexlist;/* list of OIDs of indexes on relation */
111+
Oidrd_oidindex;/* OID of unique index on OID, if any */
112+
Oidrd_replidindex;/* OID of replica identity index, if any */
113+
114+
/* data managed by RelationGetIndexAttrBitmap: */
115+
Bitmapset*rd_indexattr;/* identifies columns used in indexes */
116+
Bitmapset*rd_keyattr;/* cols that can be ref'd by foreign keys */
117+
Bitmapset*rd_idattr;/* included in replica identity index */
120118

121119
/*
122120
* rd_options is set whenever rd_rel is loaded into the relcache entry.

‎src/include/utils/relcache.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ extern void RelationClose(Relation relation);
3939
*/
4040
externList*RelationGetIndexList(Relationrelation);
4141
externOidRelationGetOidIndex(Relationrelation);
42+
externOidRelationGetReplicaIndex(Relationrelation);
4243
externList*RelationGetIndexExpressions(Relationrelation);
4344
externList*RelationGetIndexPredicate(Relationrelation);
4445

0 commit comments

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