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

Commit2aaec65

Browse files
committed
Avoid returning stale attribute bitmaps in RelationGetIndexAttrBitmap().
The problem with the original coding here is that we might receive (andclear) a relcache invalidation signal for the target relation down insideone of the index_open calls we're doing. Since the target is open, wewould not drop the relcache entry, just reset its rd_indexvalid andrd_indexlist fields. But RelationGetIndexAttrBitmap() kept going, andwould eventually cache and return potentially-obsolete attribute bitmaps.The case where this matters is where the inval signal was from a CREATEINDEX CONCURRENTLY telling us about a new index on a formerly-unindexedcolumn. (In all other cases, the lock we hold on the target rel shouldprevent any concurrent change in index state.) Even just returning thestale attribute bitmap is not such a problem, because it shouldn't matterduring the transaction in which we receive the signal. What hurts iscaching the stale data, because it can survive into later transactions,breaking CREATE INDEX CONCURRENTLY's expectation that later transactionswill not create new broken HOT chains. The upshot is that there's a windowfor building corrupted indexes during CREATE INDEX CONCURRENTLY.This patch fixes the problem by rechecking that the set of index OIDsis still the same at the end of RelationGetIndexAttrBitmap() as it wasat the start. If not, we loop back and try again. That's a littlemore than is strictly necessary to fix the bug --- in principle, wecould return the stale data but not cache it --- but it seems like abad idea on general principles for relcache to return data it knowsis stale.There might be more hazards of the same ilk, or there might be a betterway to fix this one, but this patch definitely improves matters and seemsunlikely to make anything worse. So let's push it into today's releaseseven as we continue to study the problem.Pavan Deolasee and myselfDiscussion:https://postgr.es/m/CABOikdM2MUq9cyZJi1KyLmmkCereyGp5JQ4fuwKoyKEde_mzkQ@mail.gmail.com
1 parent549f747 commit2aaec65

File tree

1 file changed

+36
-7
lines changed

1 file changed

+36
-7
lines changed

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

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4747,8 +4747,10 @@ RelationGetIndexPredicate(Relation relation)
47474747
* we can include system attributes (e.g., OID) in the bitmap representation.
47484748
*
47494749
* Caller had better hold at least RowExclusiveLock on the target relation
4750-
* to ensure that it has a stable set of indexes. This also makes it safe
4751-
* (deadlock-free) for us to take locks on the relation's indexes.
4750+
* to ensure it is safe (deadlock-free) for us to take locks on the relation's
4751+
* indexes. Note that since the introduction of CREATE INDEX CONCURRENTLY,
4752+
* that lock level doesn't guarantee a stable set of indexes, so we have to
4753+
* be prepared to retry here in case of a change in the set of indexes.
47524754
*
47534755
* The returned result is palloc'd in the caller's memory context and should
47544756
* be bms_free'd when not needed anymore.
@@ -4761,6 +4763,7 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
47614763
Bitmapset*pkindexattrs;/* columns in the primary index */
47624764
Bitmapset*idindexattrs;/* columns in the replica identity */
47634765
List*indexoidlist;
4766+
List*newindexoidlist;
47644767
Oidrelpkindex;
47654768
Oidrelreplindex;
47664769
ListCell*l;
@@ -4789,8 +4792,9 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
47894792
returnNULL;
47904793

47914794
/*
4792-
* Get cached list of index OIDs
4795+
* Get cached list of index OIDs. If we have to start over, we do so here.
47934796
*/
4797+
restart:
47944798
indexoidlist=RelationGetIndexList(relation);
47954799

47964800
/* Fall out if no indexes (but relhasindex was set) */
@@ -4801,9 +4805,8 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
48014805
* Copy the rd_pkindex and rd_replidindex values computed by
48024806
* RelationGetIndexList before proceeding. This is needed because a
48034807
* relcache flush could occur inside index_open below, resetting the
4804-
* fields managed by RelationGetIndexList. (The values we're computing
4805-
* will still be valid, assuming that caller has a sufficient lock on
4806-
* the relation.)
4808+
* fields managed by RelationGetIndexList. We need to do the work with
4809+
* stable values of these fields.
48074810
*/
48084811
relpkindex=relation->rd_pkindex;
48094812
relreplindex=relation->rd_replidindex;
@@ -4881,7 +4884,33 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
48814884
index_close(indexDesc,AccessShareLock);
48824885
}
48834886

4884-
list_free(indexoidlist);
4887+
/*
4888+
* During one of the index_opens in the above loop, we might have received
4889+
* a relcache flush event on this relcache entry, which might have been
4890+
* signaling a change in the rel's index list. If so, we'd better start
4891+
* over to ensure we deliver up-to-date attribute bitmaps.
4892+
*/
4893+
newindexoidlist=RelationGetIndexList(relation);
4894+
if (equal(indexoidlist,newindexoidlist)&&
4895+
relpkindex==relation->rd_pkindex&&
4896+
relreplindex==relation->rd_replidindex)
4897+
{
4898+
/* Still the same index set, so proceed */
4899+
list_free(newindexoidlist);
4900+
list_free(indexoidlist);
4901+
}
4902+
else
4903+
{
4904+
/* Gotta do it over ... might as well not leak memory */
4905+
list_free(newindexoidlist);
4906+
list_free(indexoidlist);
4907+
bms_free(uindexattrs);
4908+
bms_free(pkindexattrs);
4909+
bms_free(idindexattrs);
4910+
bms_free(indexattrs);
4911+
4912+
gotorestart;
4913+
}
48854914

48864915
/* Don't leak the old values of these bitmaps, if any */
48874916
bms_free(relation->rd_indexattr);

0 commit comments

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