diff options
author | Eric Milkie <milkie@10gen.com> | 2012-05-13 16:48:04 -0400 |
---|---|---|
committer | Eric Milkie <milkie@10gen.com> | 2012-05-13 16:48:04 -0400 |
commit | 03c34bd5fa1f4ebe9f626306f2c67d507c11a4eb (patch) | |
tree | 21320f5d4ac2dd0f8f03ed5213fdbbbefb4b0689 | |
parent | db9d17af5cba0c432269f6e7bd02bac9af60438b (diff) | |
download | mongo-03c34bd5fa1f4ebe9f626306f2c67d507c11a4eb.tar.gz |
SERVER-5353 Backport print capped collection scan on _id query warning message in appropriate cases.
-rw-r--r-- | db/queryoptimizer.cpp | 43 | ||||
-rw-r--r-- | db/queryoptimizer.h | 1 | ||||
-rw-r--r-- | jstests/queryoptimizera.js | 87 | ||||
-rw-r--r-- | jstests/slowNightly/sharding_passthrough.js | 2 | ||||
-rw-r--r-- | shell/utils.js | 3 |
5 files changed, 118 insertions, 18 deletions
diff --git a/db/queryoptimizer.cpp b/db/queryoptimizer.cpp index 71ca6576fc4..d91b79aac12 100644 --- a/db/queryoptimizer.cpp +++ b/db/queryoptimizer.cpp @@ -214,22 +214,6 @@ doneCheckOrder: if ( willScanTable() ) { if ( _frs.nNontrivialRanges() ) { checkTableScanAllowed( _frs.ns() ); - - // if we are doing a table scan on _id - // and its a capped collection - // we disallow as its a common user error - // .system. and local collections are exempt - if ( _d && _d->capped && _frs.range( "_id" ).nontrivial() ) { - if ( cc().isSyncThread() || - str::contains( _frs.ns() , ".system." ) || - str::startsWith( _frs.ns() , "local." ) ) { - // ok - } - else { - warning() << "_id query on capped collection without an _id index, performance will be poor collection: " << _frs.ns() << endl; - //uassert( 14820, str::stream() << "doing _id query on a capped collection without an index is not allowed: " << _frs.ns() , - } - } } return findTableScan( _frs.ns(), _order, startLoc ); } @@ -486,12 +470,14 @@ doneCheckOrder: _usingPrerecordedPlan = true; _mayRecordPlan = false; _plans.push_back( p ); + warnOnCappedIdTableScan(); return; } } } addOtherPlans( false ); + warnOnCappedIdTableScan(); } void QueryPlanSet::addOtherPlans( bool checkFirst ) { @@ -633,6 +619,31 @@ doneCheckOrder: } return _plans[0]; } + + void QueryPlanSet::warnOnCappedIdTableScan() const { + // if we are doing a table scan on _id + // and it's a capped collection + // we warn as it's a common user error + // .system. and local collections are exempt + const char *ns = _frsp->ns(); + NamespaceDetails *d = nsdetails( ns ); + if ( d && + d->capped && + nPlans() == 1 && + firstPlan()->willScanTable() && + firstPlan()->multikeyFrs().range( "_id" ).nontrivial() ) { + if ( cc().isSyncThread() || + str::contains( ns , ".system." ) || + str::startsWith( ns , "local." ) ) { + // ok + } + else { + warning() + << "unindexed _id query on capped collection, " + << "performance will be poor collection: " << ns << endl; + } + } + } QueryPlanSet::Runner::Runner( QueryPlanSet &plans, QueryOp &op ) : _op( op ), diff --git a/db/queryoptimizer.h b/db/queryoptimizer.h index fea6c0b82ae..78d169d7e1c 100644 --- a/db/queryoptimizer.h +++ b/db/queryoptimizer.h @@ -314,6 +314,7 @@ namespace mongo { } void init(); void addHint( IndexDetails &id ); + void warnOnCappedIdTableScan() const; class Runner { public: Runner( QueryPlanSet &plans, QueryOp &op ); diff --git a/jstests/queryoptimizera.js b/jstests/queryoptimizera.js new file mode 100644 index 00000000000..48d7ccf6180 --- /dev/null +++ b/jstests/queryoptimizera.js @@ -0,0 +1,87 @@ +// Check that a warning message about doing a capped collection scan for a query with an _id +// constraint is printed at appropriate times. SERVER-5353 + +function numWarnings() { + logs = db.adminCommand( { getLog:"global" } ).log + ret = 0; + logs.forEach( function( x ) { + if ( x.match( warningMatchRegexp ) ) { + ++ret; + } + } ); + return ret; +} + +collectionNameIndex = 0; + +// Generate a collection name not already present in the log. +do { + testCollectionName = 'jstests_queryoptimizera__' + collectionNameIndex++; + warningMatchString = 'unindexed _id query on capped collection.*collection: test.' + + testCollectionName; + warningMatchRegexp = new RegExp( warningMatchString ); + +} while( numWarnings() > 0 ); + +t = db[ testCollectionName ]; +t.drop(); + +notCappedCollectionName = testCollectionName + '_notCapped'; + +notCapped = db[ notCappedCollectionName ]; +notCapped.drop(); + +db.createCollection( testCollectionName, { capped:true, size:1000 } ); +db.createCollection( notCappedCollectionName, { autoIndexId:false } ); + +t.insert( {} ); +notCapped.insert( {} ); + +oldNumWarnings = 0; + +function assertNoNewWarnings() { + assert.eq( oldNumWarnings, numWarnings() ); +} + +function assertNewWarning() { + ++oldNumWarnings; + assert.eq( oldNumWarnings, numWarnings() ); +} + +// Simple _id query without an _id index. +t.find( { _id:0 } ).itcount(); +assertNewWarning(); + +// Simple _id query without an _id index, on a non capped collection. +notCapped.find( { _id:0 } ).itcount(); +assertNoNewWarnings(); + +// A multi field query, including _id. +t.find( { _id:0, a:0 } ).itcount(); +assertNewWarning(); + +// An unsatisfiable query. +t.find( { _id:0, a:{$in:[]} } ).itcount(); +assertNoNewWarnings(); + +// An hinted query. +t.find( { _id:0 } ).hint( { $natural:1 } ).itcount(); +assertNoNewWarnings(); + +// Retry a multi field query. +t.find( { _id:0, a:0 } ).itcount(); +assertNewWarning(); + +// Warnings should not be printed when an index is added on _id. +t.ensureIndex( { _id:1 } ); + +t.find( { _id:0 } ).itcount(); +assertNoNewWarnings(); + +t.find( { _id:0, a:0 } ).itcount(); +assertNoNewWarnings(); + +t.find( { _id:0, a:0 } ).itcount(); +assertNoNewWarnings(); + +t.drop(); // cleanup diff --git a/jstests/slowNightly/sharding_passthrough.js b/jstests/slowNightly/sharding_passthrough.js index d81df685bc5..fa1f7bc9a99 100644 --- a/jstests/slowNightly/sharding_passthrough.js +++ b/jstests/slowNightly/sharding_passthrough.js @@ -72,7 +72,7 @@ files.forEach( return; } // These aren't supposed to get run under sharding: - if (/[\/\\](dbadmin|error1|fsync|fsync2|geo.*|indexh|remove5|update4|notablescan|compact.*|check_shard_index|bench_test.*|mr_replaceIntoDB)\.js$/.test(x.name)) { + if (/[\/\\](dbadmin|error1|fsync|fsync2|geo.*|indexh|remove5|update4|notablescan|compact.*|check_shard_index|bench_test.*|mr_replaceIntoDB|queryoptimizera)\.js$/.test(x.name)) { print(" >>>>>>>>>>>>>>> skipping test that would fail under sharding " + x.name) return; } diff --git a/shell/utils.js b/shell/utils.js index 7d7a23b6063..4e206034d2a 100644 --- a/shell/utils.js +++ b/shell/utils.js @@ -739,7 +739,8 @@ if ( typeof _threadInject != "undefined" ){ "jstests/notablescan.js", "jstests/drop2.js", "jstests/dropdb_race.js", - "jstests/bench_test1.js"] ); + "jstests/bench_test1.js", + "jstests/queryoptimizera.js"] ); // some tests can't be run in parallel with each other var serialTestsArr = [ "jstests/fsync.js", |