SERVER-15017: benchRun might return incorrect stats values

1 files changed, 68 insertions, 56 deletions

diff --git a/src/mongo/shell/bench.cpp b/src/mongo/shell/bench.cpp
index 07171e0c834..1ef831adf28 100644
--- a/src/mongo/shell/bench.cpp
+++ b/src/mongo/shell/bench.cpp
@@ -103,13 +103,14 @@ namespace mongo {
     void BenchRunStats::reset() {
         error = false;
         errCount = 0;
+        opCount = 0;
 
         findOneCounter.reset();
         updateCounter.reset();
         insertCounter.reset();
         deleteCounter.reset();
         queryCounter.reset();
-
+        commandCounter.reset();
         trappedErrors.clear();
     }
 
@@ -117,12 +118,14 @@ namespace mongo {
         if (other.error)
             error = true;
         errCount += other.errCount;
+        opCount += other.opCount;
 
         findOneCounter.updateFrom(other.findOneCounter);
         updateCounter.updateFrom(other.updateCounter);
         insertCounter.updateFrom(other.insertCounter);
         deleteCounter.updateFrom(other.deleteCounter);
         queryCounter.updateFrom(other.queryCounter);
+        commandCounter.updateFrom(other.commandCounter);
 
         for (size_t i = 0; i < other.trappedErrors.size(); ++i)
             trappedErrors.push_back(other.trappedErrors[i]);
@@ -139,7 +142,7 @@ namespace mongo {
         password = "";
 
         parallel = 1;
-        seconds = 1;
+        seconds = 1.0;
         hideResults = true;
         handleErrors = false;
         hideErrors = false;
@@ -234,7 +237,8 @@ namespace mongo {
         : _mutex(),
           _numUnstartedWorkers( numWorkers ),
           _numActiveWorkers( 0 ),
-          _isShuttingDown( 0 ) {
+          _isShuttingDown( 0 ), 
+          _isCollectingStats( 0 ) {
     }
 
     BenchRunState::~BenchRunState() {
@@ -265,6 +269,10 @@ namespace mongo {
         _isShuttingDown.store( 1 );
     }
 
+    void BenchRunState::tellWorkersToCollectStats() {
+        _isCollectingStats.store( 1 );
+    }
+
     void BenchRunState::assertFinished() {
         boost::lock_guard<boost::mutex> lk(_mutex);
         verify(0 == _numUnstartedWorkers + _numActiveWorkers);
@@ -274,6 +282,10 @@ namespace mongo {
         return (_isShuttingDown.loadRelaxed() == 1);
     }
 
+    bool BenchRunState::shouldWorkerCollectStats() {
+        return (_isCollectingStats.loadRelaxed() == 1);
+    }
+
     void BenchRunState::onWorkerStarted() {
         boost::lock_guard<boost::mutex> lk(_mutex);
         verify( _numUnstartedWorkers > 0 );
@@ -336,6 +348,10 @@ namespace mongo {
         return _brState->shouldWorkerFinish();
     }
 
+    bool BenchRunWorker::shouldCollectStats() const {
+        return _brState->shouldWorkerCollectStats();
+    }
+
     void doNothing(const BSONObj&) { }
 
     void BenchRunWorker::generateLoadOnConnection( DBClientBase* conn ) {
@@ -358,7 +374,7 @@ namespace mongo {
             while ( i.more() ) {
 
                 if ( shouldStop() ) break;
-
+                auto& stats = shouldCollectStats() ? _stats : _statsBlackHole;
                 BSONElement e = i.next();
 
                 string ns = e["ns"].String();
@@ -407,7 +423,7 @@ namespace mongo {
 
                         BSONObj result;
                         {
-                            BenchRunEventTrace _bret(&_stats.findOneCounter);
+                            BenchRunEventTrace _bret(&stats.findOneCounter);
                             result = conn->findOne( ns , fixQuery( e["query"].Obj(),
                                                                    bsonTemplateEvaluator ) );
                         }
@@ -417,7 +433,7 @@ namespace mongo {
                             if( err ){
                                 log() << "Error checking in benchRun thread [findOne]" << causedBy( scope->getError() ) << endl;
 
-                                _stats.errCount++;
+                                stats.errCount++;
 
                                 return;
                             }
@@ -428,16 +444,25 @@ namespace mongo {
                     }
                     else if ( op == "command" ) {
 
+                        bool ok;
                         BSONObj result;
-                        conn->runCommand( ns, fixQuery( e["command"].Obj(), bsonTemplateEvaluator ),
-                                          result, e["options"].numberInt() );
-
-                        if( check ){
+                        {
+                            BenchRunEventTrace _bret(&stats.commandCounter);
+                            ok = conn->runCommand( ns,
+                                                   fixQuery( e["command"].Obj(),
+                                                             bsonTemplateEvaluator ),
+                                                   result,
+                                                   e["options"].numberInt());
+                        }
+                        if (!ok) {
+                            stats.errCount++;
+                        }
+                        else if( check ){
                             int err = scope->invoke( scopeFunc , 0 , &result,  1000 * 60 , false );
                             if( err ){
                                 log() << "Error checking in benchRun thread [command]" << causedBy( scope->getError() ) << endl;
 
-                                _stats.errCount++;
+                                stats.errCount++;
 
                                 return;
                             }
@@ -462,12 +487,12 @@ namespace mongo {
 
                         // use special query function for exhaust query option
                         if (options & QueryOption_Exhaust) {
-                            BenchRunEventTrace _bret(&_stats.queryCounter);
+                            BenchRunEventTrace _bret(&stats.queryCounter);
                             stdx::function<void (const BSONObj&)> castedDoNothing(doNothing);
                             count =  conn->query(castedDoNothing, ns, fixedQuery, &filter, options);
                         }
                         else {
-                            BenchRunEventTrace _bret(&_stats.queryCounter);
+                            BenchRunEventTrace _bret(&stats.queryCounter);
                             cursor = conn->query(ns, fixedQuery, limit, skip, &filter, options,
                                                  batchSize);
                             count = cursor->itcount();
@@ -484,7 +509,7 @@ namespace mongo {
                             if( err ){
                                 log() << "Error checking in benchRun thread [find]" << causedBy( scope->getError() ) << endl;
 
-                                _stats.errCount++;
+                                stats.errCount++;
 
                                 return;
                             }
@@ -503,7 +528,7 @@ namespace mongo {
                         bool safe = e["safe"].trueValue();
 
                         {
-                            BenchRunEventTrace _bret(&_stats.updateCounter);
+                            BenchRunEventTrace _bret(&stats.updateCounter);
                             BSONObj query = fixQuery(queryOrginal, bsonTemplateEvaluator);
                             BSONObj update = fixQuery(updateOriginal, bsonTemplateEvaluator);
 
@@ -537,7 +562,7 @@ namespace mongo {
                                 if( err ){
                                     log() << "Error checking in benchRun thread [update]" << causedBy( scope->getError() ) << endl;
 
-                                    _stats.errCount++;
+                                    stats.errCount++;
 
                                     return;
                                 }
@@ -555,7 +580,7 @@ namespace mongo {
                         BSONObj result;
 
                         {
-                            BenchRunEventTrace _bret(&_stats.insertCounter);
+                            BenchRunEventTrace _bret(&stats.insertCounter);
 
                             BSONObj insertDoc = fixQuery(e["doc"].Obj(), bsonTemplateEvaluator);
 
@@ -584,7 +609,7 @@ namespace mongo {
                                 if( err ){
                                     log() << "Error checking in benchRun thread [insert]" << causedBy( scope->getError() ) << endl;
 
-                                    _stats.errCount++;
+                                    stats.errCount++;
 
                                     return;
                                 }
@@ -604,7 +629,7 @@ namespace mongo {
                         bool safe = e["safe"].trueValue();
                         BSONObj result;
                         {
-                            BenchRunEventTrace _bret(&_stats.deleteCounter);
+                            BenchRunEventTrace _bret(&stats.deleteCounter);
                             BSONObj predicate = fixQuery(query, bsonTemplateEvaluator);
                             if (useWriteCmd) {
 
@@ -636,7 +661,7 @@ namespace mongo {
                                 if( err ){
                                     log() << "Error checking in benchRun thread [delete]" << causedBy( scope->getError() ) << endl;
 
-                                    _stats.errCount++;
+                                    stats.errCount++;
 
                                     return;
                                 }
@@ -666,9 +691,11 @@ namespace mongo {
                     }
                     else {
                         log() << "don't understand op: " << op << endl;
-                        _stats.error = true;
+                        stats.error = true;
                         return;
                     }
+                    // Count 1 for total ops. Successfully got through the try phrase
+                    stats.opCount++;
                 }
                 catch( DBException& ex ){
                     if( ! _config->hideErrors || e["showError"].trueValue() ){
@@ -689,19 +716,19 @@ namespace mongo {
                         ( ! _config->noTrapPattern && _config->trapPattern && yesTrap ) ||
                         ( _config->trapPattern && _config->noTrapPattern && yesTrap && ! noTrap ) ){
                         {
-                            _stats.trappedErrors.push_back( BSON( "error" << ex.what() << "op" << e << "count" << count ) );
+                            stats.trappedErrors.push_back( BSON( "error" << ex.what() << "op" << e << "count" << count ) );
                         }
                         if( _config->breakOnTrap ) return;
                     }
                     if( ! _config->handleErrors && ! e["handleError"].trueValue() ) return;
 
-                    _stats.errCount++;
+                    stats.errCount++;
                 }
                 catch( ... ){
                     if( ! _config->hideErrors || e["showError"].trueValue() ) log() << "Error in benchRun thread caused by unknown error for op " << e << endl;
                     if( ! _config->handleErrors && ! e["handleError"].trueValue() ) return;
 
-                    _stats.errCount++;
+                    stats.errCount++;
                 }
 
                 if (++count % 100 == 0 && !useWriteCmd) {
@@ -736,7 +763,6 @@ namespace mongo {
 
     void BenchRunWorker::run() {
         try {
-            BenchRunWorkerStateGuard _workerStateGuard( _brState );
             std::unique_ptr<DBClientBase> conn( _config->createConnection() );
             if ( !_config->username.empty() ) {
                 string errmsg;
@@ -745,6 +771,7 @@ namespace mongo {
                               "Authenticating to connection for benchThread failed: " + errmsg);
                 }
             }
+            BenchRunWorkerStateGuard _workerStateGuard( _brState );
             generateLoadOnConnection( conn.get() );
         }
         catch( DBException& e ){
@@ -800,8 +827,7 @@ namespace mongo {
              _brState.waitForState(BenchRunState::BRS_RUNNING);
 
              // initial stats
-             conn->simpleCommand( "admin" , &before , "serverStatus" );
-             before = before.getOwned();
+             _brState.tellWorkersToCollectStats();
              _brTimer = new mongo::Timer();
          }
      }
@@ -824,9 +850,6 @@ namespace mongo {
                                "still required to use benchRun with auth enabled");
                  }
              }
-             // Get final stats
-             conn->simpleCommand( "admin" , &after , "serverStatus" );
-             after = after.getOwned();
          }
 
          {
@@ -850,17 +873,6 @@ namespace mongo {
         stats->reset();
         for ( size_t i = 0; i < _workers.size(); ++i )
             stats->updateFrom( _workers[i]->stats() );
-        BSONObj before = this->before["opcounters"].Obj();
-        BSONObj after = this->after["opcounters"].Obj();
-        {
-             BSONObjIterator i( after );
-             while ( i.more() ) {
-                 BSONElement e = i.next();
-                 long long delta = e.numberLong();
-                 delta -= before[e.fieldName()].numberLong();
-                 stats->opcounters[e.fieldName()] = delta;
-             }
-        }
     }
 
      static void appendAverageMicrosIfAvailable(
@@ -884,30 +896,30 @@ namespace mongo {
          if ( error )
              return BSON( "err" << 1 );
 
-         // compute actual ops/sec
-         BSONObj before = runner->before["opcounters"].Obj();
-         BSONObj after = runner->after["opcounters"].Obj();
-
          BSONObjBuilder buf;
          buf.append( "note" , "values per second" );
-         buf.append( "errCount", (long long) stats.errCount );
+         buf.append( "errCount", static_cast<long long>(stats.errCount) );
          buf.append( "trapped", "error: not implemented" );
          appendAverageMicrosIfAvailable(buf, "findOneLatencyAverageMicros", stats.findOneCounter);
          appendAverageMicrosIfAvailable(buf, "insertLatencyAverageMicros", stats.insertCounter);
          appendAverageMicrosIfAvailable(buf, "deleteLatencyAverageMicros", stats.deleteCounter);
          appendAverageMicrosIfAvailable(buf, "updateLatencyAverageMicros", stats.updateCounter);
          appendAverageMicrosIfAvailable(buf, "queryLatencyAverageMicros", stats.queryCounter);
+         appendAverageMicrosIfAvailable(buf, "commandsLatencyAverageMicros", stats.commandCounter);
 
-         {
-             BSONObjIterator i( after );
-             while ( i.more() ) {
-                 BSONElement e = i.next();
-                 double x = e.number();
-                 x -= before[e.fieldName()].number();
-                 std::string s = e.fieldName();
-                 buf.append( s, x / (runner->_microsElapsed / 1000000.0) );
-             }
-         }
+         buf.append("totalOps", static_cast<long long>(stats.opCount));
+
+         auto appendPerSec = [&buf, runner](StringData name, double total) {
+             buf.append(name, total / (runner->_microsElapsed / 1000000.0));
+         };
+
+         appendPerSec("totalOps/s", stats.opCount);
+         appendPerSec("findOne", stats.findOneCounter.getNumEvents());
+         appendPerSec("insert", stats.insertCounter.getNumEvents());
+         appendPerSec("delete", stats.deleteCounter.getNumEvents());
+         appendPerSec("update", stats.updateCounter.getNumEvents());
+         appendPerSec("query", stats.queryCounter.getNumEvents());
+         appendPerSec("command", stats.commandCounter.getNumEvents());
 
          BSONObj zoo = buf.obj();