Further reduce memory footprint of CLOBBER_CACHE_ALWAYS testing.

Some buildfarm members using CLOBBER_CACHE_ALWAYS have been having OOM
problems of late.  Commit 2455ab488 addressed this problem by recovering
space transiently used within RelationBuildPartitionDesc, but it turns
out that leaves quite a lot on the table, because other subroutines of
RelationBuildDesc also leak memory like mad.  Let's move the temp-context
management into RelationBuildDesc so that leakage from the other
subroutines is also recovered.

I examined this issue by arranging for postgres.c to dump the size of
MessageContext just before resetting it in each command cycle, and
then running the update.sql regression test (which is one of the two
that are seeing buildfarm OOMs) with and without CLOBBER_CACHE_ALWAYS.
Before 2455ab488, the peak space usage with CCA was as much as 250MB.
That patch got it down to ~80MB, but with this patch it's about 0.5MB,
and indeed the space usage now seems nearly indistinguishable from a
non-CCA build.

RelationBuildDesc's traditional behavior of not worrying about leaking
transient data is of many years' standing, so I'm pretty hesitant to
change that without more evidence that it'd be useful in a normal build.
(So far as I can see, non-CCA memory consumption is about the same with
or without this change, whuch if anything suggests that it isn't useful.)
Hence, configure the patch so that we recover space only when
CLOBBER_CACHE_ALWAYS or CLOBBER_CACHE_RECURSIVELY is defined.  However,
that choice can be overridden at compile time, in case somebody would
like to do some performance testing and try to develop evidence for
changing that decision.

It's possible that we ought to back-patch this change, but in the
absence of back-branch OOM problems in the buildfarm, I'm not in
a hurry to do that.

Discussion: https://postgr.es/m/CA+TgmoY3bRmGB6-DUnoVy5fJoreiBJ43rwMrQRCdPXuKt4Ykaw@mail.gmail.com

diff --git a/src/backend/partitioning/partdesc.c b/src/backend/partitioning/partdesc.c
index 5f349ad46addc2a738e0a31aa8eb67833e9de391..a65cc526da54b4762b841554f66f183c29be3712 100644 (file)
--- a/src/backend/partitioning/partdesc.c
+++ b/src/backend/partitioning/partdesc.c
@@ -67,19 +67,8 @@ RelationBuildPartitionDesc(Relation rel)
                                nparts;
        PartitionKey key = RelationGetPartitionKey(rel);
        MemoryContext oldcxt;
-       MemoryContext rbcontext;
        int                *mapping;
 
-       /*
-        * While building the partition descriptor, we create various temporary
-        * data structures; in CLOBBER_CACHE_ALWAYS mode, at least, it's important
-        * not to leak them, since this can get called a lot.
-        */
-       rbcontext = AllocSetContextCreate(CurrentMemoryContext,
-                                                                         "RelationBuildPartitionDesc",
-                                                                         ALLOCSET_DEFAULT_SIZES);
-       oldcxt = MemoryContextSwitchTo(rbcontext);
-
        /*
         * Get partition oids from pg_inherits.  This uses a single snapshot to
         * fetch the list of children, so while more children may be getting
@@ -197,14 +186,15 @@ RelationBuildPartitionDesc(Relation rel)
        /* If there are no partitions, the rest of the partdesc can stay zero */
        if (nparts > 0)
        {
-               /* Create PartitionBoundInfo, using the temporary context. */
+               /* Create PartitionBoundInfo, using the caller's context. */
                boundinfo = partition_bounds_create(boundspecs, nparts, key, &mapping);
 
                /* Now copy all info into relcache's partdesc. */
-               MemoryContextSwitchTo(rel->rd_pdcxt);
+               oldcxt = 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);
 
                /*
                 * Assign OIDs from the original array into mapped indexes of the
@@ -214,9 +204,8 @@ RelationBuildPartitionDesc(Relation rel)
                 *
                 * Also record leaf-ness of each partition.  For this we use
                 * get_rel_relkind() which may leak memory, so be sure to run it in
-                * the temporary context.
+                * the caller's context.
                 */
-               MemoryContextSwitchTo(rbcontext);
                for (i = 0; i < nparts; i++)
                {
                        int                     index = mapping[i];
@@ -228,10 +217,6 @@ RelationBuildPartitionDesc(Relation rel)
        }
 
        rel->rd_partdesc = partdesc;
-
-       /* Return to caller's context, and blow away the temporary context. */
-       MemoryContextSwitchTo(oldcxt);
-       MemoryContextDelete(rbcontext);
 }
 
 /*

diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index eba77491fd568582690f717549e5b36f2d718f3e..84609e07253607634639b9003ae1b35fc62afb71 100644 (file)
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -93,6 +93,19 @@
 
 #define RELCACHE_INIT_FILEMAGIC                0x573266        /* version ID value */
 
