Fix task->fetchedExplainAnalyzePlan memory issue.

pull/3988/head
Hadi Moshayedi 2020-07-06 14:09:43 -07:00
parent 1c9810e395
commit 23fa421639
3 changed files with 42 additions and 8 deletions

View File

@ -694,13 +694,7 @@ AdaptiveExecutor(CitusScanState *scanState)
Assert(!scanState->finishedRemoteScan);
/* Reset Task fields that are only valid for a single execution */
Task *task = NULL;
foreach_ptr(task, taskList)
{
task->totalReceivedTupleData = 0;
task->fetchedExplainAnalyzePlacementIndex = 0;
task->fetchedExplainAnalyzePlan = NULL;
}
ResetExplainAnalyzeData(taskList);
scanState->tuplestorestate =
tuplestore_begin_heap(randomAccess, interTransactions, work_mem);

View File

@ -1194,8 +1194,26 @@ ExplainAnalyzeDestPutTuple(TupleDestination *self, Task *task,
char *fetchedExplainAnalyzePlan = TextDatumGetCString(explainAnalyze);
/*
* Allocate fetchedExplainAnalyzePlan in the same context as the Task, since we are
* currently in execution context and a Task can span multiple executions.
*
* Although we won't reuse the same value in a future execution, but we have
* calls to CheckNodeCopyAndSerialization() which asserts copy functions of the task
* work as expected, which will try to copy this value in a future execution.
*
* Why we don't we just allocate this field in executor context and reset it before
* the next execution? Because when an error is raised we can skip pretty much most
* of the meaningful places that we can insert the reset.
*
* TODO: Take all EXPLAIN ANALYZE related fields out of Task and store them in a
* Task to ExplainAnalyzePrivate mapping in multi_explain.c, so we don't need to
* do these hacky memory context management tricks.
*/
MemoryContext taskContext = GetMemoryChunkContext(tupleDestination->originalTask);
tupleDestination->originalTask->fetchedExplainAnalyzePlan =
pstrdup(fetchedExplainAnalyzePlan);
MemoryContextStrdup(taskContext, fetchedExplainAnalyzePlan);
tupleDestination->originalTask->fetchedExplainAnalyzePlacementIndex =
placementIndex;
}
@ -1207,6 +1225,27 @@ ExplainAnalyzeDestPutTuple(TupleDestination *self, Task *task,
}
/*
* ResetExplainAnalyzeData reset fields in Task that are used by multi_explain.c
*/
void
ResetExplainAnalyzeData(List *taskList)
{
Task *task = NULL;
foreach_ptr(task, taskList)
{
if (task->fetchedExplainAnalyzePlan != NULL)
{
pfree(task->fetchedExplainAnalyzePlan);
}
task->totalReceivedTupleData = 0;
task->fetchedExplainAnalyzePlacementIndex = 0;
task->fetchedExplainAnalyzePlan = NULL;
}
}
/*
* ExplainAnalyzeDestTupleDescForQuery implements TupleDestination->tupleDescForQuery
* for ExplainAnalyzeDestination.

View File

@ -26,5 +26,6 @@ extern List * ExplainAnalyzeTaskList(List *originalTaskList,
TupleDestination *defaultTupleDest, TupleDesc
tupleDesc, ParamListInfo params);
extern bool RequestedForExplainAnalyze(CitusScanState *node);
extern void ResetExplainAnalyzeData(List *taskList);
#endif /* MULTI_EXPLAIN_H */