diff --git a/jstests/core/explain_shell_helpers.js b/jstests/core/explain_shell_helpers.js index 0e4f9b3e72e..836092bb0d4 100644 --- a/jstests/core/explain_shell_helpers.js +++ b/jstests/core/explain_shell_helpers.js @@ -175,10 +175,8 @@ assert.commandWorked(explain); // .snapshot() explain = t.explain().find().snapshot().finish(); assert.commandWorked(explain); -assert(isIxscan(explain.queryPlanner.winningPlan)); explain = t.find().snapshot().explain(); assert.commandWorked(explain); -assert(isIxscan(explain.queryPlanner.winningPlan)); // .next() explain = t.explain().find().next(); diff --git a/jstests/mmap_v1/explain_snapshot.js b/jstests/mmap_v1/explain_snapshot.js new file mode 100644 index 00000000000..5b1ea5a7070 --- /dev/null +++ b/jstests/mmap_v1/explain_snapshot.js @@ -0,0 +1,23 @@ +/** + * This test ensures that .snapshot() performs an index scan on mmap_v1. + */ +(function() { + 'use strict'; + + load("jstests/libs/analyze_plan.js"); + + var coll = db.jstests_explain_snapshot; + coll.drop(); + + assert.writeOK(coll.insert({})); + + var explain = coll.explain().find().snapshot().finish(); + assert.commandWorked(explain); + assert(isIxscan(explain.queryPlanner.winningPlan)); + + explain = coll.find().snapshot().explain(); + assert.commandWorked(explain); + assert(isIxscan(explain.queryPlanner.winningPlan)); + + coll.drop(); +})(); \ No newline at end of file diff --git a/src/mongo/db/query/get_executor.cpp b/src/mongo/db/query/get_executor.cpp index 7b23c1c10e5..4491d2de6a8 100644 --- a/src/mongo/db/query/get_executor.cpp +++ b/src/mongo/db/query/get_executor.cpp @@ -197,6 +197,13 @@ void fillOutPlannerParams(OperationContext* txn, } else { plannerParams->options |= QueryPlannerParams::KEEP_MUTATIONS; } + + // MMAPv1 storage engine should have snapshot() perform an index scan on _id rather than a + // collection scan since a collection scan on the MMAP storage engine can return duplicates + // or miss documents. + if (isMMAPV1()) { + plannerParams->options |= QueryPlannerParams::SNAPSHOT_USE_ID; + } } namespace { @@ -1374,8 +1381,8 @@ StatusWith> getExecutorDistinct(OperationContext* txn, BSONObj(), // hint BSONObj(), // min BSONObj(), // max + false, // snapshot isExplain, - false, // snapshot whereCallback); if (!statusWithCQ.isOK()) { return statusWithCQ.getStatus(); diff --git a/src/mongo/db/query/query_planner.cpp b/src/mongo/db/query/query_planner.cpp index e027e00c086..ea6e5721a1e 100644 --- a/src/mongo/db/query/query_planner.cpp +++ b/src/mongo/db/query/query_planner.cpp @@ -436,15 +436,16 @@ Status QueryPlanner::plan(const CanonicalQuery& query, LOG(5) << "Index " << i << " is " << params.indices[i].toString() << endl; } - bool canTableScan = !(params.options & QueryPlannerParams::NO_TABLE_SCAN); + const bool canTableScan = !(params.options & QueryPlannerParams::NO_TABLE_SCAN); + const bool isTailable = query.getParsed().isTailable(); // If the query requests a tailable cursor, the only solution is a collscan + filter with // tailable set on the collscan. TODO: This is a policy departure. Previously I think you // could ask for a tailable cursor and it just tried to give you one. Now, we fail if we // can't provide one. Is this what we want? - if (query.getParsed().isTailable()) { + if (isTailable) { if (!QueryPlannerCommon::hasNode(query.root(), MatchExpression::GEO_NEAR) && canTableScan) { - QuerySolution* soln = buildCollscanSoln(query, true, params); + QuerySolution* soln = buildCollscanSoln(query, isTailable, params); if (NULL != soln) { out->push_back(soln); } @@ -468,7 +469,7 @@ Status QueryPlanner::plan(const CanonicalQuery& query, // min/max are incompatible with $natural. if (canTableScan && query.getParsed().getMin().isEmpty() && query.getParsed().getMax().isEmpty()) { - QuerySolution* soln = buildCollscanSoln(query, false, params); + QuerySolution* soln = buildCollscanSoln(query, isTailable, params); if (NULL != soln) { out->push_back(soln); } @@ -497,14 +498,30 @@ Status QueryPlanner::plan(const CanonicalQuery& query, hintIndex = query.getParsed().getHint(); } - // Snapshot is a form of a hint. If snapshot is set, try to use _id index to make a real - // plan. If that fails, just scan the _id index. - if (query.getParsed().isSnapshot()) { - // Find the ID index in indexKeyPatterns. It's our hint. - for (size_t i = 0; i < params.indices.size(); ++i) { - if (isIdIndex(params.indices[i].keyPattern)) { - hintIndex = params.indices[i].keyPattern; - break; + // If snapshot is set, default to collscanning. If the query param SNAPSHOT_USE_ID is set, + // snapshot is a form of a hint, so try to use _id index to make a real plan. If that fails, + // just scan the _id index. + // + // Don't do this if the query is a geonear or text as as text search queries must be answered + // using full text indices and geoNear queries must be answered using geospatial indices. + if (query.getParsed().isSnapshot() && + !QueryPlannerCommon::hasNode(query.root(), MatchExpression::GEO_NEAR) && + !QueryPlannerCommon::hasNode(query.root(), MatchExpression::TEXT)) { + const bool useIXScan = params.options & QueryPlannerParams::SNAPSHOT_USE_ID; + + if (!useIXScan) { + QuerySolution* soln = buildCollscanSoln(query, isTailable, params); + if (soln) { + out->push_back(soln); + } + return Status::OK(); + } else { + // Find the ID index in indexKeyPatterns. It's our hint. + for (size_t i = 0; i < params.indices.size(); ++i) { + if (isIdIndex(params.indices[i].keyPattern)) { + hintIndex = params.indices[i].keyPattern; + break; + } } } } @@ -893,7 +910,7 @@ Status QueryPlanner::plan(const CanonicalQuery& query, bool collscanNeeded = (0 == out->size() && canTableScan); if (possibleToCollscan && (collscanRequested || collscanNeeded)) { - QuerySolution* collscan = buildCollscanSoln(query, false, params); + QuerySolution* collscan = buildCollscanSoln(query, isTailable, params); if (NULL != collscan) { SolutionCacheData* scd = new SolutionCacheData(); scd->solnType = SolutionCacheData::COLLSCAN_SOLN; diff --git a/src/mongo/db/query/query_planner_params.h b/src/mongo/db/query/query_planner_params.h index cfa41b08f83..133d2312554 100644 --- a/src/mongo/db/query/query_planner_params.h +++ b/src/mongo/db/query/query_planner_params.h @@ -87,6 +87,11 @@ struct QueryPlannerParams { // Set this to prevent the planner from generating plans which answer a predicate // implicitly via exact index bounds for index intersection solutions. CANNOT_TRIM_IXISECT = 1 << 8, + + // Set this if snapshot() should scan the _id index rather than performing a + // collection scan. The MMAPv1 storage engine sets this option since it cannot + // guarantee that a collection scan won't miss documents or return duplicates. + SNAPSHOT_USE_ID = 1 << 9, }; // See Options enum above. diff --git a/src/mongo/db/query/query_planner_test.cpp b/src/mongo/db/query/query_planner_test.cpp index d58f179399f..0d85ccae1ad 100644 --- a/src/mongo/db/query/query_planner_test.cpp +++ b/src/mongo/db/query/query_planner_test.cpp @@ -1195,12 +1195,32 @@ TEST_F(QueryPlannerTest, Snapshot) { addIndex(BSON("a" << 1)); runQuerySnapshot(fromjson("{a: {$gt: 0}}")); + assertNumSolutions(1U); + assertSolutionExists("{cscan: {filter: {a: {$gt: 0}}, dir: 1}}"); +} + +TEST_F(QueryPlannerTest, SnapshotUseId) { + params.options = QueryPlannerParams::SNAPSHOT_USE_ID; + + addIndex(BSON("a" << 1)); + runQuerySnapshot(fromjson("{a: {$gt: 0}}")); + assertNumSolutions(1U); assertSolutionExists( "{fetch: {filter: {a:{$gt:0}}, node: " "{ixscan: {filter: null, pattern: {_id: 1}}}}}"); } +TEST_F(QueryPlannerTest, CannotSnapshotWithGeoNear) { + // Snapshot is skipped with geonear queries. + addIndex(BSON("a" + << "2d")); + runQuerySnapshot(fromjson("{a: {$near: [0,0]}}")); + + ASSERT_EQUALS(getNumSolutions(), 1U); + assertSolutionExists("{geoNear2d: {a: '2d'}}"); +} + // // Tree operations that require simple tree rewriting. //