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

Commitd4c50b4

Browse files
committed
Prevent memory leaks associated with relcache rd_partcheck structures.
The original coding of generate_partition_qual() just copied the listof predicate expressions into the global CacheMemoryContext, making iteffectively impossible to clean up when the owning relcache entry isdestroyed --- the relevant code in RelationDestroyRelation() only managedto free the topmost List header :-(. This resulted in a session-lifespanmemory leak whenever a table partition's relcache entry is rebuilt.Fortunately, that's not normally a large data structure, and rebuildsshouldn't occur all that often in production situations; but this isstill a bug worth fixing back to v10 where the code was introduced.To fix, put the cached expression tree into its own small memory context,as we do with other complicated substructures of relcache entries.Also, deal more honestly with the case that a partition has an emptypartcheck list; while that probably isn't a case that's very interestingfor production use, it's legal.In passing, clarify comments about how partitioning-related relcachedata structures are managed, and add some Asserts that we're not leakingold copies when we overwrite these data fields.Amit Langote and Tom LaneDiscussion:https://postgr.es/m/7961.1552498252@sss.pgh.pa.us
1 parent6d81e3c commitd4c50b4

File tree

3 files changed

+76
-29
lines changed

3 files changed

+76
-29
lines changed

‎src/backend/catalog/partition.c

Lines changed: 50 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -142,10 +142,13 @@ static int partition_bound_bsearch(PartitionKey key,
142142

143143
/*
144144
* RelationBuildPartitionDesc
145-
*Form rel's partition descriptor
145+
*Form rel's partition descriptor, and store in relcache entry
146146
*
147-
* Not flushed from the cache by RelationClearRelation() unless changed because
148-
* of addition or removal of partition.
147+
* Note: the descriptor won't be flushed from the cache by
148+
* RelationClearRelation() unless it's changed because of
149+
* addition or removal of a partition. Hence, code holding a lock
150+
* that's sufficient to prevent that can assume that rd_partdesc
151+
* won't change underneath it.
149152
*/
150153
void
151154
RelationBuildPartitionDesc(Relationrel)
@@ -422,10 +425,22 @@ RelationBuildPartitionDesc(Relation rel)
422425
(int)key->strategy);
423426
}
424427

425-
/* Now build the actual relcache partition descriptor */
428+
/* Assert we aren't about to leak any old data structure */
429+
Assert(rel->rd_pdcxt==NULL);
430+
Assert(rel->rd_partdesc==NULL);
431+
432+
/*
433+
* Now build the actual relcache partition descriptor. Note that the
434+
* order of operations here is fairly critical. If we fail partway
435+
* through this code, we won't have leaked memory because the rd_pdcxt is
436+
* attached to the relcache entry immediately, so it'll be freed whenever
437+
* the entry is rebuilt or destroyed. However, we don't assign to
438+
* rd_partdesc until the cached data structure is fully complete and
439+
* valid, so that no other code might try to use it.
440+
*/
426441
rel->rd_pdcxt=AllocSetContextCreate(CacheMemoryContext,
427442
RelationGetRelationName(rel),
428-
ALLOCSET_DEFAULT_SIZES);
443+
ALLOCSET_SMALL_SIZES);
429444
oldcxt=MemoryContextSwitchTo(rel->rd_pdcxt);
430445

431446
result= (PartitionDescData*)palloc0(sizeof(PartitionDescData));
@@ -1816,11 +1831,9 @@ get_qual_for_range(PartitionKey key, PartitionBoundSpec *spec)
18161831
*
18171832
* Generate partition predicate from rel's partition bound expression
18181833
*
1819-
* Result expression tree is stored CacheMemoryContext to ensure it survives
1820-
* as long as the relcache entry. But we should be running in a less long-lived
1821-
* working context. To avoid leaking cache memory if this routine fails partway
1822-
* through, we build in working memory and then copy the completed structure
1823-
* into cache memory.
1834+
* We cache a copy of the result in the relcache entry, after constructing
1835+
* it using the caller's context. This approach avoids leaking any data
1836+
* into long-lived cache contexts, especially if we fail partway through.
18241837
*/
18251838
staticList*
18261839
generate_partition_qual(Relationrel)
@@ -1838,8 +1851,8 @@ generate_partition_qual(Relation rel)
18381851
/* Guard against stack overflow due to overly deep partition tree */
18391852
check_stack_depth();
18401853

