From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sat, 23 Sep 2017 17:28:16 +0000 (-0400)
Subject: Improve memory management in autovacuum.c.
X-Git-Tag: REL_11_BETA1~1516
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=335f3d04e4c8dd495c4dd30ab1049b6fe8f25052;p=postgresql

Improve memory management in autovacuum.c.

Invoke vacuum(), as well as "work item" processing, in the PortalContext
that do_autovacuum() has manufactured, which will be reset before each
such invocation.  This ensures cleanup of any memory leaked by these
operations.  It also avoids the rather dangerous practice of calling
vacuum() in a context that vacuum() itself will destroy while it runs.
There's no known live bug there, but it's not hard to imagine introducing
one if we leave it like this.

Tom Lane, reviewed by Michael Paquier and Alvaro Herrera

Discussion: https://postgr.es/m/13849.1506114543@sss.pgh.pa.us
---

diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index b745d8962e..db6d91ffdf 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2444,8 +2444,10 @@ do_autovacuum(void)
 		 */
 		PG_TRY();
 		{
+			/* Use PortalContext for any per-table allocations */
+			MemoryContextSwitchTo(PortalContext);
+
 			/* have at it */
-			MemoryContextSwitchTo(TopTransactionContext);
 			autovacuum_do_vac_analyze(tab, bstrategy);
 
 			/*
@@ -2482,6 +2484,9 @@ do_autovacuum(void)
 		}
 		PG_END_TRY();
 
+		/* Make sure we're back in AutovacMemCxt */
+		MemoryContextSwitchTo(AutovacMemCxt);
+
 		did_vacuum = true;
 
 		/* the PGXACT flags are reset at the next end of transaction */
@@ -2533,8 +2538,7 @@ deleted:
 		perform_work_item(workitem);
 
 		/*
-		 * Check for config changes before acquiring lock for further
-		 * jobs.
+		 * Check for config changes before acquiring lock for further jobs.
 		 */
 		CHECK_FOR_INTERRUPTS();
 		if (got_SIGHUP)
@@ -2605,6 +2609,7 @@ perform_work_item(AutoVacuumWorkItem *workitem)
 	 * must live in a long-lived memory context because we call vacuum and
 	 * analyze in different transactions.
 	 */
+	Assert(CurrentMemoryContext == AutovacMemCxt);
 
 	cur_relname = get_rel_name(workitem->avw_relation);
 	cur_nspname = get_namespace_name(get_rel_namespace(workitem->avw_relation));
@@ -2614,6 +2619,9 @@ perform_work_item(AutoVacuumWorkItem *workitem)
 
 	autovac_report_workitem(workitem, cur_nspname, cur_datname);
 
+	/* clean up memory before each work item */
+	MemoryContextResetAndDeleteChildren(PortalContext);
+
 	/*
 	 * We will abort the current work item if something errors out, and
 	 * continue with the next one; in particular, this happens if we are
@@ -2622,9 +2630,10 @@ perform_work_item(AutoVacuumWorkItem *workitem)
 	 */
 	PG_TRY();
 	{
-		/* have at it */
-		MemoryContextSwitchTo(TopTransactionContext);
+		/* Use PortalContext for any per-work-item allocations */
+		MemoryContextSwitchTo(PortalContext);
 
+		/* have at it */
 		switch (workitem->avw_type)
 		{
 			case AVW_BRINSummarizeRange:
@@ -2668,6 +2677,9 @@ perform_work_item(AutoVacuumWorkItem *workitem)
 	}
 	PG_END_TRY();
 
+	/* Make sure we're back in AutovacMemCxt */
+	MemoryContextSwitchTo(AutovacMemCxt);
+
 	/* We intentionally do not set did_vacuum here */
 
 	/* be tidy */