Bugfix. AQOUtilityMemCtx is reset although some allocated data still in use.

Remove the AQOUtilityMemCtx memory context at all. It is used for too small
operations. I don't buy that such operations can allocate so much memory that
backend must free memory right after the end of operation to avoid OOM.
I guess, prediction, planning and execution memory context set is good enough.

Author: danolivo

aqo.c

@@ -87,9 +87,6 @@ MemoryContext		AQOTopMemCtx = NULL;
 /* Is released at the end of transaction */
 MemoryContext		AQOCacheMemCtx = NULL;
 
-/* Should be released in-place, just after a huge calculation */
-MemoryContext		AQOUtilityMemCtx = NULL;
-
 /* Is released at the end of planning */
 MemoryContext 		AQOPredictMemCtx = NULL;
 
@@ -348,15 +345,7 @@ _PG_init(void)
 	AQOCacheMemCtx = AllocSetContextCreate(AQOTopMemCtx,
 											 "AQOCacheMemCtx",
 											 ALLOCSET_DEFAULT_SIZES);
-	/*
-	 * AQOUtilityMemoryContext containe short-lived information which
-	 * is appeared from having got clause, selectivity arrays and relid lists
-	 * while calculating hashes. It clean up inside calculated
-	 * function or immediately after her having completed.
-	 */
-	AQOUtilityMemCtx = AllocSetContextCreate(AQOTopMemCtx,
-											 "AQOUtilityMemoryContext",
-											 ALLOCSET_DEFAULT_SIZES);
+
 	/*
 	 * AQOPredictMemoryContext save necessary information for making predict of plan nodes
 	 * and clean up in the execution stage of query.

aqo.h

@@ -225,7 +225,6 @@ extern int njoins;
 /* AQO Memory contexts */
 extern MemoryContext AQOTopMemCtx;
 extern MemoryContext AQOCacheMemCtx;
-extern MemoryContext AQOUtilityMemCtx;
 extern MemoryContext AQOPredictMemCtx;
 extern MemoryContext AQOLearnMemCtx;
 

hash.c

