Skip to content

Commit 7e90a89

Browse files
committed
SERVER-13764 update system retrieves nscanned / nscannedObjects from plan executor
1 parent 1f5be13 commit 7e90a89

File tree

3 files changed

+65
-23
lines changed

3 files changed

+65
-23
lines changed

jstests/core/profile5.js

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// Basic test to ensure that the nscanned and nscannedObjects are tracked
2+
// correctly for updates.
3+
4+
var t = db.jstests_update_profile;
5+
t.drop();
6+
7+
// Turn off profiling so that we can drop the profiler's collection. Then
8+
// enable profiling again. This is OK because this test is blacklisted for the
9+
// parallel suite.
10+
db.setProfilingLevel(0);
11+
db.system.profile.drop();
12+
db.setProfilingLevel(2);
13+
14+
for (var i = 0; i < 5; i++) {
15+
t.insert({x: i});
16+
}
17+
18+
// No index---this update will do a collection scan.
19+
t.update({x: {$gt: 3}}, {$set: {y: true}}, {multi: true});
20+
21+
printjson(t.find().toArray());
22+
23+
assert.eq(1, db.system.profile.count({op: "update"}),
24+
"expected exactly one update op in system.profile");
25+
var prof = db.system.profile.findOne({op: "update"});
26+
printjson(prof);
27+
28+
// Since we're doing a collection scan, we should have examined zero
29+
// index keys and all 5 documents.
30+
assert.eq(0, prof.nscanned, "wrong nscanned");
31+
assert.eq(5, prof.nscannedObjects, "wrong nscannedObjects");
32+
33+
// Disable profiling and drop the profiler data.
34+
db.setProfilingLevel(0);
35+
db.system.profile.drop();

jstests/libs/parallelTester.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,9 +119,13 @@ if ( typeof _threadInject != "undefined" ){
119119
"recstore.js",
120120
"extent.js",
121121
"indexb.js",
122+
123+
// tests turn on profiling
122124
"profile1.js",
123125
"profile3.js",
124126
"profile4.js",
127+
"profile5.js",
128+
125129
"mr_drop.js",
126130
"mr3.js",
127131
"indexh.js",

src/mongo/db/ops/update.cpp

Lines changed: 26 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@
4343
#include "mongo/db/ops/update_driver.h"
4444
#include "mongo/db/ops/update_executor.h"
4545
#include "mongo/db/ops/update_lifecycle.h"
46-
#include "mongo/db/query/get_runner.h"
46+
#include "mongo/db/query/explain.h"
47+
#include "mongo/db/query/get_executor.h"
4748
#include "mongo/db/query/lite_parsed_query.h"
4849
#include "mongo/db/query/query_planner_common.h"
4950
#include "mongo/db/repl/repl_coordinator_global.h"
@@ -455,19 +456,20 @@ namespace mongo {
455456
driver->refreshIndexKeys(lifecycle->getIndexKeys());
456457
}
457458

458-
Runner* rawRunner;
459+
PlanExecutor* rawExec;
459460
Status status = cq ?
460-
getRunner(txn, collection, cqHolder.release(), &rawRunner) :
461-
getRunner(txn, collection, nsString.ns(), request.getQuery(), &rawRunner, &cq);
461+
getExecutor(txn, collection, cqHolder.release(), &rawExec) :
462+
getExecutor(txn, collection, nsString.ns(), request.getQuery(), &rawExec);
462463
uassert(17243,
463-
"could not get runner " + request.getQuery().toString() + "; " + causedBy(status),
464+
"could not get executor" + request.getQuery().toString() + "; " + causedBy(status),
464465
status.isOK());
465466

466-
// Create the runner and setup all deps.
467-
auto_ptr<Runner> runner(rawRunner);
467+
// Create the plan executor and setup all deps.
468+
auto_ptr<PlanExecutor> exec(rawExec);
468469

469-
// Register Runner with ClientCursor
470-
const ScopedRunnerRegistration safety(runner.get());
470+
// Get the canonical query which the underlying executor is using. This may be NULL in
471+
// the case of idhack updates.
472+
cq = exec->getCanonicalQuery();
471473

472474
//
473475
// We'll start assuming we have one or more documents for this update. (Otherwise,
@@ -498,10 +500,13 @@ namespace mongo {
498500
// update if we throw a page fault exception below, and we rely on these counters
499501
// reflecting only the actions taken locally. In particlar, we must have the no-op
500502
// counter reset so that we can meaningfully comapre it with numMatched above.
501-
opDebug->nscanned = 0;
502-
opDebug->nscannedObjects = 0;
503503
opDebug->nModified = 0;
504504

505+
// -1 for these fields means that we don't have a value. Once the update completes
506+
// we request these values from the plan executor.
507+
opDebug->nscanned = -1;
508+
opDebug->nscannedObjects = -1;
509+
505510
// Get the cached document from the update driver.
506511
mutablebson::Document& doc = driver->getDocument();
507512
mutablebson::DamageVector damages;
@@ -521,7 +526,7 @@ namespace mongo {
521526
while (true) {
522527
// Get next doc, and location
523528
DiskLoc loc;
524-
state = runner->getNext(&oldObj, &loc);
529+
state = exec->getNext(&oldObj, &loc);
525530

526531
if (state != Runner::RUNNER_ADVANCED) {
527532
if (state == Runner::RUNNER_EOF) {
@@ -540,14 +545,6 @@ namespace mongo {
540545
continue;
541546
}
542547

543-
// We count how many documents we scanned even though we may skip those that are
544-
// deemed duplicated. The final 'numMatched' and 'nscanned' numbers may differ for
545-
// that reason.
546-
// TODO: Do we want to pull this out of the underlying query plan?
547-
opDebug->nscanned++;
548-
549-
// Found a matching document
550-
opDebug->nscannedObjects++;
551548
numMatched++;
552549

553550
// Ask the driver to apply the mods. It may be that the driver can apply those "in
@@ -624,7 +621,7 @@ namespace mongo {
624621
}
625622

626623
// Save state before making changes
627-
runner->saveState();
624+
exec->saveState();
628625

629626
if (inPlace && !driver->modsAffectIndices()) {
630627
// If a set of modifiers were all no-ops, we are still 'in place', but there is
@@ -670,8 +667,8 @@ namespace mongo {
670667

671668
// Restore state after modification
672669
uassert(17278,
673-
"Update could not restore runner state after updating a document.",
674-
runner->restoreState(txn));
670+
"Update could not restore plan executor state after updating a document.",
671+
exec->restoreState());
675672

676673
// Call logOp if requested.
677674
if (request.shouldCallLogOp() && !logObj.isEmpty()) {
@@ -692,6 +689,12 @@ namespace mongo {
692689
txn->recoveryUnit()->commitIfNeeded();
693690
}
694691

692+
// Get summary information about the plan.
693+
PlanSummaryStats stats;
694+
Explain::getSummaryStats(exec.get(), &stats);
695+
opDebug->nscanned = stats.totalKeysExamined;
696+
opDebug->nscannedObjects = stats.totalDocsExamined;
697+
695698
// TODO: Can this be simplified?
696699
if ((numMatched > 0) || (numMatched == 0 && !request.isUpsert()) ) {
697700
opDebug->nMatched = numMatched;

0 commit comments

Comments
 (0)