The v04-0001 looks good for me. I am also considering whether there is a way to
detect a memory leak. One preliminary idea is that for short-lived context such
as the "Subplan HashTable Temp Context", we can assume that MemoryContextReset
will be called frequently. Therefore, at the time of deletion, the memory usage
should not be excessively large. Based on this assumption, we could implement
the following check:
```
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 15fa4d0a55e..56218cb6863 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -166,6 +166,7 @@ static void MemoryContextStatsInternal(MemoryContext context, int level,
static void MemoryContextStatsPrint(MemoryContext context, void *passthru,
const char *stats_string,
bool print_to_stderr);
+static bool CheckMemoryContextLeak(MemoryContext context);
/*
* You should not do memory allocations within a critical section, because
@@ -502,6 +503,7 @@ MemoryContextDeleteOnly(MemoryContext context)
Assert(context != CurrentMemoryContext);
/* All the children should've been deleted already */
Assert(context->firstchild == NULL);
+ Assert(CheckMemoryContextLeak(context));
/*
* It's not entirely clear whether 'tis better to do this before or after
@@ -530,6 +532,24 @@ MemoryContextDeleteOnly(MemoryContext context)
VALGRIND_DESTROY_MEMPOOL(context);
}
+static bool
+CheckMemoryContextLeak(MemoryContext context)
+{
+#ifdef MEMORY_CONTEXT_CHECKING
+ if (!context->name)
+ return true;
+
+ if (!strcmp(context->name, "Subplan HashTable Temp Context") == 0)
+ return true;
+
+ /* The size of short-lived contexts should be kept under 1 MB. */
+ if ((MemoryContextMemAllocated(context, false) > 1024 * 1024))
+ return false;
+#endif
+ return true;
+}
+
+
/*
* MemoryContextDeleteChildren
* Delete all the descendants of the named context and release all
```
In debug mode, a memory leak can be easily detected with the following SQL.
After applying v04-0001, it runs normally:
```sql
with s as (
select
i::numeric as a
from
generate_series(1, 50000) i
)
select * from s where a not in (select * from s)
```
For v04-0002, I checked the commit history, and before commit 133924e1,
buildSubPlanHash was using ecxt_per_tuple_memory. It seems that the
"Subplan HashTable Temp Context" was introduced in order to fix a certain bug.
