database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-07-05 07:21:24 +03:00
Code Activity

Prevent memory leaks associated with relcache rd_partcheck structures.

Browse Source 
The original coding of generate_partition_qual() just copied the list
of predicate expressions into the global CacheMemoryContext, making it
effectively impossible to clean up when the owning relcache entry is
destroyed --- the relevant code in RelationDestroyRelation() only managed
to free the topmost List header :-(.  This resulted in a session-lifespan
memory leak whenever a table partition's relcache entry is rebuilt.
Fortunately, that's not normally a large data structure, and rebuilds
shouldn't occur all that often in production situations; but this is
still 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 empty
partcheck list; while that probably isn't a case that's very interesting
for production use, it's legal.

In passing, clarify comments about how partitioning-related relcache
data structures are managed, and add some Asserts that we're not leaking
old copies when we overwrite these data fields.

Amit Langote and Tom Lane

Discussion: https://postgr.es/m/7961.1552498252@sss.pgh.pa.us
This commit is contained in:
Tom Lane
2019-04-13 13:22:26 -04:00
parent c098509927
commit 5f1433ac5e
4 changed files with 75 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

54
src/backend/utils/cache/partcache.c vendored
View File

@ -41,11 +41,11 @@ static List *generate_partition_qual(Relation rel);
/*
* RelationBuildPartitionKey
* Build and attach to relcache partition key data of relation
* Build partition key data of relation, and attach to relcache
*
* Partitioning key data is a complex structure; to avoid complicated logic to
* free individual elements whenever the relcache entry is flushed, we give it
* its own memory context, child of CacheMemoryContext, which can easily be
* its own memory context, a child of CacheMemoryContext, which can easily be
* deleted on its own. To avoid leaking memory in that context in case of an
* error partway through this function, the context is initially created as a
* child of CurTransactionContext and only re-parented to CacheMemoryContext
@ -144,6 +144,7 @@ RelationBuildPartitionKey(Relation relation)
MemoryContextSwitchTo(oldcxt);
}
/* Allocate assorted arrays in the partkeycxt, which we'll fill below */
oldcxt = MemoryContextSwitchTo(partkeycxt);
key->partattrs = (AttrNumber *) palloc0(key->partnatts * sizeof(AttrNumber));
key->partopfamily = (Oid *) palloc0(key->partnatts * sizeof(Oid));
@ -151,8 +152,6 @@ RelationBuildPartitionKey(Relation relation)
key->partsupfunc = (FmgrInfo *) palloc0(key->partnatts * sizeof(FmgrInfo));
key->partcollation = (Oid *) palloc0(key->partnatts * sizeof(Oid));
/* Gather type and collation info as well */
key->parttypid = (Oid *) palloc0(key->partnatts * sizeof(Oid));
key->parttypmod = (int32 *) palloc0(key->partnatts * sizeof(int32));
key->parttyplen = (int16 *) palloc0(key->partnatts * sizeof(int16));
@ -235,6 +234,10 @@ RelationBuildPartitionKey(Relation relation)
ReleaseSysCache(tuple);
/* Assert that we're not leaking any old data during assignments below */
Assert(relation->rd_partkeycxt == NULL);
Assert(relation->rd_partkey == NULL);
/*
* Success --- reparent our context and make the relcache point to the
* newly constructed key
@ -305,11 +308,9 @@ get_partition_qual_relid(Oid relid)
* Generate partition predicate from rel's partition bound expression. The
* function returns a NIL list if there is no predicate.
*
* Result expression tree is stored CacheMemoryContext to ensure it survives
* as long as the relcache entry. But we should be running in a less long-lived
* working context. To avoid leaking cache memory if this routine fails partway
* through, we build in working memory and then copy the completed structure
* into cache memory.
* We cache a copy of the result in the relcache entry, after constructing
* it using the caller's context. This approach avoids leaking any data
* into long-lived cache contexts, especially if we fail partway through.
*/
static List *
generate_partition_qual(Relation rel)
@ -326,8 +327,8 @@ generate_partition_qual(Relation rel)
/* Guard against stack overflow due to overly deep partition tree */
check_stack_depth();
/* Quick copy */
if (rel->rd_partcheck != NIL)
/* If we already cached the result, just return a copy */
if (rel->rd_partcheckvalid)
return copyObject(rel->rd_partcheck);
/* Grab at least an AccessShareLock on the parent table */
@ -373,13 +374,36 @@ generate_partition_qual(Relation rel)
if (found_whole_row)
elog(ERROR, "unexpected whole-row reference found in partition key");
/* Save a copy in the relcache */
oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
rel->rd_partcheck = copyObject(result);
MemoryContextSwitchTo(oldcxt);
/* Assert that we're not leaking any old data during assignments below */
Assert(rel->rd_partcheckcxt == NULL);
Assert(rel->rd_partcheck == NIL);
/*
* Save a copy in the relcache. The order of these operations is fairly
* critical to avoid memory leaks and ensure that we don't leave a corrupt
* relcache entry if we fail partway through copyObject.
*
* If, as is definitely possible, the partcheck list is NIL, then we do
* not need to make a context to hold it.
*/
if (result != NIL)
{
rel->rd_partcheckcxt = AllocSetContextCreate(CacheMemoryContext,
"partition constraint",
ALLOCSET_SMALL_SIZES);
MemoryContextCopyAndSetIdentifier(rel->rd_partcheckcxt,
RelationGetRelationName(rel));
oldcxt = MemoryContextSwitchTo(rel->rd_partcheckcxt);
rel->rd_partcheck = copyObject(result);
MemoryContextSwitchTo(oldcxt);
}
else
rel->rd_partcheck = NIL;
rel->rd_partcheckvalid = true;
/* Keep the parent locked until commit */
relation_close(parent, NoLock);
/* Return the working copy to the caller */
return result;
}