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

Commite2c8100

Browse files
committed
Fix race condition in predicate-lock init code in EXEC_BACKEND builds.
Trading a little too heavily on letting the code path be the same whetherwe were creating shared data structures or only attaching to them,InitPredicateLocks() inserted the "scratch" PredicateLockTargetHash entryunconditionally. This is just wrong if we're in a postmaster child,which would only reach this code in EXEC_BACKEND builds. Most of thetime, the hash_search(HASH_ENTER) call would simply report that theentry already existed, causing no visible effect since the code did notbother to check for that possibility. However, if this happened whilesome other backend had transiently removed the "scratch" entry, thenthat other backend's eventual RestoreScratchTarget would suffer anassert failure; this appears to be the explanation for a recent failureon buildfarm member culicidae. In non-assert builds, there would beno visible consequences there either. But nonetheless this is a prettybad bug for EXEC_BACKEND builds, for two reasons:1. Each new backend would perform the hash_search(HASH_ENTER) callwithout holding any lock that would prevent concurrent access to thePredicateLockTargetHash hash table. This creates a low but certainlynonzero risk of corruption of that hash table.2. In the event that the race condition occurred, by reinserting thescratch entry too soon, we were defeating the entire purpose of thescratch entry, namely to guarantee that transaction commit could movehash table entries around with no risk of out-of-memory failure.The odds of an actual OOM failure are quite low, but not zero, and ifit did happen it would again result in corruption of the hash table.The user-visible symptoms of such corruption are a little hard to predict,but would presumably amount to misbehavior of SERIALIZABLE transactionsthat'd require a crash or postmaster restart to fix.To fix, just skip the hash insertion if IsUnderPostmaster. I alsoinserted a bunch of assertions that the expected things happendepending on whether IsUnderPostmaster is true. That might be overkill,since most comparable code in other functions isn't quite that paranoid,but once burnt twice shy.In passing, also move a couple of lines to places where they seemedto make more sense.Diagnosis of problem by Thomas Munro, patch by me. Back-patch toall supported branches.Discussion:https://postgr.es/m/10593.1500670709@sss.pgh.pa.us
1 parent7086be6 commite2c8100

File tree

1 file changed

+21
-8
lines changed

1 file changed

+21
-8
lines changed

‎src/backend/storage/lmgr/predicate.c

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -815,6 +815,7 @@ OldSerXidInit(void)
815815
oldSerXidControl= (OldSerXidControl)
816816
ShmemInitStruct("OldSerXidControlData",sizeof(OldSerXidControlData),&found);
817817

818+
Assert(found==IsUnderPostmaster);
818819
if (!found)
819820
{
820821
/*
@@ -1109,6 +1110,10 @@ InitPredicateLocks(void)
11091110
SizerequestSize;
11101111
boolfound;
11111112

1113+
#ifndefEXEC_BACKEND
1114+
Assert(!IsUnderPostmaster);
1115+
#endif
1116+
11121117
/*
11131118
* Compute size of predicate lock target hashtable. Note these
11141119
* calculations must agree with PredicateLockShmemSize!
@@ -1131,16 +1136,22 @@ InitPredicateLocks(void)
11311136
HASH_ELEM |HASH_BLOBS |
11321137
HASH_PARTITION |HASH_FIXED_SIZE);
11331138

1134-
/* Assume an average of 2 xacts per target */
1135-
max_table_size *=2;
1136-
11371139
/*
11381140
* Reserve a dummy entry in the hash table; we use it to make sure there's
11391141
* always one entry available when we need to split or combine a page,
11401142
* because running out of space there could mean aborting a
11411143
* non-serializable transaction.
11421144
*/
1143-
hash_search(PredicateLockTargetHash,&ScratchTargetTag,HASH_ENTER,NULL);
1145+
if (!IsUnderPostmaster)
1146+
{
1147+
(void)hash_search(PredicateLockTargetHash,&ScratchTargetTag,
1148+
HASH_ENTER,&found);
1149+
Assert(!found);
1150+
}
1151+
1152+
/* Pre-calculate the hash and partition lock of the scratch entry */
1153+
ScratchTargetTagHash=PredicateLockTargetTagHashCode(&ScratchTargetTag);
1154+
ScratchPartitionLock=PredicateLockHashPartitionLock(ScratchTargetTagHash);
11441155

11451156
/*
11461157
* Allocate hash table for PREDICATELOCK structs. This stores per
@@ -1152,6 +1163,9 @@ InitPredicateLocks(void)
11521163
info.hash=predicatelock_hash;
11531164
info.num_partitions=NUM_PREDICATELOCK_PARTITIONS;
11541165

1166+
/* Assume an average of 2 xacts per target */
1167+
max_table_size *=2;
1168+
11551169
PredicateLockHash=ShmemInitHash("PREDICATELOCK hash",
11561170
max_table_size,
11571171
max_table_size,
@@ -1178,6 +1192,7 @@ InitPredicateLocks(void)
11781192
PredXact=ShmemInitStruct("PredXactList",
11791193
PredXactListDataSize,
11801194
&found);
1195+
Assert(found==IsUnderPostmaster);
11811196
if (!found)
11821197
{
11831198
inti;
@@ -1250,6 +1265,7 @@ InitPredicateLocks(void)
12501265
RWConflictPool=ShmemInitStruct("RWConflictPool",
12511266
RWConflictPoolHeaderDataSize,
12521267
&found);
1268+
Assert(found==IsUnderPostmaster);
12531269
if (!found)
12541270
{
12551271
inti;
@@ -1275,6 +1291,7 @@ InitPredicateLocks(void)
12751291
ShmemInitStruct("FinishedSerializableTransactions",
12761292
sizeof(SHM_QUEUE),
12771293
&found);
1294+
Assert(found==IsUnderPostmaster);
12781295
if (!found)
12791296
SHMQueueInit(FinishedSerializableTransactions);
12801297

@@ -1283,10 +1300,6 @@ InitPredicateLocks(void)
12831300
* transactions.
12841301
*/
12851302
OldSerXidInit();
1286-
1287-
/* Pre-calculate the hash and partition lock of the scratch entry */
1288-
ScratchTargetTagHash=PredicateLockTargetTagHashCode(&ScratchTargetTag);
1289-
ScratchPartitionLock=PredicateLockHashPartitionLock(ScratchTargetTagHash);
12901303
}
12911304

12921305
/*

0 commit comments

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