[SERVER-79009] Remove debug calls to VariableEnvironment::rebuild Created: 17/Jul/23  Updated: 29/Oct/23  Resolved: 03/Oct/23

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: 7.2.0-rc0

Type: Task Priority: Major - P3
Reporter: Timour Katchaounov Assignee: Henri Nikku
Resolution: Fixed Votes: 0
Labels: M1
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
related to SERVER-81783 Investigate a segfault when not rebui... Backlog
Assigned Teams:
Query Optimization
Backwards Compatibility: Fully Compatible
Sprint: QO 2023-10-02, QO 2023-10-16
Participants:

 Description   

opt_phase_manager.cpp has several calls to VariableEnvironment::rebuild whose sole purpose is to call tassert to verify that the variable environment is in a consistent state.

These calls are costly, and should be avoided in release builds. These calls and their checks should wrapped in a kDebugBuild.

This is a sketch of the proposed change:

diff --git a/src/mongo/db/query/optimizer/opt_phase_manager.cpp b/src/mongo/db/query/optimizer/opt_phase_manager.cpp
index 8b7b218bda0..ce3b380b13f 100644
--- a/src/mongo/db/query/optimizer/opt_phase_manager.cpp
+++ b/src/mongo/db/query/optimizer/opt_phase_manager.cpp
@@ -181,12 +197,14 @@ void OptPhaseManager::runMemoLogicalRewrite(const OptPhase phase,
         tassert(6808702, "Logical writer failed to rewrite fix point.", fixPointRewritten);
 
         input = extractLatestPlan(_memo, rootGroupId);
-        env.rebuild(input);
+        // TODO: wrap in kDebugBuild
+        // env.rebuild(input);
     }
 
-    if (env.hasFreeVariables()) {
-        tasserted(6808703, "Plan has free variables: " + generateFreeVarsAssertMsg(env));
-    }
+    // TODO: wrap in kDebugBuild
+    // if (env.hasFreeVariables()) {
+    //     tasserted(6808703, "Plan has free variables: " + generateFreeVarsAssertMsg(env));
+    // }
 }
 
 PlanExtractorResult OptPhaseManager::runMemoPhysicalRewrite(
@@ -247,12 +265,13 @@ PlanExtractorResult OptPhaseManager::runMemoPhysicalRewrite(
     result =
         extractPhysicalPlans(includeRejected, _physicalNodeId, _metadata, _ridProjections, _memo);
 
-    for (const auto& planEntry : result) {
-        env.rebuild(planEntry._node);
-        if (env.hasFreeVariables()) {
-            tasserted(6808707, "Plan has free variables: " + generateFreeVarsAssertMsg(env));
-        }
-    }
+    // TODO: wrap in kDebugBuild
+    // for (const auto& planEntry : result) {
+    //     env.rebuild(planEntry._node);
+    //     if (env.hasFreeVariables()) {
+    //         tasserted(6808707, "Plan has free variables: " + generateFreeVarsAssertMsg(env));
+    //     }
+    // }
     return result;
 }
 
@@ -304,7 +323,10 @@ PlanExtractorResult OptPhaseManager::optimizeNoAssert(ABT input, const bool incl
     };
 
     runStructuralPhases<OptPhase::ConstEvalPre, OptPhase::PathFuse, ConstEval, PathFusion>(
-        ConstEval{env, canInlineEvalPre}, PathFusion{env}, env, input);
+        ConstEval{env, canInlineEvalPre, {}, {}, false /*eagerRefTracking*/},
+        PathFusion{env},
+        env,
+        input);
 
     auto planExtractionResult = runMemoRewritePhases(includeRejected, env, input);
     // At this point "input" has been siphoned out.
@@ -366,14 +388,19 @@ PlanExtractorResult OptPhaseManager::optimizeNoAssert(ABT input, const bool incl
         };
 
         runStructuralPhase<OptPhase::ConstEvalPost, ConstEval>(
-            ConstEval{env, {} /*canInlineEvalFn*/, erasedProjFn, renamedProjFn},
+            ConstEval{env,
+                      {} /*canInlineEvalFn*/,
+                      erasedProjFn,
+                      renamedProjFn,
+                      false /*eagerRefTracking*/},
             env,
             planEntry._node);
 
-        env.rebuild(planEntry._node);
-        if (env.hasFreeVariables()) {
-            tasserted(6808710, "Plan has free variables: " + generateFreeVarsAssertMsg(env));
-        }
+        // TODO: wrap in kDebugBuild
+        // env.rebuild(planEntry._node);
+        // if (env.hasFreeVariables()) {
+        //     tasserted(6808710, "Plan has free variables: " + generateFreeVarsAssertMsg(env));
+        // }
     }
 
     tassert(6624174,
 



 Comments   
Comment by Githook User [ 03/Oct/23 ]

Author:

{'name': 'henrinikku', 'email': 'henri.nikku@mongodb.com', 'username': 'henrinikku'}

Message: SERVER-79009 Remove debug calls to VariableEnvironment::rebuild
Branch: master
https://github.com/mongodb/mongo/commit/e5cfd8e6fabae7b808455b017df2a2564bbc8a60

Generated at Thu Feb 08 06:39:50 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.