Move memoryAccountId out of PlannedStmt/Plan Nodes

Functions using SQL and PL/pgSQL will plan and execute arbitrary SQL
inside a running query. The first time we initialize a plan for an SQL
block, the memory accounting system creates a new memory account for
each Executor/Node.  In the case that we are executing a cached plan,
(i.e.  plancache.c) the memory accounts will have already been assigned
in a previous execution of the plan.

As a result, when explain_memory_verbosity is set to 'detail', it is not
clear what memory account corresponds to which executor. Instead, move
the memoryAccountId into PlanState/QueryDesc, which will insure that
every time we initialize an executor, it will be assigned a unique
memoryAccountId.
Co-authored-by: NMelanie Plageman <mplageman@pivotal.io>

src/backend/commands/explain_gp.c

@@ -981,7 +981,7 @@ cdbexplain_collectStatsFromNode(PlanState *planstate, CdbExplain_SendStatCtx *ct
 	si->workmemused = instr->workmemused;
 	si->workmemwanted = instr->workmemwanted;
 	si->workfileCreated = instr->workfileCreated;
-	si->peakMemBalance = MemoryAccounting_GetAccountPeakBalance(planstate->plan->memoryAccountId);
+	si->peakMemBalance = MemoryAccounting_GetAccountPeakBalance(planstate->memoryAccountId);
 	si->firststart = instr->firststart;
 	si->numPartScanned = instr->numPartScanned;
 	si->sortMethod = String2ExplainSortMethod(instr->sortMethod);

src/backend/executor/execMain.c

@@ -263,15 +263,13 @@ standard_ExecutorStart(QueryDesc *queryDesc, int eflags)
 	Assert(queryDesc != NULL);
 	Assert(queryDesc->estate == NULL);
 	Assert(queryDesc->plannedstmt != NULL);
+	Assert(queryDesc->memoryAccountId == MEMORY_OWNER_TYPE_Undefined);
 
 	PlannedStmt *plannedStmt = queryDesc->plannedstmt;
 
-	if (MEMORY_OWNER_TYPE_Undefined == plannedStmt->memoryAccountId)
-	{
-		plannedStmt->memoryAccountId = MemoryAccounting_CreateAccount(0, MEMORY_OWNER_TYPE_EXECUTOR);
-	}
+	queryDesc->memoryAccountId = MemoryAccounting_CreateAccount(0, MEMORY_OWNER_TYPE_EXECUTOR);
 
-	START_MEMORY_ACCOUNT(plannedStmt->memoryAccountId);
+	START_MEMORY_ACCOUNT(queryDesc->memoryAccountId);
 
 	Assert(plannedStmt->intoPolicy == NULL ||
 		GpPolicyIsPartitioned(plannedStmt->intoPolicy) ||
@@ -864,9 +862,9 @@ standard_ExecutorRun(QueryDesc *queryDesc,
 	Assert(estate != NULL);
 	Assert(!(estate->es_top_eflags & EXEC_FLAG_EXPLAIN_ONLY));
 
-	Assert(NULL != queryDesc->plannedstmt && MEMORY_OWNER_TYPE_Undefined != queryDesc->plannedstmt->memoryAccountId);
+	Assert(NULL != queryDesc->plannedstmt && MEMORY_OWNER_TYPE_Undefined != queryDesc->memoryAccountId);
 
-	START_MEMORY_ACCOUNT(queryDesc->plannedstmt->memoryAccountId);
+	START_MEMORY_ACCOUNT(queryDesc->memoryAccountId);
 
 	/*
 	 * Switch into per-query memory context
@@ -1148,9 +1146,9 @@ standard_ExecutorEnd(QueryDesc *queryDesc)
 
 	Assert(estate != NULL);
 
-	Assert(NULL != queryDesc->plannedstmt && MEMORY_OWNER_TYPE_Undefined != queryDesc->plannedstmt->memoryAccountId);
+	Assert(NULL != queryDesc->plannedstmt && MEMORY_OWNER_TYPE_Undefined != queryDesc->memoryAccountId);
 
-	START_MEMORY_ACCOUNT(queryDesc->plannedstmt->memoryAccountId);
+	START_MEMORY_ACCOUNT(queryDesc->memoryAccountId);
 
 	if (DEBUG1 >= log_min_messages)
 	{
@@ -1313,9 +1311,9 @@ ExecutorRewind(QueryDesc *queryDesc)
 
 	Assert(estate != NULL);
 
-	Assert(NULL != queryDesc->plannedstmt && MEMORY_OWNER_TYPE_Undefined != queryDesc->plannedstmt->memoryAccountId);
+	Assert(NULL != queryDesc->plannedstmt && MEMORY_OWNER_TYPE_Undefined != queryDesc->memoryAccountId);
 
-	START_MEMORY_ACCOUNT(queryDesc->plannedstmt->memoryAccountId);
+	START_MEMORY_ACCOUNT(queryDesc->memoryAccountId);
 
 	/* It's probably not sensible to rescan updating queries */
 	Assert(queryDesc->operation == CMD_SELECT);

src/backend/executor/execProcnode.c

@@ -894,7 +894,7 @@ ExecProcNode(PlanState *node)
 {
 	TupleTableSlot *result = NULL;
 
-	START_MEMORY_ACCOUNT(node->plan->memoryAccountId);
+	START_MEMORY_ACCOUNT(node->memoryAccountId);
 	{
 
 	CHECK_FOR_INTERRUPTS();
@@ -1190,7 +1190,7 @@ MultiExecProcNode(PlanState *node)
 
 	Assert(NULL != node->plan);
 
-	START_MEMORY_ACCOUNT(node->plan->memoryAccountId);
+	START_MEMORY_ACCOUNT(node->memoryAccountId);
 {
 	TRACE_POSTGRESQL_EXECPROCNODE_ENTER(GpIdentity.segindex, currentSliceId, nodeTag(node), node->plan->plan_node_id);
 

src/backend/executor/nodeHash.c

@@ -281,7 +281,7 @@ ExecHashTableCreate(HashState *hashState, HashJoinState *hjstate, List *hashOper
 	ListCell   *ho;
 	MemoryContext oldcxt;
 
-	START_MEMORY_ACCOUNT(hashState->ps.plan->memoryAccountId);
+	START_MEMORY_ACCOUNT(hashState->ps.memoryAccountId);
 	{
 	Hash *node = (Hash *) hashState->ps.plan;
 
@@ -654,7 +654,7 @@ ExecHashTableDestroy(HashState *hashState, HashJoinTable hashtable)
 	Assert(hashtable);
 	Assert(!hashtable->eagerlyReleased);
 
-	START_MEMORY_ACCOUNT(hashState->ps.plan->memoryAccountId);
+	START_MEMORY_ACCOUNT(hashState->ps.memoryAccountId);
 	{
 
 	/*
@@ -879,7 +879,7 @@ ExecHashTableInsert(HashState *hashState, HashJoinTable hashtable,
 	int			batchno;
 	int			hashTupleSize;
 
-	START_MEMORY_ACCOUNT(hashState->ps.plan->memoryAccountId);
+	START_MEMORY_ACCOUNT(hashState->ps.memoryAccountId);
 	{
 	PlanState *ps = &hashState->ps;
 
@@ -984,7 +984,7 @@ ExecHashGetHashValue(HashState *hashState, HashJoinTable hashtable,
 	MemoryContext oldContext;
 	bool		result = true;
 
-	START_MEMORY_ACCOUNT(hashState->ps.plan->memoryAccountId);
+	START_MEMORY_ACCOUNT(hashState->ps.memoryAccountId);
 	{
 
 	Assert(hashkeys_null);
@@ -1125,7 +1125,7 @@ ExecScanHashBucket(HashState *hashState, HashJoinState *hjstate,
 	HashJoinTuple hashTuple = hjstate->hj_CurTuple;
 	uint32		hashvalue = hjstate->hj_CurHashValue;
 
-	START_MEMORY_ACCOUNT(hashState->ps.plan->memoryAccountId);
+	START_MEMORY_ACCOUNT(hashState->ps.memoryAccountId);
 	{
 	/*
 	 * hj_CurTuple is the address of the tuple last returned from the current
@@ -1275,7 +1275,7 @@ ExecHashTableReset(HashState *hashState, HashJoinTable hashtable)
 	MemoryContext oldcxt;
 	int			nbuckets = hashtable->nbuckets;
 
-	START_MEMORY_ACCOUNT(hashState->ps.plan->memoryAccountId);
+	START_MEMORY_ACCOUNT(hashState->ps.memoryAccountId);
 	{
 	Assert(!hashtable->eagerlyReleased);
 
@@ -1350,7 +1350,7 @@ ExecHashTableExplainInit(HashState *hashState, HashJoinState *hjstate,
 	MemoryContext oldcxt;
 	int			nbatch = Max(hashtable->nbatch, 1);
 
-    START_MEMORY_ACCOUNT(hashState->ps.plan->memoryAccountId);
+    START_MEMORY_ACCOUNT(hashState->ps.memoryAccountId);
     {
     /* Switch to a memory context that survives until ExecutorEnd. */
     oldcxt = MemoryContextSwitchTo(hjstate->js.ps.state->es_query_cxt);
@@ -1630,7 +1630,7 @@ ExecHashTableExplainBatchEnd(HashState *hashState, HashJoinTable hashtable)
     HashJoinBatchStats *batchstats = &stats->batchstats[curbatch];
     int                 i;
     
-    START_MEMORY_ACCOUNT(hashState->ps.plan->memoryAccountId);
+    START_MEMORY_ACCOUNT(hashState->ps.memoryAccountId);
     {
     Assert(!hashtable->eagerlyReleased);
 

src/backend/tcop/pquery.c

@@ -112,6 +112,7 @@ CreateQueryDesc(PlannedStmt *plannedstmt,
 
 	qd->ddesc = NULL;
 	qd->gpmon_pkt = NULL;
+	qd->memoryAccountId = MEMORY_OWNER_TYPE_Undefined;
 	
 	if (Gp_role != GP_ROLE_EXECUTE)
 		increment_command_count();

src/include/executor/execdesc.h

@@ -264,6 +264,9 @@ typedef struct QueryDesc
 
 	/* This is always set NULL by the core system, but plugins can change it */
 	struct Instrumentation *totaltime;	/* total time spent in ExecutorRun */
+
+	/* The overall memory consumption account (i.e., outside of an operator) */
+	MemoryAccountIdType memoryAccountId;
 } QueryDesc;
 
 /* in pquery.c */

src/include/nodes/execnodes.h

@@ -1442,6 +1442,9 @@ typedef struct PlanState
 	gpmon_packet_t gpmon_pkt;
 
 	bool		fHadSentNodeStart;
+
+	/* MemoryAccount to use for recording the memory usage of different plan nodes. */
+	MemoryAccountIdType memoryAccountId;
 } PlanState;
 
 /* Gpperfmon helper functions defined in execGpmon.c */

src/include/nodes/plannodes.h

@@ -138,9 +138,6 @@ typedef struct PlannedStmt
 	/* What is the memory reserved for this query's execution? */
 	uint64		query_mem;
 
-	/* The overall memory consumption account (i.e., outside of an operator) */
-	MemoryAccountIdType memoryAccountId;
-
 	/*
 	 * GPDB: Used to keep target information for CTAS and it is needed
 	 * to be dispatched to QEs.
@@ -274,9 +271,6 @@ typedef struct Plan
 	 */
 	uint64 operatorMemKB;
 
-	/* MemoryAccount to use for recording the memory usage of different plan nodes. */
-	MemoryAccountIdType memoryAccountId;
-
 	/*
 	 * The parent motion node of a plan node.
 	 */

src/include/utils/memaccounting.h

@@ -194,21 +194,17 @@ extern MemoryAccountIdType ActiveMemoryAccountId;
  * that will not be executed in current slice
  */
 #define CREATE_EXECUTOR_MEMORY_ACCOUNT(isAlienPlanNode, planNode, NodeType) \
-		(MEMORY_OWNER_TYPE_Undefined != planNode->memoryAccountId) ?\
-			planNode->memoryAccountId : \
-			(isAlienPlanNode ? MEMORY_OWNER_TYPE_Exec_AlienShared : \
-				MemoryAccounting_CreateAccount(((Plan*)node)->operatorMemKB == 0 ? \
-				work_mem : ((Plan*)node)->operatorMemKB, MEMORY_OWNER_TYPE_Exec_##NodeType));
+		isAlienPlanNode ? MEMORY_OWNER_TYPE_Exec_AlienShared : \
+			MemoryAccounting_CreateAccount(((Plan*)node)->operatorMemKB == 0 ? \
+			work_mem : ((Plan*)node)->operatorMemKB, MEMORY_OWNER_TYPE_Exec_##NodeType);
 
 /*
  * SAVE_EXECUTOR_MEMORY_ACCOUNT saves an operator specific memory account
  * into the PlanState of that operator
  */
 #define SAVE_EXECUTOR_MEMORY_ACCOUNT(execState, curMemoryAccountId)\
-		Assert(MEMORY_OWNER_TYPE_Undefined == ((PlanState *)execState)->plan->memoryAccountId || \
-		MEMORY_OWNER_TYPE_Undefined == ((PlanState *)execState)->plan->memoryAccountId || \
-		curMemoryAccountId == ((PlanState *)execState)->plan->memoryAccountId);\
-		((PlanState *)execState)->plan->memoryAccountId = curMemoryAccountId;
+		Assert(MEMORY_OWNER_TYPE_Undefined == ((PlanState *)execState)->memoryAccountId); \
+		((PlanState *)execState)->memoryAccountId = curMemoryAccountId;
 
 extern MemoryAccountIdType
 MemoryAccounting_CreateAccount(long maxLimit, enum MemoryOwnerType ownerType);