+/*
+ * Default policy for whether to apply RECOVER_RELATION_BUILD_MEMORY:
+ * do so in clobber-cache builds but not otherwise.  This choice can be
+ * overridden at compile time with -DRECOVER_RELATION_BUILD_MEMORY=1 or =0.
+ */
+#ifndef RECOVER_RELATION_BUILD_MEMORY
+#if defined(CLOBBER_CACHE_ALWAYS) || defined(CLOBBER_CACHE_RECURSIVELY)
+#define RECOVER_RELATION_BUILD_MEMORY 1
+#else
+#define RECOVER_RELATION_BUILD_MEMORY 0
+#endif
+#endif
+
 /*
  *             hardcoded tuple descriptors, contents generated by genbki.pl
  */
@@ -1014,6 +1027,28 @@ RelationBuildDesc(Oid targetRelId, bool insertIt)
        HeapTuple       pg_class_tuple;
        Form_pg_class relp;
 
+       /*
+        * This function and its subroutines can allocate a good deal of transient
+        * data in CurrentMemoryContext.  Traditionally we've just leaked that
+        * data, reasoning that the caller's context is at worst of transaction
+        * scope, and relcache loads shouldn't happen so often that it's essential
+        * to recover transient data before end of statement/transaction.  However
+        * that's definitely not true in clobber-cache test builds, and perhaps
+        * it's not true in other cases.  If RECOVER_RELATION_BUILD_MEMORY is not
+        * zero, arrange to allocate the junk in a temporary context that we'll
+        * free before returning.  Make it a child of caller's context so that it
+        * will get cleaned up appropriately if we error out partway through.
+        */
+#if RECOVER_RELATION_BUILD_MEMORY
+       MemoryContext tmpcxt;
+       MemoryContext oldcxt;
+
+       tmpcxt = AllocSetContextCreate(CurrentMemoryContext,
+                                                                  "RelationBuildDesc workspace",
+                                                                  ALLOCSET_DEFAULT_SIZES);
+       oldcxt = MemoryContextSwitchTo(tmpcxt);
+#endif
+
        /*
         * find the tuple in pg_class corresponding to the given relation id
         */
@@ -1023,7 +1058,14 @@ RelationBuildDesc(Oid targetRelId, bool insertIt)
         * if no such tuple exists, return NULL
         */
        if (!HeapTupleIsValid(pg_class_tuple))
+       {
+#if RECOVER_RELATION_BUILD_MEMORY
+               /* Return to caller's context, and blow away the temporary context */
+               MemoryContextSwitchTo(oldcxt);
+               MemoryContextDelete(tmpcxt);
+#endif
                return NULL;
+       }
 
        /*
         * get information from the pg_class_tuple
@@ -1203,6 +1245,12 @@ RelationBuildDesc(Oid targetRelId, bool insertIt)
        /* It's fully valid */
        relation->rd_isvalid = true;
 
+#if RECOVER_RELATION_BUILD_MEMORY
+       /* Return to caller's context, and blow away the temporary context */
+       MemoryContextSwitchTo(oldcxt);
+       MemoryContextDelete(tmpcxt);
+#endif
+
        return relation;
 }