@@ -194,19 +194,20 @@ get_fss_for_object(List *relsigns, List *clauselist,
 	int			sh = 0,
 				old_sh;
 	int			fss_hash;
-	MemoryContext old_ctx_m;
 
 	n = list_length(clauselist);
 
 	/* Check parameters state invariant. */
 	Assert(n == list_length(selectivities) ||
 		   (nfeatures == NULL && features == NULL));
 
+	/*
+	 * It should be allocated in a caller memory context, because it will be
+	 * returned.
+	 */
 	if (nfeatures != NULL)
 		*features = palloc0(sizeof(**features) * n);
 
-	old_ctx_m = MemoryContextSwitchTo(AQOUtilityMemCtx);
-
 	get_eclasses(clauselist, &nargs, &args_hash, &eclass_hash);
 	clause_hashes = palloc(sizeof(*clause_hashes) * n);
 	clause_has_consts = palloc(sizeof(*clause_has_consts) * n);
@@ -281,9 +282,6 @@ get_fss_for_object(List *relsigns, List *clauselist,
 	relations_hash = get_relations_hash(relsigns);
 	fss_hash = get_fss_hash(clauses_hash, eclasses_hash, relations_hash);
 
-	MemoryContextSwitchTo(old_ctx_m);
-	MemoryContextReset(AQOUtilityMemCtx);
-
 	if (nfeatures != NULL)
 	{
 		*nfeatures = n - sh;

postprocessing.c

@@ -178,16 +178,13 @@ restore_selectivities(List *clauselist, List *relidslist, JoinType join_type,
 	double		*cur_sel;
 	int			cur_hash;
 	int			cur_relid;
-	MemoryContext old_ctx_m;
 
 	parametrized_sel = was_parametrized && (list_length(relidslist) == 1);
 	if (parametrized_sel)
 	{
 		cur_relid = linitial_int(relidslist);
 
-		old_ctx_m = MemoryContextSwitchTo(AQOUtilityMemCtx);
 		get_eclasses(clauselist, &nargs, &args_hash, &eclass_hash);
-		MemoryContextSwitchTo(old_ctx_m);
 	}
 
 	foreach(l, clauselist)
@@ -221,11 +218,6 @@ restore_selectivities(List *clauselist, List *relidslist, JoinType join_type,
 		lst = lappend(lst, cur_sel);
 	}
 
-	if (parametrized_sel)
-	{
-		MemoryContextReset(AQOUtilityMemCtx);
-	}
-
 	return lst;
 }
 

preprocessing.c

@@ -127,7 +127,8 @@ aqo_planner(Query *parse,
 			ParamListInfo boundParams)
 {
 	bool			query_is_stored = false;
-	MemoryContext oldctx;
+	MemoryContext	oldctx;
+
 	oldctx = MemoryContextSwitchTo(AQOPredictMemCtx);
 
 	 /*
@@ -156,21 +157,14 @@ aqo_planner(Query *parse,
 	}
 
 	selectivity_cache_clear();
-	MemoryContextSwitchTo(oldctx);
 
-	oldctx = MemoryContextSwitchTo(AQOUtilityMemCtx);
 	/* Check unlucky case (get a hash of zero) */
 	if (parse->queryId == UINT64CONST(0))
 		JumbleQuery(parse, query_string);
 
 	Assert(parse->utilityStmt == NULL);
 	Assert(parse->queryId != UINT64CONST(0));
 	query_context.query_hash = parse->queryId;
-	MemoryContextSwitchTo(oldctx);
-
-	MemoryContextReset(AQOUtilityMemCtx);
-
-	oldctx = MemoryContextSwitchTo(AQOPredictMemCtx);
 
 	/* By default, they should be equal */
 	query_context.fspace_hash = query_context.query_hash;
@@ -191,15 +185,14 @@ aqo_planner(Query *parse,
 									cursorOptions,
 									boundParams);
 	}
-	MemoryContextSwitchTo(oldctx);
 
 	elog(DEBUG1, "AQO will be used for query '%s', class "UINT64_FORMAT,
 		 query_string ? query_string : "null string", query_context.query_hash);
 
+	MemoryContextSwitchTo(oldctx);
 	oldctx = MemoryContextSwitchTo(AQOCacheMemCtx);
 	cur_classes = lappend_uint64(cur_classes, query_context.query_hash);
 	MemoryContextSwitchTo(oldctx);
-
 	oldctx = MemoryContextSwitchTo(AQOPredictMemCtx);
 
 	if (aqo_mode == AQO_MODE_DISABLED)

storage.c

@@ -2096,7 +2096,6 @@ cleanup_aqo_database(bool gentle, int *fs_num, int *fss_num)
 				for(i = 0; i < dentry->nrels; i++)
 				{
 					Oid reloid = ObjectIdGetDatum(*(Oid *)ptr);
-					MemoryContext oldctx = MemoryContextSwitchTo(AQOUtilityMemCtx);
 
 					if (!SearchSysCacheExists1(RELOID, reloid))
 						/* Remember this value */
@@ -2105,7 +2104,6 @@ cleanup_aqo_database(bool gentle, int *fs_num, int *fss_num)
 					else
 						actual_fss = list_append_unique_int(actual_fss,
 															dentry->key.fss);
-					MemoryContextSwitchTo(oldctx);
 
 					ptr += sizeof(Oid);
 				}
@@ -2155,8 +2153,6 @@ cleanup_aqo_database(bool gentle, int *fs_num, int *fss_num)
 			/* Query class preferences */
 			(*fs_num) += (int) _aqo_queries_remove(entry->queryid);
 		}
-
-		MemoryContextReset(AQOUtilityMemCtx);
 	}
 
 	/*