Date: Wed, 15 Sep 2021 15:09:28 -0500
From: Justin Pryzby <pryzby@telsasoft.com>
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: pgsql-hackers@postgresql.org
Subject: mem context is not reset between extended stats
Message-ID: <20210915200928.GP831@telsasoft.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="kXdP64Ggrk/fb43R"
Content-Disposition: inline
User-Agent: Mutt/1.9.4 (2018-02-28)
Archived-At: 
 <https://www.postgresql.org/message-id/20210915200928.GP831%40telsasoft.com>
Precedence: bulk


--kXdP64Ggrk/fb43R
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Memory allocation appeared be O(1) WRT the number of statistics objects, which
was not expected to me.  This is true in v13 (and probably back to v10).

It seems to work fine to reset the memory context within the loop, so long as
the statslist is allocated in the parent context.

|DROP TABLE t; CREATE TABLE t AS SELECT i, i+1 AS a, i+2 AS b, i+3 AS c, i+4 AS d, i+5 AS e FROM generate_series(1,99999)i;

|SELECT format('CREATE STATISTICS sta%s (ndistinct) ON a,(1+b),(2+c),(3+d),(4+e) FROM t', a) FROM generate_series(1,9)a\gexec
|SET log_statement_stats=on; SET client_min_messages=debug; ANALYZE t;
|=> 369432 kB max resident size

|SELECT format('CREATE STATISTICS sta%s (ndistinct) ON a,b,c,d,e FROM t', a) FROM generate_series(1,33)a\gexec
|SET log_statement_stats=on; SET client_min_messages=debug; ANALYZE t;
|=> 1284368 kB max resident size

--kXdP64Ggrk/fb43R
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0001-stx-do-not-leak-memory-for-each-stats-obj.patch"

From 6ae4d059f2ed8baf2af92ec0847458055147383c Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Wed, 15 Sep 2021 14:38:49 -0500
Subject: [PATCH] stx: do not leak memory for each stats obj

---
 src/backend/statistics/extended_stats.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c
index 63549757ec..d9387ceeb6 100644
--- a/src/backend/statistics/extended_stats.c
+++ b/src/backend/statistics/extended_stats.c
@@ -136,14 +136,14 @@ BuildRelationExtStatistics(Relation onerel, double totalrows,
 	if (!natts)
 		return;
 
+	pg_stext = table_open(StatisticExtRelationId, RowExclusiveLock);
+	statslist = fetch_statentries_for_relation(pg_stext, RelationGetRelid(onerel));
+
 	cxt = AllocSetContextCreate(CurrentMemoryContext,
 								"BuildRelationExtStatistics",
 								ALLOCSET_DEFAULT_SIZES);
 	oldcxt = MemoryContextSwitchTo(cxt);
 
-	pg_stext = table_open(StatisticExtRelationId, RowExclusiveLock);
-	statslist = fetch_statentries_for_relation(pg_stext, RelationGetRelid(onerel));
-
 	/* report this phase */
 	if (statslist != NIL)
 	{
@@ -245,14 +245,12 @@ BuildRelationExtStatistics(Relation onerel, double totalrows,
 		pgstat_progress_update_param(PROGRESS_ANALYZE_EXT_STATS_COMPUTED,
 									 ++ext_cnt);
 
-		/* free the build data (allocated as a single chunk) */
-		pfree(data);
+		MemoryContextReset(cxt);
 	}
 
-	table_close(pg_stext, RowExclusiveLock);
-
 	MemoryContextSwitchTo(oldcxt);
 	MemoryContextDelete(cxt);
+	table_close(pg_stext, RowExclusiveLock);
 }
 
 /*
-- 
2.17.0


--kXdP64Ggrk/fb43R--