1841-
/*Quick copy */
1842-
if (rel->rd_partcheck!=NIL)
1854+
/*If we already cached the result, just return a copy */
1855+
if (rel->rd_partcheckvalid)
18431856
returncopyObject(rel->rd_partcheck);
18441857

18451858
/* Grab at least an AccessShareLock on the parent table */
@@ -1882,14 +1895,35 @@ generate_partition_qual(Relation rel)
18821895
if (found_whole_row)
18831896
elog(ERROR,"unexpected whole-row reference found in partition key");
18841897

1885-
/* Save a copy in the relcache */
1886-
oldcxt=MemoryContextSwitchTo(CacheMemoryContext);
1887-
rel->rd_partcheck=copyObject(result);
1888-
MemoryContextSwitchTo(oldcxt);
1898+
/* Assert that we're not leaking any old data during assignments below */
1899+
Assert(rel->rd_partcheckcxt==NULL);
1900+
Assert(rel->rd_partcheck==NIL);
1901+
1902+
/*
1903+
* Save a copy in the relcache. The order of these operations is fairly
1904+
* critical to avoid memory leaks and ensure that we don't leave a corrupt
1905+
* relcache entry if we fail partway through copyObject.
1906+
*
1907+
* If, as is definitely possible, the partcheck list is NIL, then we do
1908+
* not need to make a context to hold it.
1909+
*/
1910+
if (result!=NIL)
1911+
{
1912+
rel->rd_partcheckcxt=AllocSetContextCreate(CacheMemoryContext,
1913+
RelationGetRelationName(rel),
1914+
ALLOCSET_SMALL_SIZES);
1915+
oldcxt=MemoryContextSwitchTo(rel->rd_partcheckcxt);
1916+
rel->rd_partcheck=copyObject(result);
1917+
MemoryContextSwitchTo(oldcxt);
1918+
}
1919+
else
1920+
rel->rd_partcheck=NIL;
1921+
rel->rd_partcheckvalid= true;
18891922

18901923
/* Keep the parent locked until commit */
18911924
relation_close(parent,NoLock);
18921925

1926+
/* Return the working copy to the caller */
18931927
returnresult;
18941928
}
18951929

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

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -808,11 +808,11 @@ RelationBuildRuleLock(Relation relation)
808808

809809
/*
810810
* RelationBuildPartitionKey
811-
*Buildand attach to relcachepartition key data of relation
811+
*Build partition key data of relation, and attach to relcache
812812
*
813813
* Partitioning key data is a complex structure; to avoid complicated logic to
814814
* free individual elements whenever the relcache entry is flushed, we give it
815-
* its own memory context, child of CacheMemoryContext, which can easily be
815+
* its own memory context,achild of CacheMemoryContext, which can easily be
816816
* deleted on its own. To avoid leaking memory in that context in case of an
817817
* error partway through this function, the context is initially created as a
818818
* child of CurTransactionContext and only re-parented to CacheMemoryContext
@@ -907,15 +907,14 @@ RelationBuildPartitionKey(Relation relation)
907907
MemoryContextSwitchTo(oldcxt);
908908
}
909909

910+
/* Allocate assorted arrays in the partkeycxt, which we'll fill below */
910911
oldcxt=MemoryContextSwitchTo(partkeycxt);
911912
key->partattrs= (AttrNumber*)palloc0(key->partnatts*sizeof(AttrNumber));
912913
key->partopfamily= (Oid*)palloc0(key->partnatts*sizeof(Oid));
913914
key->partopcintype= (Oid*)palloc0(key->partnatts*sizeof(Oid));
914915
key->partsupfunc= (FmgrInfo*)palloc0(key->partnatts*sizeof(FmgrInfo));
915916

916917
key->partcollation= (Oid*)palloc0(key->partnatts*sizeof(Oid));
917-
918-
/* Gather type and collation info as well */
919918
key->parttypid= (Oid*)palloc0(key->partnatts*sizeof(Oid));
920919
key->parttypmod= (int32*)palloc0(key->partnatts*sizeof(int32));
921920
key->parttyplen= (int16*)palloc0(key->partnatts*sizeof(int16));
@@ -990,6 +989,10 @@ RelationBuildPartitionKey(Relation relation)
990989

991990
ReleaseSysCache(tuple);
992991

