From ed352092eda254155d715d87325c7b7889ded11f Mon Sep 17 00:00:00 2001 From: Fox Lady Date: Tue, 20 Mar 2018 13:05:50 -0400 Subject: [PATCH 01/14] SERVER-19904 Added command to check element's type --- src/mongo/bson/bsonelement.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mongo/bson/bsonelement.h b/src/mongo/bson/bsonelement.h index 43cb8aa206bdd..a5386d5cf3f36 100644 --- a/src/mongo/bson/bsonelement.h +++ b/src/mongo/bson/bsonelement.h @@ -463,7 +463,7 @@ class BSONElement { /** Get binary data. Element must be of type BinData. Handles type 2 */ const char* binDataClean(int& len) const { // BinData: - if (binDataType() != ByteArrayDeprecated) { + if (type() == BinData && binDataType() != ByteArrayDeprecated) { return binData(len); } else { // Skip extra size From a7c1914f52803bb855bc5ca42f26426b37805138 Mon Sep 17 00:00:00 2001 From: Fox Lady Date: Tue, 20 Mar 2018 13:17:12 -0400 Subject: [PATCH 02/14] SERVER-22480: Changed DocumentSource's member variable name to start with an underscore --- src/mongo/db/pipeline/document_source_sort.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/mongo/db/pipeline/document_source_sort.h b/src/mongo/db/pipeline/document_source_sort.h index d2ad1cd9ec408..36194bd2bfca4 100644 --- a/src/mongo/db/pipeline/document_source_sort.h +++ b/src/mongo/db/pipeline/document_source_sort.h @@ -74,7 +74,7 @@ class DocumentSourceSort final : public DocumentSource, public SplittableDocumen : ChangeStreamRequirement::kBlacklist); // Can't swap with a $match if a limit has been absorbed, as $match can't swap with $limit. - constraints.canSwapWithMatch = !limitSrc; + constraints.canSwapWithMatch = !_limitSrc; return constraints; } @@ -144,7 +144,7 @@ class DocumentSourceSort final : public DocumentSource, public SplittableDocumen }; boost::intrusive_ptr getLimitSrc() const { - return limitSrc; + return _limitSrc; } protected: @@ -242,8 +242,8 @@ class DocumentSourceSort final : public DocumentSource, public SplittableDocumen * the smallest limit. */ void setLimitSrc(boost::intrusive_ptr limit) { - if (!limitSrc || limit->getLimit() < limitSrc->getLimit()) { - limitSrc = limit; + if (!_limitSrc || limit->getLimit() < _limitSrc->getLimit()) { + _limitSrc = limit; } } @@ -258,7 +258,7 @@ class DocumentSourceSort final : public DocumentSource, public SplittableDocumen // The set of paths on which we're sorting. std::set _paths; - boost::intrusive_ptr limitSrc; + boost::intrusive_ptr _limitSrc; uint64_t _maxMemoryUsageBytes; bool _done; From b9c7aedaf2772c67111c93b132ab6e57be4cf689 Mon Sep 17 00:00:00 2001 From: Fox Lady Date: Thu, 22 Mar 2018 08:24:09 -0400 Subject: [PATCH 03/14] Added backup plan indicator --- jstests/core/server32721.js | 18 ++++++++++++++++++ src/mongo/db/exec/multi_plan.cpp | 6 +++++- src/mongo/db/exec/multi_plan.h | 3 +++ src/mongo/db/pipeline/document_source_sort.h | 10 +++++----- src/mongo/db/query/explain.cpp | 8 ++++++++ 5 files changed, 39 insertions(+), 6 deletions(-) create mode 100644 jstests/core/server32721.js diff --git a/jstests/core/server32721.js b/jstests/core/server32721.js new file mode 100644 index 0000000000000..42750b71ba82e --- /dev/null +++ b/jstests/core/server32721.js @@ -0,0 +1,18 @@ +// This file tecnds to make expalin to use backup plan, just for testing +db.foo.drop() + +let bulk = db.foo.initializeUnorderedBulkOp(); +for (let i = 0; i < 100000; ++i) { + bulk.insert({_id: i, x: i, y: i}); +} + +bulk.execute(); +db.foo.ensureIndex({x: 1}) + +//2. Configure log level and lower the sort bytes limit: +db.setLogLevel(5, "query"); +db.adminCommand({setParameter: 1, internalQueryExecMaxBlockingSortBytes: 100}); + +//3. Run the query that will have lots of matching documents that are not sorted from the x index, and lots of non-matching documents that are sorted from the _id index: +db.foo.find({x: {$gte: 90000}}).sort({_id: 1}).explain(true) + diff --git a/src/mongo/db/exec/multi_plan.cpp b/src/mongo/db/exec/multi_plan.cpp index 5dc4bd0b86340..61e07b064108b 100644 --- a/src/mongo/db/exec/multi_plan.cpp +++ b/src/mongo/db/exec/multi_plan.cpp @@ -132,7 +132,7 @@ PlanStage::StageState MultiPlanStage::doWork(WorkingSetID* out) { _collection->infoCache()->getPlanCache()->remove(*_query).transitional_ignore(); _bestPlanIdx = _backupPlanIdx; - _backupPlanIdx = kNoSuchPlan; + // _backupPlanIdx = kNoSuchPlan; return _candidates[_bestPlanIdx].root->work(out); } @@ -245,6 +245,7 @@ Status MultiPlanStage::pickBestPlan(PlanYieldPolicy* yieldPolicy) { _backupPlanIdx = kNoSuchPlan; if (bestSolution->hasBlockingStage && (0 == alreadyProduced.size())) { LOG(5) << "Winner has blocking stage, looking for backup plan..."; + for (size_t ix = 0; ix < _candidates.size(); ++ix) { if (!_candidates[ix].solution->hasBlockingStage) { LOG(5) << "Candidate " << ix << " is backup child"; @@ -452,6 +453,9 @@ void MultiPlanStage::doInvalidate(OperationContext* opCtx, bool MultiPlanStage::hasBackupPlan() const { return kNoSuchPlan != _backupPlanIdx; } +int MultiPlanStage::backupPlanIdx() const { + return _backupPlanIdx; +} bool MultiPlanStage::bestPlanChosen() const { return kNoSuchPlan != _bestPlanIdx; diff --git a/src/mongo/db/exec/multi_plan.h b/src/mongo/db/exec/multi_plan.h index 5d6369e3c0dc4..500052055a762 100644 --- a/src/mongo/db/exec/multi_plan.h +++ b/src/mongo/db/exec/multi_plan.h @@ -132,6 +132,9 @@ class MultiPlanStage final : public PlanStage { /** Return the index of the best plan chosen, for testing */ int bestPlanIdx() const; + /** Return the index of the backup plan chosen, for testing */ + int backupPlanIdx() const; + /** * Returns the QuerySolution for the best plan, or NULL if no best plan * diff --git a/src/mongo/db/pipeline/document_source_sort.h b/src/mongo/db/pipeline/document_source_sort.h index 36194bd2bfca4..d2ad1cd9ec408 100644 --- a/src/mongo/db/pipeline/document_source_sort.h +++ b/src/mongo/db/pipeline/document_source_sort.h @@ -74,7 +74,7 @@ class DocumentSourceSort final : public DocumentSource, public SplittableDocumen : ChangeStreamRequirement::kBlacklist); // Can't swap with a $match if a limit has been absorbed, as $match can't swap with $limit. - constraints.canSwapWithMatch = !_limitSrc; + constraints.canSwapWithMatch = !limitSrc; return constraints; } @@ -144,7 +144,7 @@ class DocumentSourceSort final : public DocumentSource, public SplittableDocumen }; boost::intrusive_ptr getLimitSrc() const { - return _limitSrc; + return limitSrc; } protected: @@ -242,8 +242,8 @@ class DocumentSourceSort final : public DocumentSource, public SplittableDocumen * the smallest limit. */ void setLimitSrc(boost::intrusive_ptr limit) { - if (!_limitSrc || limit->getLimit() < _limitSrc->getLimit()) { - _limitSrc = limit; + if (!limitSrc || limit->getLimit() < limitSrc->getLimit()) { + limitSrc = limit; } } @@ -258,7 +258,7 @@ class DocumentSourceSort final : public DocumentSource, public SplittableDocumen // The set of paths on which we're sorting. std::set _paths; - boost::intrusive_ptr _limitSrc; + boost::intrusive_ptr limitSrc; uint64_t _maxMemoryUsageBytes; bool _done; diff --git a/src/mongo/db/query/explain.cpp b/src/mongo/db/query/explain.cpp index ae985487ffbb3..419bb6ad6c31f 100644 --- a/src/mongo/db/query/explain.cpp +++ b/src/mongo/db/query/explain.cpp @@ -767,6 +767,14 @@ void Explain::generateExecutionInfo(PlanExecutor* exec, } BSONObjBuilder execBob(out->subobjStart("executionStats")); + const auto mps = getMultiPlanStage(exec->getRootStage()); + BSONObjBuilder bob; + int i = static_cast(mps->backupPlanIdx()); + if (i >= 0) { + bob.append("backupPlanUsed", true); + // out->append("backupPlanUsed", true); + } + // If there is an execution error while running the query, the error is reported under // the "executionStats" section and the explain as a whole succeeds. execBob.append("executionSuccess", executePlanStatus.isOK()); From 6316125bc59ae1226e763a3c3304bf83614cdb8e Mon Sep 17 00:00:00 2001 From: Fox Lady Date: Fri, 23 Mar 2018 21:59:17 -0400 Subject: [PATCH 04/14] Added backup plan incator to explain output --- jstests/core/server32721.js | 6 ++++ src/mongo/db/exec/multi_plan.cpp | 6 +++- src/mongo/db/exec/multi_plan.h | 8 ++++- src/mongo/db/query/explain.cpp | 52 +++++++++++++++++++++++++++----- 4 files changed, 62 insertions(+), 10 deletions(-) diff --git a/jstests/core/server32721.js b/jstests/core/server32721.js index 42750b71ba82e..9ae031d4199ea 100644 --- a/jstests/core/server32721.js +++ b/jstests/core/server32721.js @@ -15,4 +15,10 @@ db.adminCommand({setParameter: 1, internalQueryExecMaxBlockingSortBytes: 100}); //3. Run the query that will have lots of matching documents that are not sorted from the x index, and lots of non-matching documents that are sorted from the _id index: db.foo.find({x: {$gte: 90000}}).sort({_id: 1}).explain(true) +db.foo.find({x: 1}).sort({_id: 1}).explain(true) +const auto mps = getMultiPlanStage(exec->getRootStage()); +int i = static_cast(mps->originalWinningPlanIdx()); +if (i >= 0) { + plannerBob.append("backupPlanUsed", true); +} \ No newline at end of file diff --git a/src/mongo/db/exec/multi_plan.cpp b/src/mongo/db/exec/multi_plan.cpp index 61e07b064108b..804524bf403be 100644 --- a/src/mongo/db/exec/multi_plan.cpp +++ b/src/mongo/db/exec/multi_plan.cpp @@ -131,8 +131,9 @@ PlanStage::StageState MultiPlanStage::doWork(WorkingSetID* out) { _collection->infoCache()->getPlanCache()->remove(*_query).transitional_ignore(); + _originalWinningPlanIdx = _bestPlanIdx; _bestPlanIdx = _backupPlanIdx; - // _backupPlanIdx = kNoSuchPlan; + _backupPlanIdx = kNoSuchPlan; return _candidates[_bestPlanIdx].root->work(out); } @@ -456,6 +457,9 @@ bool MultiPlanStage::hasBackupPlan() const { int MultiPlanStage::backupPlanIdx() const { return _backupPlanIdx; } +int MultiPlanStage::originalWinningPlanIdx() const { + return _originalWinningPlanIdx; +} bool MultiPlanStage::bestPlanChosen() const { return kNoSuchPlan != _bestPlanIdx; diff --git a/src/mongo/db/exec/multi_plan.h b/src/mongo/db/exec/multi_plan.h index 500052055a762..1bf9e8fcd1e43 100644 --- a/src/mongo/db/exec/multi_plan.h +++ b/src/mongo/db/exec/multi_plan.h @@ -135,6 +135,8 @@ class MultiPlanStage final : public PlanStage { /** Return the index of the backup plan chosen, for testing */ int backupPlanIdx() const; + /** Return the index of the backup plan chosen, for testing */ + int originalWinningPlanIdx() const; /** * Returns the QuerySolution for the best plan, or NULL if no best plan * @@ -201,10 +203,14 @@ class MultiPlanStage final : public PlanStage { // uses -1 / kNoSuchPlan when best plan is not (yet) known int _bestPlanIdx; - // index into _candidates, of the backup plan for sort + // index into _candidates, of the backup of the plan competition // uses -1 / kNoSuchPlan when best plan is not (yet) known int _backupPlanIdx; + // index into _candidates, of the original winner of the plan competition + // uses -1 / kNoSuchPlan when best plan is not (yet) known + int _originalWinningPlanIdx; + // Set if this MultiPlanStage cannot continue, and the query must fail. This can happen in // two ways. The first is that all candidate plans fail. Note that one plan can fail // during normal execution of the plan competition. Here is an example: diff --git a/src/mongo/db/query/explain.cpp b/src/mongo/db/query/explain.cpp index 419bb6ad6c31f..5a0b0056a75fa 100644 --- a/src/mongo/db/query/explain.cpp +++ b/src/mongo/db/query/explain.cpp @@ -289,6 +289,28 @@ unique_ptr getWinningPlanStatsTree(const PlanExecutor* exec) { : std::move(exec->getRootStage()->getStats()); } +/** + * Get PlanExecutor's original winning plan stats tree. + */ +std::vector> getOriginalWinningPlanStatsTree(PlanExecutor* exec) { + // Inspect the tree to see if there is a MultiPlanStage. Plan selection has already happened at + // this point, since we have a PlanExecutor. + const auto mps = getMultiPlanStage(exec->getRootStage()); + std::vector> res; + + // Get the stats from the trial period for all the plans. + if (mps) { + const auto mpsStats = mps->getStats(); + for (size_t i = 0; i < mpsStats->children.size(); ++i) { + if (i != static_cast(mps->originalWinningPlanIdx())) { + res.emplace_back(std::move(mpsStats->children[i])); + } + } + } + + return res; +} + } // namespace namespace mongo { @@ -644,6 +666,12 @@ void Explain::generatePlannerInfo(PlanExecutor* exec, BSONObjBuilder plannerBob(out->subobjStart("queryPlanner")); plannerBob.append("plannerVersion", QueryPlanner::kPlannerVersion); + + const auto mps = getMultiPlanStage(exec->getRootStage()); + int i = static_cast(mps->originalWinningPlanIdx()); + if (i >= 0) { + plannerBob.append("backupPlanUsed", true); + } plannerBob.append("namespace", exec->nss().ns()); // Find whether there is an index filter set for the query shape. The 'indexFilterSet' @@ -682,10 +710,26 @@ void Explain::generatePlannerInfo(PlanExecutor* exec, // Genenerate array of rejected plans. const vector> rejectedStats = getRejectedPlansTrialStats(exec); BSONArrayBuilder allPlansBob(plannerBob.subarrayStart("rejectedPlans")); + // BSONArrayBuilder originalWinningPlansBob(plannerBob.subarrayStart("originalWinningPlan")); for (size_t i = 0; i < rejectedStats.size(); i++) { BSONObjBuilder childBob(allPlansBob.subobjStart()); statsToBSON(*rejectedStats[i], &childBob, ExplainOptions::Verbosity::kQueryPlanner); } + + // // Genenerate array of rejected plans. + // const vector> originalPlanStats = getRejectedPlansTrialStats(exec); + // BSONArrayBuilder originalWinningPlansBob(plannerBob.subarrayStart("originalWinningPlan")); + // for (size_t i = 0; i < originalPlanStats.size(); i++) { + // BSONObjBuilder childBob(originalWinningPlansBob.subobjStart()); + // statsToBSON(*originalPlanStats[i], &childBob, ExplainOptions::Verbosity::kQueryPlanner); + // } + + // Generate array of original winning plan + BSONObjBuilder originalWinningPlanBob(plannerBob.subobjStart("originalWinningPlan")); + const auto originalWinnerStats = getOriginalWinningPlanStatsTree(exec); + //statsToBSON(*originalWinnerStats.get(), &originalWinningPlanBob, ExplainOptions::Verbosity::kQueryPlanner); + originalWinningPlanBob.doneFast(); + allPlansBob.doneFast(); plannerBob.doneFast(); @@ -767,14 +811,6 @@ void Explain::generateExecutionInfo(PlanExecutor* exec, } BSONObjBuilder execBob(out->subobjStart("executionStats")); - const auto mps = getMultiPlanStage(exec->getRootStage()); - BSONObjBuilder bob; - int i = static_cast(mps->backupPlanIdx()); - if (i >= 0) { - bob.append("backupPlanUsed", true); - // out->append("backupPlanUsed", true); - } - // If there is an execution error while running the query, the error is reported under // the "executionStats" section and the explain as a whole succeeds. execBob.append("executionSuccess", executePlanStatus.isOK()); From 8731c80583eaad8a20f734301d9c717a4e488893 Mon Sep 17 00:00:00 2001 From: Fox Lady Date: Sat, 24 Mar 2018 09:34:57 -0400 Subject: [PATCH 05/14] Added OriginalWinningPlan to explain output --- jstests/core/server32721.js | 7 ------ src/mongo/db/exec/multi_plan.cpp | 3 ++- src/mongo/db/query/explain.cpp | 37 ++++++-------------------------- 3 files changed, 9 insertions(+), 38 deletions(-) diff --git a/jstests/core/server32721.js b/jstests/core/server32721.js index 9ae031d4199ea..ed15c10556d71 100644 --- a/jstests/core/server32721.js +++ b/jstests/core/server32721.js @@ -15,10 +15,3 @@ db.adminCommand({setParameter: 1, internalQueryExecMaxBlockingSortBytes: 100}); //3. Run the query that will have lots of matching documents that are not sorted from the x index, and lots of non-matching documents that are sorted from the _id index: db.foo.find({x: {$gte: 90000}}).sort({_id: 1}).explain(true) -db.foo.find({x: 1}).sort({_id: 1}).explain(true) - -const auto mps = getMultiPlanStage(exec->getRootStage()); -int i = static_cast(mps->originalWinningPlanIdx()); -if (i >= 0) { - plannerBob.append("backupPlanUsed", true); -} \ No newline at end of file diff --git a/src/mongo/db/exec/multi_plan.cpp b/src/mongo/db/exec/multi_plan.cpp index 804524bf403be..a2b525d26b5ec 100644 --- a/src/mongo/db/exec/multi_plan.cpp +++ b/src/mongo/db/exec/multi_plan.cpp @@ -71,6 +71,7 @@ MultiPlanStage::MultiPlanStage(OperationContext* opCtx, _query(cq), _bestPlanIdx(kNoSuchPlan), _backupPlanIdx(kNoSuchPlan), + _originalWinningPlanIdx(kNoSuchPlan), _failure(false), _failureCount(0), _statusMemberId(WorkingSet::INVALID_ID) { @@ -128,10 +129,10 @@ PlanStage::StageState MultiPlanStage::doWork(WorkingSetID* out) { // cached plan runner to fall back on a different solution // if the best solution fails. Alternatively we could try to // defer cache insertion to be after the first produced result. + _originalWinningPlanIdx = _bestPlanIdx; _collection->infoCache()->getPlanCache()->remove(*_query).transitional_ignore(); - _originalWinningPlanIdx = _bestPlanIdx; _bestPlanIdx = _backupPlanIdx; _backupPlanIdx = kNoSuchPlan; diff --git a/src/mongo/db/query/explain.cpp b/src/mongo/db/query/explain.cpp index 5a0b0056a75fa..5fa1bf2faac31 100644 --- a/src/mongo/db/query/explain.cpp +++ b/src/mongo/db/query/explain.cpp @@ -289,28 +289,15 @@ unique_ptr getWinningPlanStatsTree(const PlanExecutor* exec) { : std::move(exec->getRootStage()->getStats()); } + /** * Get PlanExecutor's original winning plan stats tree. */ -std::vector> getOriginalWinningPlanStatsTree(PlanExecutor* exec) { - // Inspect the tree to see if there is a MultiPlanStage. Plan selection has already happened at - // this point, since we have a PlanExecutor. - const auto mps = getMultiPlanStage(exec->getRootStage()); - std::vector> res; - - // Get the stats from the trial period for all the plans. - if (mps) { - const auto mpsStats = mps->getStats(); - for (size_t i = 0; i < mpsStats->children.size(); ++i) { - if (i != static_cast(mps->originalWinningPlanIdx())) { - res.emplace_back(std::move(mpsStats->children[i])); - } - } - } - - return res; +unique_ptr getOriginalWinningPlanStatsTree(const PlanExecutor* exec) { + MultiPlanStage* mps = getMultiPlanStage(exec->getRootStage()); + return mps ? std::move(mps->getStats()->children[mps->originalWinningPlanIdx()]) + : std::move(exec->getRootStage()->getStats()); } - } // namespace namespace mongo { @@ -710,28 +697,18 @@ void Explain::generatePlannerInfo(PlanExecutor* exec, // Genenerate array of rejected plans. const vector> rejectedStats = getRejectedPlansTrialStats(exec); BSONArrayBuilder allPlansBob(plannerBob.subarrayStart("rejectedPlans")); - // BSONArrayBuilder originalWinningPlansBob(plannerBob.subarrayStart("originalWinningPlan")); for (size_t i = 0; i < rejectedStats.size(); i++) { BSONObjBuilder childBob(allPlansBob.subobjStart()); statsToBSON(*rejectedStats[i], &childBob, ExplainOptions::Verbosity::kQueryPlanner); } - - // // Genenerate array of rejected plans. - // const vector> originalPlanStats = getRejectedPlansTrialStats(exec); - // BSONArrayBuilder originalWinningPlansBob(plannerBob.subarrayStart("originalWinningPlan")); - // for (size_t i = 0; i < originalPlanStats.size(); i++) { - // BSONObjBuilder childBob(originalWinningPlansBob.subobjStart()); - // statsToBSON(*originalPlanStats[i], &childBob, ExplainOptions::Verbosity::kQueryPlanner); - // } + allPlansBob.doneFast(); // Generate array of original winning plan BSONObjBuilder originalWinningPlanBob(plannerBob.subobjStart("originalWinningPlan")); const auto originalWinnerStats = getOriginalWinningPlanStatsTree(exec); - //statsToBSON(*originalWinnerStats.get(), &originalWinningPlanBob, ExplainOptions::Verbosity::kQueryPlanner); + statsToBSON(*originalWinnerStats.get(), &originalWinningPlanBob, ExplainOptions::Verbosity::kQueryPlanner); originalWinningPlanBob.doneFast(); - allPlansBob.doneFast(); - plannerBob.doneFast(); } From 3ec2451c4189add943c4e5797433efb9467e1d97 Mon Sep 17 00:00:00 2001 From: Fox Lady Date: Sat, 24 Mar 2018 09:38:10 -0400 Subject: [PATCH 06/14] Removed changes for another ticket --- src/mongo/bson/bsonelement.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mongo/bson/bsonelement.h b/src/mongo/bson/bsonelement.h index a5386d5cf3f36..43cb8aa206bdd 100644 --- a/src/mongo/bson/bsonelement.h +++ b/src/mongo/bson/bsonelement.h @@ -463,7 +463,7 @@ class BSONElement { /** Get binary data. Element must be of type BinData. Handles type 2 */ const char* binDataClean(int& len) const { // BinData: - if (type() == BinData && binDataType() != ByteArrayDeprecated) { + if (binDataType() != ByteArrayDeprecated) { return binData(len); } else { // Skip extra size From 9c66dabf7ae76b793eeb0f9eb0d2473dca770a58 Mon Sep 17 00:00:00 2001 From: Fox Lady Date: Tue, 27 Mar 2018 11:55:15 -0400 Subject: [PATCH 07/14] Added if condition to prevent segmentation fault --- jstests/core/explain_use_backup_plan.js | 24 ++++++++++++++++++++++++ jstests/core/server32721.js | 17 ----------------- src/mongo/db/query/explain.cpp | 20 +++++++++++++------- 3 files changed, 37 insertions(+), 24 deletions(-) create mode 100644 jstests/core/explain_use_backup_plan.js delete mode 100644 jstests/core/server32721.js diff --git a/jstests/core/explain_use_backup_plan.js b/jstests/core/explain_use_backup_plan.js new file mode 100644 index 0000000000000..d93dca383c3d9 --- /dev/null +++ b/jstests/core/explain_use_backup_plan.js @@ -0,0 +1,24 @@ +// Test that the explain will use backup plan if the original winning plan ran out of memory in the "executionStats" mode +// This test was designed to reproduce SERVER-32721" +(function() { + "use strict"; + + db.foo.drop() + let bulk = db.foo.initializeUnorderedBulkOp(); + for (let i = 0; i < 100000; ++i) { + bulk.insert({_id: i, x: i, y: i}); + } + + bulk.execute(); + db.foo.ensureIndex({x: 1}) + + // Configure log level and lower the sort bytes limit. + db.setLogLevel(5, "query"); + db.adminCommand({setParameter: 1, internalQueryExecMaxBlockingSortBytes: 100}); + + // This query will not use the backup plan, hence it generates only two stages: winningPlan and rejectedPlans + assert.commandWorked(db.foo.find({x: {$gte: 90}}).sort({_id: 1}).explain(true)) + + // This query will use backup plan, the exaplin output for this query will generate three stages: winningPlan, rejectedPlans and originalWinningPlan + assert.commandWorked(db.foo.find({x: {$gte: 90000}}).sort({_id: 1}).explain(true)) +}()); \ No newline at end of file diff --git a/jstests/core/server32721.js b/jstests/core/server32721.js deleted file mode 100644 index ed15c10556d71..0000000000000 --- a/jstests/core/server32721.js +++ /dev/null @@ -1,17 +0,0 @@ -// This file tecnds to make expalin to use backup plan, just for testing -db.foo.drop() - -let bulk = db.foo.initializeUnorderedBulkOp(); -for (let i = 0; i < 100000; ++i) { - bulk.insert({_id: i, x: i, y: i}); -} - -bulk.execute(); -db.foo.ensureIndex({x: 1}) - -//2. Configure log level and lower the sort bytes limit: -db.setLogLevel(5, "query"); -db.adminCommand({setParameter: 1, internalQueryExecMaxBlockingSortBytes: 100}); - -//3. Run the query that will have lots of matching documents that are not sorted from the x index, and lots of non-matching documents that are sorted from the _id index: -db.foo.find({x: {$gte: 90000}}).sort({_id: 1}).explain(true) diff --git a/src/mongo/db/query/explain.cpp b/src/mongo/db/query/explain.cpp index 5fa1bf2faac31..d46158c32401b 100644 --- a/src/mongo/db/query/explain.cpp +++ b/src/mongo/db/query/explain.cpp @@ -655,10 +655,14 @@ void Explain::generatePlannerInfo(PlanExecutor* exec, plannerBob.append("plannerVersion", QueryPlanner::kPlannerVersion); const auto mps = getMultiPlanStage(exec->getRootStage()); - int i = static_cast(mps->originalWinningPlanIdx()); - if (i >= 0) { + int originaWinningPlanIdx = static_cast(mps->originalWinningPlanIdx()); + + if (originaWinningPlanIdx > -1) { plannerBob.append("backupPlanUsed", true); + } else { + plannerBob.append("backupPlanUsed", false); } + plannerBob.append("namespace", exec->nss().ns()); // Find whether there is an index filter set for the query shape. The 'indexFilterSet' @@ -703,11 +707,13 @@ void Explain::generatePlannerInfo(PlanExecutor* exec, } allPlansBob.doneFast(); - // Generate array of original winning plan - BSONObjBuilder originalWinningPlanBob(plannerBob.subobjStart("originalWinningPlan")); - const auto originalWinnerStats = getOriginalWinningPlanStatsTree(exec); - statsToBSON(*originalWinnerStats.get(), &originalWinningPlanBob, ExplainOptions::Verbosity::kQueryPlanner); - originalWinningPlanBob.doneFast(); + if (originaWinningPlanIdx > -1) { + // Generate array of original winning plan + BSONObjBuilder originalWinningPlanBob(plannerBob.subobjStart("originalWinningPlan")); + const auto originalWinnerStats = getOriginalWinningPlanStatsTree(exec); + statsToBSON(*originalWinnerStats.get(), &originalWinningPlanBob, ExplainOptions::Verbosity::kQueryPlanner); + originalWinningPlanBob.doneFast(); + } plannerBob.doneFast(); } From fa97a62baa377b31916783a34666c4c398ab8c23 Mon Sep 17 00:00:00 2001 From: Fox Lady Date: Wed, 28 Mar 2018 08:32:34 -0400 Subject: [PATCH 08/14] Formatted Files --- jstests/core/explain_use_backup_plan.js | 36 ++++++++++++++----------- src/mongo/db/exec/multi_plan.cpp | 2 +- src/mongo/db/query/explain.cpp | 6 +++-- 3 files changed, 25 insertions(+), 19 deletions(-) diff --git a/jstests/core/explain_use_backup_plan.js b/jstests/core/explain_use_backup_plan.js index d93dca383c3d9..6bf12d81673da 100644 --- a/jstests/core/explain_use_backup_plan.js +++ b/jstests/core/explain_use_backup_plan.js @@ -1,24 +1,28 @@ -// Test that the explain will use backup plan if the original winning plan ran out of memory in the "executionStats" mode +// Test that the explain will use backup plan if the original winning plan ran out of memory in the +// "executionStats" mode // This test was designed to reproduce SERVER-32721" (function() { - "use strict"; + "use strict"; - db.foo.drop() - let bulk = db.foo.initializeUnorderedBulkOp(); - for (let i = 0; i < 100000; ++i) { - bulk.insert({_id: i, x: i, y: i}); - } + db.foo.drop() let bulk = db.foo.initializeUnorderedBulkOp(); + for (let i = 0; i < 100000; ++i) { + bulk.insert({_id: i, x: i, y: i}); + } - bulk.execute(); - db.foo.ensureIndex({x: 1}) + bulk.execute(); + db.foo + .ensureIndex({x: 1}) - // Configure log level and lower the sort bytes limit. - db.setLogLevel(5, "query"); - db.adminCommand({setParameter: 1, internalQueryExecMaxBlockingSortBytes: 100}); + // Configure log level and lower the sort bytes limit. + db.setLogLevel(5, "query"); + db.adminCommand({setParameter: 1, internalQueryExecMaxBlockingSortBytes: 100}); - // This query will not use the backup plan, hence it generates only two stages: winningPlan and rejectedPlans - assert.commandWorked(db.foo.find({x: {$gte: 90}}).sort({_id: 1}).explain(true)) + // This query will not use the backup plan, hence it generates only two stages: winningPlan and + // rejectedPlans + assert + .commandWorked(db.foo.find({x: {$gte: 90}}).sort({_id: 1}).explain(true)) - // This query will use backup plan, the exaplin output for this query will generate three stages: winningPlan, rejectedPlans and originalWinningPlan - assert.commandWorked(db.foo.find({x: {$gte: 90000}}).sort({_id: 1}).explain(true)) + // This query will use backup plan, the exaplin output for this query will generate three + // stages: winningPlan, rejectedPlans and originalWinningPlan + assert.commandWorked(db.foo.find({x: {$gte: 90000}}).sort({_id: 1}).explain(true)) }()); \ No newline at end of file diff --git a/src/mongo/db/exec/multi_plan.cpp b/src/mongo/db/exec/multi_plan.cpp index a2b525d26b5ec..7161e2ba8d1d6 100644 --- a/src/mongo/db/exec/multi_plan.cpp +++ b/src/mongo/db/exec/multi_plan.cpp @@ -247,7 +247,7 @@ Status MultiPlanStage::pickBestPlan(PlanYieldPolicy* yieldPolicy) { _backupPlanIdx = kNoSuchPlan; if (bestSolution->hasBlockingStage && (0 == alreadyProduced.size())) { LOG(5) << "Winner has blocking stage, looking for backup plan..."; - + for (size_t ix = 0; ix < _candidates.size(); ++ix) { if (!_candidates[ix].solution->hasBlockingStage) { LOG(5) << "Candidate " << ix << " is backup child"; diff --git a/src/mongo/db/query/explain.cpp b/src/mongo/db/query/explain.cpp index d46158c32401b..4c741e50e9fc8 100644 --- a/src/mongo/db/query/explain.cpp +++ b/src/mongo/db/query/explain.cpp @@ -662,7 +662,7 @@ void Explain::generatePlannerInfo(PlanExecutor* exec, } else { plannerBob.append("backupPlanUsed", false); } - + plannerBob.append("namespace", exec->nss().ns()); // Find whether there is an index filter set for the query shape. The 'indexFilterSet' @@ -711,7 +711,9 @@ void Explain::generatePlannerInfo(PlanExecutor* exec, // Generate array of original winning plan BSONObjBuilder originalWinningPlanBob(plannerBob.subobjStart("originalWinningPlan")); const auto originalWinnerStats = getOriginalWinningPlanStatsTree(exec); - statsToBSON(*originalWinnerStats.get(), &originalWinningPlanBob, ExplainOptions::Verbosity::kQueryPlanner); + statsToBSON(*originalWinnerStats.get(), + &originalWinningPlanBob, + ExplainOptions::Verbosity::kQueryPlanner); originalWinningPlanBob.doneFast(); } From d0f1174d64b63c078c969e8e8a180759f368391d Mon Sep 17 00:00:00 2001 From: Fox Lady Date: Tue, 24 Apr 2018 11:58:56 -0400 Subject: [PATCH 09/14] Add Helper Function for jstest --- jstests/core/explain_use_backup_plan.js | 29 ++++++++++++++----------- jstests/libs/analyze_plan.js | 16 ++++++++++++++ src/mongo/db/exec/multi_plan.cpp | 1 - src/mongo/db/exec/multi_plan.h | 17 ++++++++++----- src/mongo/db/query/explain.cpp | 11 ++++++---- 5 files changed, 51 insertions(+), 23 deletions(-) diff --git a/jstests/core/explain_use_backup_plan.js b/jstests/core/explain_use_backup_plan.js index 6bf12d81673da..1a48e5affbdca 100644 --- a/jstests/core/explain_use_backup_plan.js +++ b/jstests/core/explain_use_backup_plan.js @@ -4,25 +4,28 @@ (function() { "use strict"; - db.foo.drop() let bulk = db.foo.initializeUnorderedBulkOp(); - for (let i = 0; i < 100000; ++i) { + db.foo.drop(); + let bulk = db.foo.initializeUnorderedBulkOp(); + + for (let i = 0; i < 1000; ++i) { bulk.insert({_id: i, x: i, y: i}); } bulk.execute(); - db.foo - .ensureIndex({x: 1}) + db.foo.ensureIndex({x: 1}); - // Configure log level and lower the sort bytes limit. - db.setLogLevel(5, "query"); + // Configure log level and lower the sort bytes limit. + db.setLogLevel(5, "query"); db.adminCommand({setParameter: 1, internalQueryExecMaxBlockingSortBytes: 100}); - // This query will not use the backup plan, hence it generates only two stages: winningPlan and - // rejectedPlans - assert - .commandWorked(db.foo.find({x: {$gte: 90}}).sort({_id: 1}).explain(true)) - // This query will use backup plan, the exaplin output for this query will generate three - // stages: winningPlan, rejectedPlans and originalWinningPlan - assert.commandWorked(db.foo.find({x: {$gte: 90000}}).sort({_id: 1}).explain(true)) + + + // // This query will not use the backup plan, hence it generates only two stages: winningPlan and + // // rejectedPlans + // assert.commandWorked(db.foo.find({x: {$gte: 90}}).sort({_id: 1}).explain(true)); + + // // This query will use backup plan, the exaplin output for this query will generate three + // // stages: winningPlan, rejectedPlans and originalWinningPlan + // assert.commandWorked(db.foo.find({x: {$gte: 900}}).sort({_id: 1}).explain(true)); }()); \ No newline at end of file diff --git a/jstests/libs/analyze_plan.js b/jstests/libs/analyze_plan.js index 6a719d611f65e..d8c2cbdb6d583 100644 --- a/jstests/libs/analyze_plan.js +++ b/jstests/libs/analyze_plan.js @@ -287,3 +287,19 @@ function assertCoveredQueryAndCount({collection, query, project, count}) { "Winning plan for count was not covered: " + tojson(explain.queryPlanner.winningPlan)); assertExplainCount({explainResults: explain, expectedCount: count}); } + +/** + * Returns true if a backup plan is used. + */ +function backupPlanUsed(root) { + function sectionHasOriginalWinningPlan(explainSection) { + assert(explainSection.hasOwnProperty("originalWinningPlan"), tojson(explainSection)); + return explainSection.originalWinningPlan.length !== 0; + } + + function cursorStageHasOriginalWinningPlan(cursorStage) { + assert(cursorStage.hasOwnProperty("$cursor"), tojson(cursorStage)); + assert(cursorStage.$cursor.hasOwnProperty("queryPlanner"), tojson(cursorStage)); + return sectionHasOriginalWinningPlan(cursorStage.$cursor.queryPlanner); + } +} \ No newline at end of file diff --git a/src/mongo/db/exec/multi_plan.cpp b/src/mongo/db/exec/multi_plan.cpp index 7161e2ba8d1d6..2c5e742de7da6 100644 --- a/src/mongo/db/exec/multi_plan.cpp +++ b/src/mongo/db/exec/multi_plan.cpp @@ -247,7 +247,6 @@ Status MultiPlanStage::pickBestPlan(PlanYieldPolicy* yieldPolicy) { _backupPlanIdx = kNoSuchPlan; if (bestSolution->hasBlockingStage && (0 == alreadyProduced.size())) { LOG(5) << "Winner has blocking stage, looking for backup plan..."; - for (size_t ix = 0; ix < _candidates.size(); ++ix) { if (!_candidates[ix].solution->hasBlockingStage) { LOG(5) << "Candidate " << ix << " is backup child"; diff --git a/src/mongo/db/exec/multi_plan.h b/src/mongo/db/exec/multi_plan.h index 1bf9e8fcd1e43..dfdeddb1688bf 100644 --- a/src/mongo/db/exec/multi_plan.h +++ b/src/mongo/db/exec/multi_plan.h @@ -129,14 +129,21 @@ class MultiPlanStage final : public PlanStage { /** Return true if a best plan has been chosen */ bool bestPlanChosen() const; - /** Return the index of the best plan chosen, for testing */ + /* + * Return the index of the best plan chosen + */ int bestPlanIdx() const; - /** Return the index of the backup plan chosen, for testing */ + /** + * Return the index of the backup plan chosen + */ int backupPlanIdx() const; - /** Return the index of the backup plan chosen, for testing */ + /** + * Return the index of the original winning plan chosen + */ int originalWinningPlanIdx() const; + /** * Returns the QuerySolution for the best plan, or NULL if no best plan * @@ -207,8 +214,8 @@ class MultiPlanStage final : public PlanStage { // uses -1 / kNoSuchPlan when best plan is not (yet) known int _backupPlanIdx; - // index into _candidates, of the original winner of the plan competition - // uses -1 / kNoSuchPlan when best plan is not (yet) known + // Index into _candidates, of the original winner of the plan which can be used if a blocking plan .fails + // This is set to 'kNoSuchPlan' if there is no backup plan, or when it is not (yet) known int _originalWinningPlanIdx; // Set if this MultiPlanStage cannot continue, and the query must fail. This can happen in diff --git a/src/mongo/db/query/explain.cpp b/src/mongo/db/query/explain.cpp index 4c741e50e9fc8..634225a0556f5 100644 --- a/src/mongo/db/query/explain.cpp +++ b/src/mongo/db/query/explain.cpp @@ -291,7 +291,10 @@ unique_ptr getWinningPlanStatsTree(const PlanExecutor* exec) { /** - * Get PlanExecutor's original winning plan stats tree. + * Returns the root of the roginal winning plan used by 'exec'. + * This might be different than the final winning plan in the case that the MultiPlanStage selected a + * blocking plan which failed, and fell back to a non-blocking plan instead. + * If there is no MultiPlanStage in the tree, returns the root stage of 'exec' */ unique_ptr getOriginalWinningPlanStatsTree(const PlanExecutor* exec) { MultiPlanStage* mps = getMultiPlanStage(exec->getRootStage()); @@ -655,9 +658,9 @@ void Explain::generatePlannerInfo(PlanExecutor* exec, plannerBob.append("plannerVersion", QueryPlanner::kPlannerVersion); const auto mps = getMultiPlanStage(exec->getRootStage()); - int originaWinningPlanIdx = static_cast(mps->originalWinningPlanIdx()); + const bool usedBackupPlan = ((mps->originalWinningPlanIdx()) > -1); - if (originaWinningPlanIdx > -1) { + if (usedBackupPlan) { plannerBob.append("backupPlanUsed", true); } else { plannerBob.append("backupPlanUsed", false); @@ -707,7 +710,7 @@ void Explain::generatePlannerInfo(PlanExecutor* exec, } allPlansBob.doneFast(); - if (originaWinningPlanIdx > -1) { + if (usedBackupPlan) { // Generate array of original winning plan BSONObjBuilder originalWinningPlanBob(plannerBob.subobjStart("originalWinningPlan")); const auto originalWinnerStats = getOriginalWinningPlanStatsTree(exec); From e2f24835c434940214195b1f75a7af02cbca7a70 Mon Sep 17 00:00:00 2001 From: Fox Lady Date: Tue, 24 Apr 2018 13:14:36 -0400 Subject: [PATCH 10/14] Modified Testing Function --- jstests/core/explain_use_backup_plan.js | 30 ++++++++++++++++--------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/jstests/core/explain_use_backup_plan.js b/jstests/core/explain_use_backup_plan.js index 1a48e5affbdca..3854f870a0cdf 100644 --- a/jstests/core/explain_use_backup_plan.js +++ b/jstests/core/explain_use_backup_plan.js @@ -1,13 +1,16 @@ // Test that the explain will use backup plan if the original winning plan ran out of memory in the // "executionStats" mode // This test was designed to reproduce SERVER-32721" + +load("jstests/libs/analyze_plan.js"); + (function() { "use strict"; db.foo.drop(); let bulk = db.foo.initializeUnorderedBulkOp(); - for (let i = 0; i < 1000; ++i) { + for (let i = 0; i < 100000; ++i) { bulk.insert({_id: i, x: i, y: i}); } @@ -17,15 +20,20 @@ // Configure log level and lower the sort bytes limit. db.setLogLevel(5, "query"); db.adminCommand({setParameter: 1, internalQueryExecMaxBlockingSortBytes: 100}); - - - - // // This query will not use the backup plan, hence it generates only two stages: winningPlan and - // // rejectedPlans - // assert.commandWorked(db.foo.find({x: {$gte: 90}}).sort({_id: 1}).explain(true)); - - // // This query will use backup plan, the exaplin output for this query will generate three - // // stages: winningPlan, rejectedPlans and originalWinningPlan - // assert.commandWorked(db.foo.find({x: {$gte: 900}}).sort({_id: 1}).explain(true)); + function checkIfBackupPlanUsed() { + // This query will not use the backup plan, hence it generates only two stages: winningPlan and + // rejectedPlans + var test1 = db.foo.find({x: {$gte: 90}}).sort({_id: 1}).explain(true); + assert(!backupPlanUsed(test1), "test1 did not use any backup plan: " + + tojson(test1)); + + // This query will use backup plan, the exaplin output for this query will generate three + // stages: winningPlan, rejectedPlans and originalWinningPlan + var test2 = db.foo.find({x: {$gte: 90000}}).sort({_id: 1}).explain(true); + assert(!backupPlanUsed(test2), "test2 did not use any backup plan: " + + tojson(test2)); + } + + checkIfBackupPlanUsed(); }()); \ No newline at end of file From 22982fdc00b21e657cfb6f6307d81b487b03001a Mon Sep 17 00:00:00 2001 From: Fox Lady Date: Tue, 24 Apr 2018 13:46:09 -0400 Subject: [PATCH 11/14] Modified Testing Function --- jstests/core/explain_use_backup_plan.js | 2 ++ jstests/libs/analyze_plan.js | 25 +++++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/jstests/core/explain_use_backup_plan.js b/jstests/core/explain_use_backup_plan.js index 3854f870a0cdf..ed2a5b6204471 100644 --- a/jstests/core/explain_use_backup_plan.js +++ b/jstests/core/explain_use_backup_plan.js @@ -25,11 +25,13 @@ load("jstests/libs/analyze_plan.js"); // This query will not use the backup plan, hence it generates only two stages: winningPlan and // rejectedPlans var test1 = db.foo.find({x: {$gte: 90}}).sort({_id: 1}).explain(true); + assert(!backupPlanUsed(test1), "test1 did not use any backup plan: " + tojson(test1)); // This query will use backup plan, the exaplin output for this query will generate three // stages: winningPlan, rejectedPlans and originalWinningPlan + var test2 = db.foo.find({x: {$gte: 90000}}).sort({_id: 1}).explain(true); assert(!backupPlanUsed(test2), "test2 did not use any backup plan: " + tojson(test2)); diff --git a/jstests/libs/analyze_plan.js b/jstests/libs/analyze_plan.js index d8c2cbdb6d583..2987691c32843 100644 --- a/jstests/libs/analyze_plan.js +++ b/jstests/libs/analyze_plan.js @@ -302,4 +302,29 @@ function backupPlanUsed(root) { assert(cursorStage.$cursor.hasOwnProperty("queryPlanner"), tojson(cursorStage)); return sectionHasOriginalWinningPlan(cursorStage.$cursor.queryPlanner); } + + // if (root.hasOwnProperty("shards")) { + // // This is a sharded agg explain. + // const cursorStages = getAggPlanStages(root, "$cursor"); + // assert(cursorStages.length !== 0, "Did not find any $cursor stages in sharded agg explain"); + // return cursorStages.find((cursorStage) => cursorStageHasOriginalWinningPlan(cursorStage)) !== + // undefined; + // } else if (root.hasOwnProperty("stages")) { + // // This is an agg explain. + // const cursorStages = getAggPlanStages(root, "$cursor"); + // return cursorStages.find((cursorStage) => cursorStageHasOriginalWinningPlan(cursorStage)) !== + // undefined; + // } else { + // // This is some sort of query explain. + // assert(root.hasOwnProperty("queryPlanner"), tojson(root)); + // assert(root.queryPlanner.hasOwnProperty("winningPlan"), tojson(root)); + // if (!root.queryPlanner.winningPlan.hasOwnProperty("shards")) { + // // This is an unsharded explain. + // return sectionHasOriginalWinningPlan(root.queryPlanner); + // } + // // This is a sharded explain. Each entry in the shards array contains a 'winningPlan' and + // // 'rejectedPlans'. + // return root.queryPlanner.winningPlan.shards.find( + // (shard) => sectionHasOriginalWinningPlan(shard)) !== undefined; + // } } \ No newline at end of file From ca5e4441847891f3b13abda8ff2a90b69868a622 Mon Sep 17 00:00:00 2001 From: Fox Lady Date: Thu, 26 Apr 2018 19:05:46 -0400 Subject: [PATCH 12/14] SERVER-32721 --- jstests/core/explain_use_backup_plan.js | 49 ++++++++++--------------- jstests/libs/analyze_plan.js | 39 +------------------- 2 files changed, 20 insertions(+), 68 deletions(-) diff --git a/jstests/core/explain_use_backup_plan.js b/jstests/core/explain_use_backup_plan.js index ed2a5b6204471..5510f5023ad87 100644 --- a/jstests/core/explain_use_backup_plan.js +++ b/jstests/core/explain_use_backup_plan.js @@ -4,38 +4,27 @@ load("jstests/libs/analyze_plan.js"); -(function() { - "use strict"; +"use strict"; - db.foo.drop(); - let bulk = db.foo.initializeUnorderedBulkOp(); +db.foo.drop(); +let bulk = db.foo.initializeUnorderedBulkOp(); - for (let i = 0; i < 100000; ++i) { - bulk.insert({_id: i, x: i, y: i}); - } +for (let i = 0; i < 100000; ++i) { + bulk.insert({_id: i, x: i, y: i}); +} - bulk.execute(); - db.foo.ensureIndex({x: 1}); +bulk.execute(); +db.foo.ensureIndex({x: 1}); - // Configure log level and lower the sort bytes limit. - db.setLogLevel(5, "query"); - db.adminCommand({setParameter: 1, internalQueryExecMaxBlockingSortBytes: 100}); - - function checkIfBackupPlanUsed() { - // This query will not use the backup plan, hence it generates only two stages: winningPlan and - // rejectedPlans - var test1 = db.foo.find({x: {$gte: 90}}).sort({_id: 1}).explain(true); - - assert(!backupPlanUsed(test1), "test1 did not use any backup plan: " + - tojson(test1)); +// Configure log level and lower the sort bytes limit. +db.setLogLevel(5, "query"); +db.adminCommand({setParameter: 1, internalQueryExecMaxBlockingSortBytes: 100}); - // This query will use backup plan, the exaplin output for this query will generate three - // stages: winningPlan, rejectedPlans and originalWinningPlan - - var test2 = db.foo.find({x: {$gte: 90000}}).sort({_id: 1}).explain(true); - assert(!backupPlanUsed(test2), "test2 did not use any backup plan: " + - tojson(test2)); - } - - checkIfBackupPlanUsed(); -}()); \ No newline at end of file +const test1 = db.foo.find({x: {$gte: 90}}).sort({_id: 1}).explain(true); +const test2 = db.foo.find({x: {$gte: 90000}}).sort({_id: 1}).explain(true); +// This query will not use the backup plan, hence it generates only two stages: winningPlan and +// rejectedPlans +assert.eq(backupPlanUsed(test1), false, "test1 did not use a backup plan"); +// This query will use backup plan, the exaplin output for this query will generate three +// stages: winningPlan, rejectedPlans and originalWinningPlan +assert(backupPlanUsed(test2), "backup plan invoked in test2"); \ No newline at end of file diff --git a/jstests/libs/analyze_plan.js b/jstests/libs/analyze_plan.js index 2987691c32843..935e6c1e9441c 100644 --- a/jstests/libs/analyze_plan.js +++ b/jstests/libs/analyze_plan.js @@ -288,43 +288,6 @@ function assertCoveredQueryAndCount({collection, query, project, count}) { assertExplainCount({explainResults: explain, expectedCount: count}); } -/** - * Returns true if a backup plan is used. - */ function backupPlanUsed(root) { - function sectionHasOriginalWinningPlan(explainSection) { - assert(explainSection.hasOwnProperty("originalWinningPlan"), tojson(explainSection)); - return explainSection.originalWinningPlan.length !== 0; - } - - function cursorStageHasOriginalWinningPlan(cursorStage) { - assert(cursorStage.hasOwnProperty("$cursor"), tojson(cursorStage)); - assert(cursorStage.$cursor.hasOwnProperty("queryPlanner"), tojson(cursorStage)); - return sectionHasOriginalWinningPlan(cursorStage.$cursor.queryPlanner); - } - - // if (root.hasOwnProperty("shards")) { - // // This is a sharded agg explain. - // const cursorStages = getAggPlanStages(root, "$cursor"); - // assert(cursorStages.length !== 0, "Did not find any $cursor stages in sharded agg explain"); - // return cursorStages.find((cursorStage) => cursorStageHasOriginalWinningPlan(cursorStage)) !== - // undefined; - // } else if (root.hasOwnProperty("stages")) { - // // This is an agg explain. - // const cursorStages = getAggPlanStages(root, "$cursor"); - // return cursorStages.find((cursorStage) => cursorStageHasOriginalWinningPlan(cursorStage)) !== - // undefined; - // } else { - // // This is some sort of query explain. - // assert(root.hasOwnProperty("queryPlanner"), tojson(root)); - // assert(root.queryPlanner.hasOwnProperty("winningPlan"), tojson(root)); - // if (!root.queryPlanner.winningPlan.hasOwnProperty("shards")) { - // // This is an unsharded explain. - // return sectionHasOriginalWinningPlan(root.queryPlanner); - // } - // // This is a sharded explain. Each entry in the shards array contains a 'winningPlan' and - // // 'rejectedPlans'. - // return root.queryPlanner.winningPlan.shards.find( - // (shard) => sectionHasOriginalWinningPlan(shard)) !== undefined; - // } + return root.queryPlanner.backupPlanUsed; } \ No newline at end of file From ffd904bd2860ba8fc198416fcc9e17f5fb381804 Mon Sep 17 00:00:00 2001 From: Fox Lady Date: Thu, 26 Apr 2018 23:13:08 -0400 Subject: [PATCH 13/14] SERVER-32721 Almost Done? --- src/mongo/db/query/explain.cpp | 37 +++++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/src/mongo/db/query/explain.cpp b/src/mongo/db/query/explain.cpp index 634225a0556f5..d5b18daa894d6 100644 --- a/src/mongo/db/query/explain.cpp +++ b/src/mongo/db/query/explain.cpp @@ -62,6 +62,7 @@ using std::string; using std::unique_ptr; using std::vector; +bool backupPlanUsed = false; /** * Traverse the tree rooted at 'root', and add all tree nodes into the list 'flattened'. */ @@ -280,6 +281,30 @@ std::vector> getRejectedPlansTrialStats(PlanExec return res; } +/** + * Gather the PlanStages when a backup plan is used + */ +std::vector> getRejectedPlansTrialStatsForAllPlanExecution(PlanExecutor* exec) { + const auto mps = getMultiPlanStage(exec->getRootStage()); + std::vector> res; + + if (backupPlanUsed) { + // Get the stats from the trial period for all the plans. + if (mps) { + const auto mpsStats = mps->getStats(); + for (size_t i = 0; i < mpsStats->children.size(); ++i) { + if (i != static_cast(mps->originalWinningPlanIdx())) { + res.emplace_back(std::move(mpsStats->children[i])); + } + } + } + } else { + res = getRejectedPlansTrialStats(exec); + } + + return res; +} + /** * Get PlanExecutor's winning plan stats tree. */ @@ -658,9 +683,10 @@ void Explain::generatePlannerInfo(PlanExecutor* exec, plannerBob.append("plannerVersion", QueryPlanner::kPlannerVersion); const auto mps = getMultiPlanStage(exec->getRootStage()); - const bool usedBackupPlan = ((mps->originalWinningPlanIdx()) > -1); + // const bool backupPlanUsed = ((mps->originalWinningPlanIdx()) > -1); + backupPlanUsed = ((mps->originalWinningPlanIdx()) > -1); - if (usedBackupPlan) { + if (backupPlanUsed) { plannerBob.append("backupPlanUsed", true); } else { plannerBob.append("backupPlanUsed", false); @@ -710,7 +736,7 @@ void Explain::generatePlannerInfo(PlanExecutor* exec, } allPlansBob.doneFast(); - if (usedBackupPlan) { + if (backupPlanUsed) { // Generate array of original winning plan BSONObjBuilder originalWinningPlanBob(plannerBob.subobjStart("originalWinningPlan")); const auto originalWinnerStats = getOriginalWinningPlanStatsTree(exec); @@ -830,7 +856,7 @@ void Explain::generateExecutionInfo(PlanExecutor* exec, planBob.doneFast(); } - const vector> rejectedStats = getRejectedPlansTrialStats(exec); + const vector> rejectedStats = getRejectedPlansTrialStatsForAllPlanExecution(exec); for (size_t i = 0; i < rejectedStats.size(); ++i) { BSONObjBuilder planBob(allPlansBob.subobjStart()); generateSinglePlanExecutionInfo( @@ -887,8 +913,9 @@ void Explain::explainStages(PlanExecutor* exec, const Collection* collection, ExplainOptions::Verbosity verbosity, BSONObjBuilder* out) { + auto winningPlanTrialStats = Explain::getWinningPlanTrialStats(exec); - + Status executePlanStatus = Status::OK(); // If we need execution stats, then run the plan in order to gather the stats. From d86caa4ced395bd64da14d4f352e2ca36b9d7250 Mon Sep 17 00:00:00 2001 From: Fox Lady Date: Tue, 1 May 2018 13:49:56 -0400 Subject: [PATCH 14/14] Server-32721 --- jstests/core/explain_use_backup_plan.js | 39 +++++++++-------- src/mongo/db/exec/multi_plan.h | 8 ++-- src/mongo/db/query/explain.cpp | 58 +++++++++---------------- src/mongo/db/query/explain.h | 4 +- 4 files changed, 47 insertions(+), 62 deletions(-) diff --git a/jstests/core/explain_use_backup_plan.js b/jstests/core/explain_use_backup_plan.js index 5510f5023ad87..f96124b21e720 100644 --- a/jstests/core/explain_use_backup_plan.js +++ b/jstests/core/explain_use_backup_plan.js @@ -3,28 +3,29 @@ // This test was designed to reproduce SERVER-32721" load("jstests/libs/analyze_plan.js"); +(function() { + "use strict"; -"use strict"; + const explain_backup_plan_test = db.explain_backup_plan; + explain_backup_plan_test.drop(); -db.foo.drop(); -let bulk = db.foo.initializeUnorderedBulkOp(); + let bulk = explain_backup_plan_test.initializeUnorderedBulkOp(); -for (let i = 0; i < 100000; ++i) { - bulk.insert({_id: i, x: i, y: i}); -} + for (let i = 0; i < 100000; ++i) { + bulk.insert({_id: i, x: i, y: i}); + } -bulk.execute(); -db.foo.ensureIndex({x: 1}); + bulk.execute(); + explain_backup_plan_test.ensureIndex({x: 1}); -// Configure log level and lower the sort bytes limit. -db.setLogLevel(5, "query"); -db.adminCommand({setParameter: 1, internalQueryExecMaxBlockingSortBytes: 100}); + db.adminCommand({setParameter: 1, internalQueryExecMaxBlockingSortBytes: 100}); -const test1 = db.foo.find({x: {$gte: 90}}).sort({_id: 1}).explain(true); -const test2 = db.foo.find({x: {$gte: 90000}}).sort({_id: 1}).explain(true); -// This query will not use the backup plan, hence it generates only two stages: winningPlan and -// rejectedPlans -assert.eq(backupPlanUsed(test1), false, "test1 did not use a backup plan"); -// This query will use backup plan, the exaplin output for this query will generate three -// stages: winningPlan, rejectedPlans and originalWinningPlan -assert(backupPlanUsed(test2), "backup plan invoked in test2"); \ No newline at end of file + const test1 = explain_backup_plan_test.find({x: {$gte: 90}}).sort({_id: 1}).explain(true); + const test2 = explain_backup_plan_test.find({x: {$gte: 90000}}).sort({_id: 1}).explain(true); + // This query will not use the backup plan, hence it generates only two stages: winningPlan and + // rejectedPlans. + assert(!backupPlanUsed(test1), "test1 did not use a backup plan"); + // This query will use backup plan, the exaplin output for this query will generate three + // stages: winningPlan, rejectedPlans and originalWinningPlan. + assert(backupPlanUsed(test2), "backup plan invoked in test2"); +})(); \ No newline at end of file diff --git a/src/mongo/db/exec/multi_plan.h b/src/mongo/db/exec/multi_plan.h index dfdeddb1688bf..3b70cb9c43582 100644 --- a/src/mongo/db/exec/multi_plan.h +++ b/src/mongo/db/exec/multi_plan.h @@ -206,15 +206,15 @@ class MultiPlanStage final : public PlanStage { // one-to-one with _candidates. std::vector _candidates; - // index into _candidates, of the winner of the plan competition + // index into '_candidates' of the winner of the plan competition // uses -1 / kNoSuchPlan when best plan is not (yet) known int _bestPlanIdx; - // index into _candidates, of the backup of the plan competition - // uses -1 / kNoSuchPlan when best plan is not (yet) known + // The index within '_candidates' of the non-blocking backup plan which can be used if a blocking plan fails. + // This is set to 'kNoSuchPlan' if there is no backup plan, or when it is not yet known. int _backupPlanIdx; - // Index into _candidates, of the original winner of the plan which can be used if a blocking plan .fails + // Index into '_candidates' of the original winner of the plan which can be used if a blocking plan fails. // This is set to 'kNoSuchPlan' if there is no backup plan, or when it is not (yet) known int _originalWinningPlanIdx; diff --git a/src/mongo/db/query/explain.cpp b/src/mongo/db/query/explain.cpp index d5b18daa894d6..57a488aeb18d1 100644 --- a/src/mongo/db/query/explain.cpp +++ b/src/mongo/db/query/explain.cpp @@ -62,7 +62,6 @@ using std::string; using std::unique_ptr; using std::vector; -bool backupPlanUsed = false; /** * Traverse the tree rooted at 'root', and add all tree nodes into the list 'flattened'. */ @@ -261,45 +260,31 @@ void appendMultikeyPaths(const BSONObj& keyPattern, /** * Gather the PlanStageStats for all of the losing plans. If exec doesn't have a MultiPlanStage * (or any losing plans), will return an empty vector. + * If a backup plan was used, includes the stats of the new winning plan instaed of the */ -std::vector> getRejectedPlansTrialStats(PlanExecutor* exec) { +std::vector> getRejectedPlansTrialStats(PlanExecutor* exec, bool backupPlanUsed) { // Inspect the tree to see if there is a MultiPlanStage. Plan selection has already happened at // this point, since we have a PlanExecutor. const auto mps = getMultiPlanStage(exec->getRootStage()); std::vector> res; - + // Get the stats from the trial period for all the plans. if (mps) { const auto mpsStats = mps->getStats(); - for (size_t i = 0; i < mpsStats->children.size(); ++i) { - if (i != static_cast(mps->bestPlanIdx())) { - res.emplace_back(std::move(mpsStats->children[i])); - } - } - } - - return res; -} - -/** - * Gather the PlanStages when a backup plan is used - */ -std::vector> getRejectedPlansTrialStatsForAllPlanExecution(PlanExecutor* exec) { - const auto mps = getMultiPlanStage(exec->getRootStage()); - std::vector> res; - if (backupPlanUsed) { - // Get the stats from the trial period for all the plans. - if (mps) { - const auto mpsStats = mps->getStats(); + if (backupPlanUsed) { for (size_t i = 0; i < mpsStats->children.size(); ++i) { if (i != static_cast(mps->originalWinningPlanIdx())) { res.emplace_back(std::move(mpsStats->children[i])); } - } + } + } else { + for (size_t i = 0; i < mpsStats->children.size(); ++i) { + if (i != static_cast(mps->bestPlanIdx())) { + res.emplace_back(std::move(mpsStats->children[i])); + } + } } - } else { - res = getRejectedPlansTrialStats(exec); } return res; @@ -314,12 +299,11 @@ unique_ptr getWinningPlanStatsTree(const PlanExecutor* exec) { : std::move(exec->getRootStage()->getStats()); } - /** - * Returns the root of the roginal winning plan used by 'exec'. + * Returns the root of the orginal winning plan used by 'exec'. * This might be different than the final winning plan in the case that the MultiPlanStage selected a * blocking plan which failed, and fell back to a non-blocking plan instead. - * If there is no MultiPlanStage in the tree, returns the root stage of 'exec' + * If there is no MultiPlanStage in the tree, returns the root stage of 'exec'. */ unique_ptr getOriginalWinningPlanStatsTree(const PlanExecutor* exec) { MultiPlanStage* mps = getMultiPlanStage(exec->getRootStage()); @@ -675,7 +659,7 @@ void Explain::getWinningPlanStats(const PlanExecutor* exec, BSONObjBuilder* bob) // static void Explain::generatePlannerInfo(PlanExecutor* exec, const Collection* collection, - BSONObjBuilder* out) { + BSONObjBuilder* out, bool backupPlanUsed) { CanonicalQuery* query = exec->getCanonicalQuery(); BSONObjBuilder plannerBob(out->subobjStart("queryPlanner")); @@ -683,7 +667,6 @@ void Explain::generatePlannerInfo(PlanExecutor* exec, plannerBob.append("plannerVersion", QueryPlanner::kPlannerVersion); const auto mps = getMultiPlanStage(exec->getRootStage()); - // const bool backupPlanUsed = ((mps->originalWinningPlanIdx()) > -1); backupPlanUsed = ((mps->originalWinningPlanIdx()) > -1); if (backupPlanUsed) { @@ -728,7 +711,7 @@ void Explain::generatePlannerInfo(PlanExecutor* exec, winningPlanBob.doneFast(); // Genenerate array of rejected plans. - const vector> rejectedStats = getRejectedPlansTrialStats(exec); + const vector> rejectedStats = getRejectedPlansTrialStats(exec, backupPlanUsed); BSONArrayBuilder allPlansBob(plannerBob.subarrayStart("rejectedPlans")); for (size_t i = 0; i < rejectedStats.size(); i++) { BSONObjBuilder childBob(allPlansBob.subobjStart()); @@ -816,13 +799,14 @@ void Explain::generateExecutionInfo(PlanExecutor* exec, ExplainOptions::Verbosity verbosity, Status executePlanStatus, PlanStageStats* winningPlanTrialStats, - BSONObjBuilder* out) { + BSONObjBuilder* out, bool backupPlanUsed) { invariant(verbosity >= ExplainOptions::Verbosity::kExecStats); if (verbosity >= ExplainOptions::Verbosity::kExecAllPlans && findStageOfType(exec->getRootStage(), STAGE_MULTI_PLAN) != nullptr) { invariant(winningPlanTrialStats, "winningPlanTrialStats must be non-null when requesting all execution stats"); } + BSONObjBuilder execBob(out->subobjStart("executionStats")); // If there is an execution error while running the query, the error is reported under @@ -856,7 +840,7 @@ void Explain::generateExecutionInfo(PlanExecutor* exec, planBob.doneFast(); } - const vector> rejectedStats = getRejectedPlansTrialStatsForAllPlanExecution(exec); + const vector> rejectedStats = getRejectedPlansTrialStats(exec, backupPlanUsed); for (size_t i = 0; i < rejectedStats.size(); ++i) { BSONObjBuilder planBob(allPlansBob.subobjStart()); generateSinglePlanExecutionInfo( @@ -879,13 +863,14 @@ void Explain::explainStages(PlanExecutor* exec, // // Use the stats trees to produce explain BSON. // + bool backupPlanUsed = false; if (verbosity >= ExplainOptions::Verbosity::kQueryPlanner) { - generatePlannerInfo(exec, collection, out); + generatePlannerInfo(exec, collection, out, backupPlanUsed); } if (verbosity >= ExplainOptions::Verbosity::kExecStats) { - generateExecutionInfo(exec, verbosity, executePlanStatus, winningPlanTrialStats, out); + generateExecutionInfo(exec, verbosity, executePlanStatus, winningPlanTrialStats, out, backupPlanUsed); } } @@ -915,7 +900,6 @@ void Explain::explainStages(PlanExecutor* exec, BSONObjBuilder* out) { auto winningPlanTrialStats = Explain::getWinningPlanTrialStats(exec); - Status executePlanStatus = Status::OK(); // If we need execution stats, then run the plan in order to gather the stats. diff --git a/src/mongo/db/query/explain.h b/src/mongo/db/query/explain.h index cb177321f1c23..a6520f528c98a 100644 --- a/src/mongo/db/query/explain.h +++ b/src/mongo/db/query/explain.h @@ -178,7 +178,7 @@ class Explain { */ static void generatePlannerInfo(PlanExecutor* exec, const Collection* collection, - BSONObjBuilder* out); + BSONObjBuilder* out, bool backupPlanUsed); /** * Private helper that does the heavy-lifting for the public statsToBSON(...) functions @@ -204,7 +204,7 @@ class Explain { ExplainOptions::Verbosity verbosity, Status executePlanStatus, PlanStageStats* winningPlanTrialStats, - BSONObjBuilder* out); + BSONObjBuilder* out, bool backupPlanUsed); /** * Generates the execution stats section for the stats tree 'stats',