On 2019-Oct-25, Tomas Vondra wrote:
> The attached patch trivially fixes that by adding a memory context
> tracking all the temporary data, and then just deletes it as a whole at
> the end of the function. This significantly reduces the memory usage for
> me, not sure it's 100% correct.
FWIW we already had this code (added by commit 2455ab48844c), but it was
removed by commit d3f48dfae42f. I think we should put it back. (I
think it may be useful to use a static MemoryContext that we can just
reset each time, instead of creating and deleting each time, to save on
memcxt churn. That'd make the function non-reentrant, but I don't see
that we'd make the catalogs partitioned any time soon. This may be
premature optimization though -- not really wedded to it.)
With Amit's patch to make RelationBuildPartitionDesc called lazily, the
time to plan the RI_InitialCheck query (using Kato Sho's test case) goes
from 30 seconds to 14 seconds on my laptop. Obviously there's more that
we'd need to fix to make the scenario fully supported, but it seems a
decent step forward.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From c9000408036903df95ae22d0918a3fffddc7bf48 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Wed, 13 Nov 2019 15:12:05 -0300
Subject: [PATCH 1/2] build partdesc memcxt
---
src/backend/partitioning/partdesc.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/src/backend/partitioning/partdesc.c b/src/backend/partitioning/partdesc.c
index 6ede084afe..c551df6673 100644
--- a/src/backend/partitioning/partdesc.c
+++ b/src/backend/partitioning/partdesc.c
@@ -71,6 +71,14 @@ RelationBuildPartitionDesc(Relation rel)
PartitionKey key = RelationGetPartitionKey(rel);
MemoryContext oldcxt;
int *mapping;
+ static MemoryContext tmpContext = NULL;
+
+ if (tmpContext == NULL)
+ tmpContext = AllocSetContextCreate(TopMemoryContext,
+ "build part tmp",
+ ALLOCSET_SMALL_SIZES);
+
+ oldcxt = MemoryContextSwitchTo(tmpContext);
/*
* Get partition oids from pg_inherits. This uses a single snapshot to
@@ -205,11 +213,11 @@ RelationBuildPartitionDesc(Relation rel)
boundinfo = partition_bounds_create(boundspecs, nparts, key, &mapping);
/* Now copy all info into relcache's partdesc. */
- oldcxt = MemoryContextSwitchTo(rel->rd_pdcxt);
+ MemoryContextSwitchTo(rel->rd_pdcxt);
partdesc->boundinfo = partition_bounds_copy(boundinfo, key);
partdesc->oids = (Oid *) palloc(nparts * sizeof(Oid));
partdesc->is_leaf = (bool *) palloc(nparts * sizeof(bool));
- MemoryContextSwitchTo(oldcxt);
+ MemoryContextSwitchTo(tmpContext);
/*
* Assign OIDs from the original array into mapped indexes of the
@@ -231,6 +239,9 @@ RelationBuildPartitionDesc(Relation rel)
}
}
+ MemoryContextSwitchTo(oldcxt);
+ MemoryContextResetAndDeleteChildren(tmpContext);
+
rel->rd_partdesc = partdesc;
}
--
2.20.1
>From 6deb1e88a188046939c8706eff8ad3e9f3121298 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Wed, 13 Nov 2019 15:34:13 -0300
Subject: [PATCH 2/2] Invoke RelationBuildPartitionDesc lazily
---
src/backend/partitioning/partdesc.c | 17 ++++++++++++++++-
src/backend/utils/cache/relcache.c | 22 +++++-----------------
src/include/partitioning/partdesc.h | 2 +-
src/include/utils/rel.h | 6 ------
4 files changed, 22 insertions(+), 25 deletions(-)
diff --git a/src/backend/partitioning/partdesc.c b/src/backend/partitioning/partdesc.c
index c551df6673..8171eb03ca 100644
--- a/src/backend/partitioning/partdesc.c
+++ b/src/backend/partitioning/partdesc.c
@@ -34,6 +34,7 @@
#include "utils/rel.h"
#include "utils/syscache.h"
+
typedef struct PartitionDirectoryData
{
MemoryContext pdir_mcxt;
@@ -47,6 +48,20 @@ typedef struct PartitionDirectoryEntry
PartitionDesc pd;
} PartitionDirectoryEntry;
+
+static void RelationBuildPartitionDesc(Relation rel);
+
+PartitionDesc
+RelationGetPartitionDesc(Relation rel)
+{
+ Assert(rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE);
+
+ if (rel->rd_partdesc == NULL)
+ RelationBuildPartitionDesc(rel);
+
+ return rel->rd_partdesc;
+}
+
/*
* RelationBuildPartitionDesc
* Form rel's partition descriptor, and store in relcache entry
@@ -57,7 +72,7 @@ typedef struct PartitionDirectoryEntry
* that's sufficient to prevent that can assume that rd_partdesc
* won't change underneath it.
*/
-void
+static void
RelationBuildPartitionDesc(Relation rel)
{
PartitionDesc partdesc;
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index ad1ff01b32..5ef61d187a 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -1165,20 +1165,17 @@ RelationBuildDesc(Oid targetRelId, bool insertIt)
relation->rd_fkeylist = NIL;
relation->rd_fkeyvalid = false;
- /* if a partitioned table, initialize key and partition descriptor info */
+ /* if a partitioned table, initialize key info */
if (relation->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
- {
RelationBuildPartitionKey(relation);
- RelationBuildPartitionDesc(relation);
- }
else
{
relation->rd_partkey = NULL;
relation->rd_partkeycxt = NULL;
- relation->rd_partdesc = NULL;
- relation->rd_pdcxt = NULL;
}
- /* ... but partcheck is not loaded till asked for */
+ /* ... but partcheck and partdesc are not loaded till asked for */
+ relation->rd_pdcxt = NULL;
+ relation->rd_partdesc = NULL;
relation->rd_partcheck = NIL;
relation->rd_partcheckvalid = false;
relation->rd_partcheckcxt = NULL;
@@ -3908,7 +3905,7 @@ RelationCacheInitializePhase3(void)
}
/*
- * Reload the partition key and descriptor for a partitioned table.
+ * Reload the partition key for a partitioned table.
*/
if (relation->rd_rel->relkind == RELKIND_PARTITIONED_TABLE &&
relation->rd_partkey == NULL)
@@ -3919,15 +3916,6 @@ RelationCacheInitializePhase3(void)
restart = true;
}
- if (relation->rd_rel->relkind == RELKIND_PARTITIONED_TABLE &&
- relation->rd_partdesc == NULL)
- {
- RelationBuildPartitionDesc(relation);
- Assert(relation->rd_partdesc != NULL);
-
- restart = true;
- }
-
if (relation->rd_tableam == NULL &&
(relation->rd_rel->relkind == RELKIND_RELATION ||
relation->rd_rel->relkind == RELKIND_SEQUENCE ||
diff --git a/src/include/partitioning/partdesc.h b/src/include/partitioning/partdesc.h
index 38712c1550..d30c8feb48 100644
--- a/src/include/partitioning/partdesc.h
+++ b/src/include/partitioning/partdesc.h
@@ -29,7 +29,7 @@ typedef struct PartitionDescData
PartitionBoundInfo boundinfo; /* collection of partition bounds */
} PartitionDescData;
-extern void RelationBuildPartitionDesc(Relation rel);
+extern PartitionDesc RelationGetPartitionDesc(Relation rel);
extern PartitionDirectory CreatePartitionDirectory(MemoryContext mcxt);
extern PartitionDesc PartitionDirectoryLookup(PartitionDirectory, Relation);
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index 8b8b237f0d..4ec27a63e8 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -603,12 +603,6 @@ typedef struct ViewOptions
*/
#define RelationGetPartitionKey(relation) ((relation)->rd_partkey)
-/*
- * RelationGetPartitionDesc
- * Returns partition descriptor for a relation.
- */
-#define RelationGetPartitionDesc(relation) ((relation)->rd_partdesc)
-
/* routines in utils/cache/relcache.c */
extern void RelationIncrementReferenceCount(Relation rel);
extern void RelationDecrementReferenceCount(Relation rel);
--
2.20.1
