Skip to content
Browse files

SERVER-10026 don't add a collscan solution if ixscan soln exists

  • Loading branch information...
1 parent 997798b commit a3d910925350d2f0204b41ea145e24f74e5c39ce Hari Khalsa committed Feb 26, 2014
View
24 jstests/plan_cache_commands.js
@@ -9,19 +9,22 @@
*/
var t = db.jstests_plan_cache_commands;
-
t.drop();
-t.save({a: 1});
+// Insert some data so we don't go to EOF.
+t.save({a: 1, b: 1});
t.save({a: 2, b: 2});
+// We need two indices so that the MultiPlanRunner is executed.
t.ensureIndex({a: 1});
+t.ensureIndex({a: 1, b:1});
-var queryA1 = {a: 1};
+// Run the query.
+var queryA1 = {a: 1, b:1};
var projectionA1 = {_id: 0, a: 1};
var sortA1 = {a: -1};
assert.eq(1, t.find(queryA1, projectionA1).sort(sortA1).itcount(), 'unexpected document count');
-
+// We now expect the two indices to be compared and a cache entry to exist.
//
@@ -38,8 +41,8 @@ function getShapes() {
return res.shapes;
}
-// Attempting to retrieve cache information on non-existent collection
-// is an error.
+
+// Attempting to retrieve cache information on non-existent collection is an error.
var missingCollection = db.jstests_query_cache_missing;
missingCollection.drop();
assert.commandFailed(missingCollection.runCommand('planCacheListQueryShapes'));
@@ -205,12 +208,14 @@ assert.gt(plans[0].feedback.averageScore, 0, 'invalid average score');
t.drop();
var n = 200;
for (var i = 0; i < n; i++) {
- t.save({b: i});
+ t.save({a:i, b: i});
}
+t.ensureIndex({a: 1});
t.ensureIndex({b: 1});
+t.ensureIndex({a: 1, b: 1});
// Repopulate plan cache with 3 query shapes.
-var queryB = {b: {$gte: 0}};
+var queryB = {a: {$gte: 0}, b: {$gte: 0}};
var projectionB = {_id: 0, b: 1};
var sortB = {b: -1};
assert.eq(n, t.find(queryB, projectionB).sort(sortB).itcount(), 'unexpected document count');
@@ -346,6 +351,7 @@ for (var i = 0; i < multiPlanRunnerExplain.allPlans.length; ++i) {
t.drop();
t.ensureIndex({a: 1});
+t.ensureIndex({b: 1});
for (var i = 0; i < 200; i++) {
t.save({a: 1, b: 1});
@@ -361,5 +367,5 @@ assert.eq(1, t.find({a: 2, b: 2}).itcount(), 'unexpected count');
assert.eq(1, getShapes().length, 'unexpected number of query shapes in plan cache');
// A query that returns results but does not hit EOF will also be cached.
-assert.eq(200, t.find({a: 1}).itcount(), 'unexpected count');
+assert.eq(200, t.find({a: {$gte: 0}, b:1}).itcount(), 'unexpected count');
assert.eq(2, getShapes().length, 'unexpected number of query shapes in plan cache');
View
1 src/mongo/db/pipeline/pipeline_d.cpp
@@ -147,7 +147,6 @@ namespace {
// cursor. Either way, we can then apply other optimizations there
// are tickets for, such as SERVER-4507.
const size_t runnerOptions = QueryPlannerParams::DEFAULT
- | QueryPlannerParams::INCLUDE_COLLSCAN
| QueryPlannerParams::INCLUDE_SHARD_FILTER
| QueryPlannerParams::NO_BLOCKING_SORT
;
View
129 src/mongo/db/query/get_runner.cpp
@@ -139,6 +139,70 @@ namespace mongo {
bool turnIxscanIntoCount(QuerySolution* soln);
} // namespace
+
+ void fillOutPlannerParams(Collection* collection,
+ CanonicalQuery* canonicalQuery,
+ QueryPlannerParams* plannerParams) {
+ // If it's not NULL, we may have indices. Access the catalog and fill out IndexEntry(s)
+ IndexCatalog::IndexIterator ii = collection->getIndexCatalog()->getIndexIterator(false);
+ while (ii.more()) {
+ const IndexDescriptor* desc = ii.next();
+ plannerParams->indices.push_back(IndexEntry(desc->keyPattern(),
+ desc->getAccessMethodName(),
+ desc->isMultikey(),
+ desc->isSparse(),
+ desc->indexName(),
+ desc->infoObj()));
+ }
+
+ // If query supports index filters, filter params.indices by indices in query settings.
+ QuerySettings* querySettings = collection->infoCache()->getQuerySettings();
+ AllowedIndices* allowedIndicesRaw;
+
+ // Filter index catalog if index filters are specified for query.
+ // Also, signal to planner that application hint should be ignored.
+ if (querySettings->getAllowedIndices(*canonicalQuery, &allowedIndicesRaw)) {
+ boost::scoped_ptr<AllowedIndices> allowedIndices(allowedIndicesRaw);
+ filterAllowedIndexEntries(*allowedIndices, &plannerParams->indices);
+ plannerParams->indexFiltersApplied = true;
+ }
+
+ // We will not output collection scans unless there are no indexed solutions. NO_TABLE_SCAN
+ // overrides this behavior by not outputting a collscan even if there are no indexed
+ // solutions.
+ if (storageGlobalParams.noTableScan) {
+ const string& ns = canonicalQuery->ns();
+ // There are certain cases where we ignore this restriction:
+ bool ignore = canonicalQuery->getQueryObj().isEmpty()
+ || (string::npos != ns.find(".system."))
+ || (0 == ns.find("local."));
+ if (!ignore) {
+ plannerParams->options |= QueryPlannerParams::NO_TABLE_SCAN;
+ }
+ }
+
+ // If the caller wants a shard filter, make sure we're actually sharded.
+ if (plannerParams->options & QueryPlannerParams::INCLUDE_SHARD_FILTER) {
+ CollectionMetadataPtr collMetadata =
+ shardingState.getCollectionMetadata(canonicalQuery->ns());
+
+ if (collMetadata) {
+ plannerParams->shardKey = collMetadata->getKeyPattern();
+ }
+ else {
+ // If there's no metadata don't bother w/the shard filter since we won't know what
+ // the key pattern is anyway...
+ plannerParams->options &= ~QueryPlannerParams::INCLUDE_SHARD_FILTER;
+ }
+ }
+
+ if (enableIndexIntersection) {
+ plannerParams->options |= QueryPlannerParams::INDEX_INTERSECTION;
+ }
+
+ plannerParams->options |= QueryPlannerParams::KEEP_MUTATIONS;
+ }
+
/**
* For a given query, get a runner. The runner could be a SingleSolutionRunner, a
* CachedQueryRunner, or a MultiPlanRunner, depending on the cache/query solver/etc.
@@ -164,32 +228,6 @@ namespace mongo {
return Status::OK();
}
- // If it's not NULL, we may have indices. Access the catalog and fill out IndexEntry(s)
- QueryPlannerParams plannerParams;
-
- IndexCatalog::IndexIterator ii = collection->getIndexCatalog()->getIndexIterator(false);
- while (ii.more()) {
- const IndexDescriptor* desc = ii.next();
- plannerParams.indices.push_back(IndexEntry(desc->keyPattern(),
- desc->getAccessMethodName(),
- desc->isMultikey(),
- desc->isSparse(),
- desc->indexName(),
- desc->infoObj()));
- }
-
- // If query supports index filters, filter params.indices by indices in query settings.
- QuerySettings* querySettings = collection->infoCache()->getQuerySettings();
- AllowedIndices* allowedIndicesRaw;
-
- // Filter index catalog if index filters are specified for query.
- // Also, signal to planner that application hint should be ignored.
- if (querySettings->getAllowedIndices(*canonicalQuery, &allowedIndicesRaw)) {
- boost::scoped_ptr<AllowedIndices> allowedIndices(allowedIndicesRaw);
- filterAllowedIndexEntries(*allowedIndices, &plannerParams.indices);
- plannerParams.indexFiltersApplied = true;
- }
-
// Tailable: If the query requests tailable the collection must be capped.
if (canonicalQuery->getParsed().hasOption(QueryOption_CursorTailable)) {
if (!collection->isCapped()) {
@@ -209,37 +247,10 @@ namespace mongo {
}
}
- // Process the planning options.
+ // Fill out the planning params. We use these for both cached solutions and non-cached.
+ QueryPlannerParams plannerParams;
plannerParams.options = plannerOptions;
- if (storageGlobalParams.noTableScan) {
- const string& ns = canonicalQuery->ns();
- // There are certain cases where we ignore this restriction:
- bool ignore = canonicalQuery->getQueryObj().isEmpty()
- || (string::npos != ns.find(".system."))
- || (0 == ns.find("local."));
- if (!ignore) {
- plannerParams.options |= QueryPlannerParams::NO_TABLE_SCAN;
- }
- }
-
- if (!(plannerParams.options & QueryPlannerParams::NO_TABLE_SCAN)) {
- plannerParams.options |= QueryPlannerParams::INCLUDE_COLLSCAN;
- }
-
- // If the caller wants a shard filter, make sure we're actually sharded.
- if (plannerParams.options & QueryPlannerParams::INCLUDE_SHARD_FILTER) {
- CollectionMetadataPtr collMetadata =
- shardingState.getCollectionMetadata(canonicalQuery->ns());
-
- if (collMetadata) {
- plannerParams.shardKey = collMetadata->getKeyPattern();
- }
- else {
- // If there's no metadata don't bother w/the shard filter since we won't know what
- // the key pattern is anyway...
- plannerParams.options &= ~QueryPlannerParams::INCLUDE_SHARD_FILTER;
- }
- }
+ fillOutPlannerParams(collection, rawCanonicalQuery, &plannerParams);
// Try to look up a cached solution for the query.
//
@@ -310,12 +321,6 @@ namespace mongo {
}
}
- if (enableIndexIntersection) {
- plannerParams.options |= QueryPlannerParams::INDEX_INTERSECTION;
- }
-
- plannerParams.options |= QueryPlannerParams::KEEP_MUTATIONS;
-
vector<QuerySolution*> solutions;
Status status = QueryPlanner::plan(*canonicalQuery, plannerParams, &solutions);
if (!status.isOK()) {
View
15 src/mongo/db/query/get_runner.h
@@ -27,6 +27,7 @@
*/
#include "mongo/db/query/canonical_query.h"
+#include "mongo/db/query/query_planner_params.h"
#include "mongo/db/query/query_settings.h"
#include "mongo/db/query/runner.h"
@@ -43,6 +44,13 @@ namespace mongo {
void filterAllowedIndexEntries(const AllowedIndices& allowedIndices,
std::vector<IndexEntry>* indexEntries);
+ /**
+ * Fill out the provided 'plannerParams' for the 'canonicalQuery' operating on the collection
+ * 'collection'. Exposed for testing.
+ */
+ void fillOutPlannerParams(Collection* collection,
+ CanonicalQuery* canonicalQuery,
+ QueryPlannerParams* plannerParams);
/**
* Get a runner for a query. Takes ownership of rawCanonicalQuery.
@@ -80,8 +88,11 @@ namespace mongo {
* the returned runner. On failure, returns other status values, and '*outRunner' and
* '*outCanonicalQuery' have unspecified values.
*/
- Status getRunner(Collection* collection, const std::string& ns, const BSONObj& unparsedQuery,
- Runner** outRunner, CanonicalQuery** outCanonicalQuery,
+ Status getRunner(Collection* collection,
+ const std::string& ns,
+ const BSONObj& unparsedQuery,
+ Runner** outRunner,
+ CanonicalQuery** outCanonicalQuery,
size_t plannerOptions = 0);
/*
View
20 src/mongo/db/query/query_planner.cpp
@@ -818,13 +818,19 @@ namespace mongo {
}
}
- // TODO: Do we always want to offer a collscan solution?
- // XXX: currently disabling the always-use-a-collscan in order to find more planner bugs.
- if ( !QueryPlannerCommon::hasNode(query.root(), MatchExpression::GEO_NEAR)
- && !QueryPlannerCommon::hasNode(query.root(), MatchExpression::TEXT)
- && hintIndex.isEmpty()
- && ((params.options & QueryPlannerParams::INCLUDE_COLLSCAN) || (0 == out->size() && canTableScan)))
- {
+ // geoNear and text queries *require* an index.
+ // Also, if a hint is specified it indicates that we MUST use it.
+ bool possibleToCollscan = !QueryPlannerCommon::hasNode(query.root(), MatchExpression::GEO_NEAR)
+ && !QueryPlannerCommon::hasNode(query.root(), MatchExpression::TEXT)
+ && hintIndex.isEmpty();
+
+ // The caller can explicitly ask for a collscan.
+ bool collscanRequested = (params.options & QueryPlannerParams::INCLUDE_COLLSCAN);
+
+ // No indexed plans? We must provide a collscan if possible or else we can't run the query.
+ bool collscanNeeded = (0 == out->size() && canTableScan);
+
+ if (possibleToCollscan && (collscanRequested || collscanNeeded)) {
QuerySolution* collscan = buildCollscanSoln(query, false, params);
if (NULL != collscan) {
SolutionCacheData* scd = new SolutionCacheData();
View
3 src/mongo/db/query/query_planner_params.h
@@ -52,7 +52,8 @@ namespace mongo {
// See http://docs.mongodb.org/manual/reference/parameters/
NO_TABLE_SCAN = 1,
- // Set this if you want a collscan outputted even if there's an ixscan.
+ // Set this if you *always* want a collscan outputted, even if there's an ixscan. This
+ // makes ranking less accurate, especially in the presence of blocking stages.
INCLUDE_COLLSCAN = 1 << 1,
// Set this if you're running on a sharded cluster. We'll add a "drop all docs that
View
76 src/mongo/dbtests/plan_ranking.cpp
@@ -37,6 +37,7 @@
#include "mongo/db/instance.h"
#include "mongo/db/json.h"
#include "mongo/db/query/multi_plan_runner.h"
+#include "mongo/db/query/get_runner.h"
#include "mongo/db/query/qlog.h"
#include "mongo/db/query/query_planner.h"
#include "mongo/db/query/query_planner_test_lib.h"
@@ -82,7 +83,9 @@ namespace PlanRankingTests {
Collection* collection = ctx.ctx().db()->getCollection(ns);
QueryPlannerParams plannerParams;
- plannerParams.options |= QueryPlannerParams::INDEX_INTERSECTION;
+ fillOutPlannerParams(collection, cq, &plannerParams);
+ // Turn this off otherwise it pops up in some plans.
+ plannerParams.options &= ~QueryPlannerParams::KEEP_MUTATIONS;
// Fill out the available indices.
IndexCatalog::IndexIterator ii = collection->getIndexCatalog()->getIndexIterator(false);
@@ -101,8 +104,7 @@ namespace PlanRankingTests {
Status status = QueryPlanner::plan(*cq, plannerParams, &solutions);
ASSERT(status.isOK());
- // MPR requires >1 soln.
- ASSERT_GREATER_THAN(solutions.size(), 1U);
+ ASSERT_GREATER_THAN_OR_EQUALS(solutions.size(), 1U);
// Fill out the MPR.
_mpr.reset(new MultiPlanRunner(collection, cq));
@@ -329,6 +331,72 @@ namespace PlanRankingTests {
}
};
+ /**
+ * We have an index on _id and a query over _id with a sort. Ensure that we don't pick a
+ * collscan as the best plan even though the _id-scanning solution doesn't produce any results.
+ */
+ class PlanRankingNoCollscan : public PlanRankingTestBase {
+ public:
+ void run() {
+ static const int N = 10000;
+
+ for (int i = 0; i < N; ++i) {
+ insert(BSON("_id" << i));
+ }
+
+ addIndex(BSON("_id" << 1));
+
+ // Run a query with a sort. The blocking sort won't produce any data during the
+ // evaluation period.
+ CanonicalQuery* cq;
+ BSONObj queryObj = BSON("_id" << BSON("$gte" << 20 << "$lte" << 200));
+ BSONObj sortObj = BSON("c" << 1);
+ BSONObj projObj = BSONObj();
+ ASSERT(CanonicalQuery::canonicalize(ns,
+ queryObj,
+ sortObj,
+ projObj,
+ &cq).isOK());
+
+ // Takes ownership of cq.
+ QuerySolution* soln = pickBestPlan(cq);
+
+ // The best must not be a collscan.
+ ASSERT(QueryPlannerTestLib::solutionMatches(
+ "{sort: {pattern: {c: 1}, limit: 0, node: {"
+ "fetch: {filter: null, node: "
+ "{ixscan: {filter: null, pattern: {_id: 1}}}}}}}}",
+ soln->root.get()));
+ }
+ };
+
+ /**
+ * No indices are available, output a collscan.
+ */
+ class PlanRankingCollscan : public PlanRankingTestBase {
+ public:
+ void run() {
+ static const int N = 10000;
+
+ // Insert data for which we have no index.
+ for (int i = 0; i < N; ++i) {
+ insert(BSON("foo" << i));
+ }
+
+ // Look for A Space Odyssey.
+ CanonicalQuery* cq;
+ verify(CanonicalQuery::canonicalize(ns, BSON("foo" << 2001), &cq).isOK());
+ ASSERT(NULL != cq);
+
+ // Takes ownership of cq.
+ QuerySolution* soln = pickBestPlan(cq);
+
+ // The best must be a collscan.
+ ASSERT(QueryPlannerTestLib::solutionMatches(
+ "{cscan: {dir: 1, filter: {foo: 2001}}}",
+ soln->root.get()));
+ }
+ };
class All : public Suite {
public:
@@ -340,6 +408,8 @@ namespace PlanRankingTests {
add<PlanRankingAvoidIntersectIfNoResults>();
add<PlanRankingPreferCoveredEvenIfNoResults>();
add<PlanRankingPreferImmediateEOF>();
+ add<PlanRankingNoCollscan>();
+ add<PlanRankingCollscan>();
}
} planRankingAll;

0 comments on commit a3d9109

Please sign in to comment.
Something went wrong with that request. Please try again.