992+
/* Assert that we're not leaking any old data during assignments below */
993+
Assert(relation->rd_partkeycxt==NULL);
994+
Assert(relation->rd_partkey==NULL);
995+
993996
/*
994997
* Success --- reparent our context and make the relcache point to the
995998
* newly constructed key
@@ -1314,11 +1317,15 @@ RelationBuildDesc(Oid targetRelId, bool insertIt)
13141317
}
13151318
else
13161319
{
1317-
relation->rd_partkeycxt=NULL;
13181320
relation->rd_partkey=NULL;
1321+
relation->rd_partkeycxt=NULL;
13191322
relation->rd_partdesc=NULL;
13201323
relation->rd_pdcxt=NULL;
13211324
}
1325+
/* ... but partcheck is not loaded till asked for */
1326+
relation->rd_partcheck=NIL;
1327+
relation->rd_partcheckvalid= false;
1328+
relation->rd_partcheckcxt=NULL;
13221329

13231330
/*
13241331
* if it's an index, initialize index-related information
@@ -2403,8 +2410,8 @@ RelationDestroyRelation(Relation relation, bool remember_tupdesc)
24032410
MemoryContextDelete(relation->rd_partkeycxt);
24042411
if (relation->rd_pdcxt)
24052412
MemoryContextDelete(relation->rd_pdcxt);
2406-
if (relation->rd_partcheck)
2407-
pfree(relation->rd_partcheck);
2413+
if (relation->rd_partcheckcxt)
2414+
MemoryContextDelete(relation->rd_partcheckcxt);
24082415
if (relation->rd_fdwroutine)
24092416
pfree(relation->rd_fdwroutine);
24102417
pfree(relation);
@@ -5663,18 +5670,20 @@ load_relcache_init_file(bool shared)
56635670
* format is complex and subject to change). They must be rebuilt if
56645671
* needed by RelationCacheInitializePhase3. This is not expected to
56655672
* be a big performance hit since few system catalogs have such. Ditto
5666-
* for RLS policy data, index expressions, predicates, exclusion info,
5667-
* and FDW info.
5673+
* for RLS policy data,partition info,index expressions, predicates,
5674+
*exclusion info,and FDW info.
56685675
*/
56695676
rel->rd_rules=NULL;
56705677
rel->rd_rulescxt=NULL;
56715678
rel->trigdesc=NULL;
56725679
rel->rd_rsdesc=NULL;
5673-
rel->rd_partkeycxt=NULL;
56745680
rel->rd_partkey=NULL;
5675-
rel->rd_pdcxt=NULL;
5681+
rel->rd_partkeycxt=NULL;
56765682
rel->rd_partdesc=NULL;
5683+
rel->rd_pdcxt=NULL;
56775684
rel->rd_partcheck=NIL;
5685+
rel->rd_partcheckvalid= false;
5686+
rel->rd_partcheckcxt=NULL;
56785687
rel->rd_indexprs=NIL;
56795688
rel->rd_indpred=NIL;
56805689
rel->rd_exclops=NULL;

‎src/include/utils/rel.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,9 +125,9 @@ typedef struct RelationData
125125
List*rd_fkeylist;/* list of ForeignKeyCacheInfo (see below) */
126126
boolrd_fkeyvalid;/* true if list has been computed */
127127

128-
MemoryContextrd_partkeycxt;/* privatememory cxtforthe below */
128+
MemoryContextrd_partkeycxt;/* privatecontextforrd_partkey, if any */
129129
structPartitionKeyData*rd_partkey;/* partition key, or NULL */
130-
MemoryContextrd_pdcxt;/* private context forpartdesc */
130+
MemoryContextrd_pdcxt;/* private context forrd_partdesc, if any */
131131
structPartitionDescData*rd_partdesc;/* partitions, or NULL */
132132
List*rd_partcheck;/* partition CHECK quals */
133133

@@ -216,6 +216,10 @@ typedef struct RelationData
216216

217217
/* use "struct" here to avoid needing to include pgstat.h: */
218218
structPgStat_TableStatus*pgstat_info;/* statistics collection area */
219+
220+
/* placed here to avoid ABI break before v12: */
221+
boolrd_partcheckvalid;/* true if list has been computed */
222+
MemoryContextrd_partcheckcxt;/* private cxt for rd_partcheck, if any */
219223
}RelationData;
220224

221225

0 commit comments

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