Merge #61863

61863: sql: don't save memo unnecessarily r=RaduBerinde a=RaduBerinde We save a reference to the Memo, which is useful for explaining plans. However, this means that we're holding on to the memory used by the entire explored memo during execution of the query. This change makes it so that we only save it only if we're building an explain plan. Fixes #59065. Release note: None Co-authored-by: Radu Berinde <[email protected]>
cockroachdb · Mar 12, 2021 · 4a39f9a · 4a39f9a
2 parents a5b80ff + 0416f01
commit 4a39f9a
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 3 deletions.
diff --git a/pkg/sql/instrumentation.go b/pkg/sql/instrumentation.go
@@ -354,6 +354,11 @@ func (ih *instrumentationHelper) ShouldCollectExecStats() bool {
 	return ih.collectExecStats
 }
 
+// ShouldSaveMemo returns true if we should save the memo and catalog in planTop.
+func (ih *instrumentationHelper) ShouldSaveMemo() bool {
+	return ih.ShouldBuildExplainPlan()
+}
+
 // RecordExplainPlan records the explain.Plan for this query.
 func (ih *instrumentationHelper) RecordExplainPlan(explainPlan *explain.Plan) {
 	ih.explainPlan = explainPlan

diff --git a/pkg/sql/plan.go b/pkg/sql/plan.go
@@ -285,7 +285,7 @@ type planTop struct {
 	planComponents
 
 	// mem/catalog retains the memo and catalog that were used to create the
-	// plan.
+	// plan. Only set if needed by instrumentation (see ShouldSaveMemo).
 	mem     *memo.Memo
 	catalog *optCatalog
 

diff --git a/pkg/sql/plan_opt.go b/pkg/sql/plan_opt.go
@@ -584,8 +584,6 @@ func (opc *optPlanningCtx) runExecBuilder(
 	}
 
 	planTop.planComponents = *result
-	planTop.mem = mem
-	planTop.catalog = &opc.catalog
 	planTop.stmt = stmt
 	planTop.flags = opc.flags
 	if isDDL {
@@ -597,5 +595,9 @@ func (opc *optPlanningCtx) runExecBuilder(
 	if containsFullIndexScan {
 		planTop.flags.Set(planFlagContainsFullIndexScan)
 	}
+	if planTop.instrumentation.ShouldSaveMemo() {
+		planTop.mem = mem
+		planTop.catalog = &opc.catalog
+	}
 	return nil
 }