SERVER-31170 Cleanup bench_run.cpp

10 files changed, 246 insertions, 307 deletions

diff --git a/src/mongo/db/db.cpp b/src/mongo/db/db.cpp
index 3013b1dc333..b71537cb235 100644
--- a/src/mongo/db/db.cpp
+++ b/src/mongo/db/db.cpp
@@ -168,16 +168,8 @@
 
 namespace mongo {
 
-using std::unique_ptr;
-using std::cout;
-using std::cerr;
-using std::endl;
-using std::list;
-using std::string;
-using std::stringstream;
-using std::vector;
-
 using logger::LogComponent;
+using std::endl;
 
 namespace {
 
@@ -185,13 +177,13 @@ const NamespaceString startupLogCollectionName("local.startup_log");
 const NamespaceString kSystemReplSetCollection("local.system.replset");
 
 #ifdef _WIN32
-ntservice::NtServiceDefaultStrings defaultServiceStrings = {
+const ntservice::NtServiceDefaultStrings defaultServiceStrings = {
     L"MongoDB", L"MongoDB", L"MongoDB Server"};
 #endif
 
 void logStartup(OperationContext* opCtx) {
     BSONObjBuilder toLog;
-    stringstream id;
+    std::stringstream id;
     id << getHostNameCached() << "-" << jsTime().asInt64();
     toLog.append("_id", id.str());
     toLog.append("hostname", getHostNameCached());
@@ -241,12 +233,11 @@ void checkForIdIndexesAndDropPendingCollections(OperationContext* opCtx, Databas
         return;
     }
 
-    list<string> collections;
-    db->getDatabaseCatalogEntry()->getCollectionNamespaces(&collections);
+    std::list<std::string> collectionNames;
+    db->getDatabaseCatalogEntry()->getCollectionNamespaces(&collectionNames);
 
-    for (list<string>::iterator i = collections.begin(); i != collections.end(); ++i) {
-        const string& collectionName = *i;
-        NamespaceString ns(collectionName);
+    for (const auto& collectionName : collectionNames) {
+        const NamespaceString ns(collectionName);
 
         if (ns.isDropPendingNamespace()) {
             auto dropOpTime = fassertStatusOK(40459, ns.getDropPendingNamespaceOpTime());
@@ -264,7 +255,7 @@ void checkForIdIndexesAndDropPendingCollections(OperationContext* opCtx, Databas
         if (coll->getIndexCatalog()->findIdIndex(opCtx))
             continue;
 
-        log() << "WARNING: the collection '" << *i << "' lacks a unique index on _id."
+        log() << "WARNING: the collection '" << collectionName << "' lacks a unique index on _id."
               << " This index is needed for replication to function properly" << startupWarningsLog;
         log() << "\t To fix this, you need to create a unique index on _id."
               << " See http://dochub.mongodb.org/core/build-replica-set-indexes"
@@ -307,20 +298,18 @@ void checkForCappedOplog(OperationContext* opCtx, Database* db) {
 void repairDatabasesAndCheckVersion(OperationContext* opCtx) {
     LOG(1) << "enter repairDatabases (to check pdfile version #)";
 
-    Lock::GlobalWrite lk(opCtx);
+    auto const storageEngine = opCtx->getServiceContext()->getGlobalStorageEngine();
 
-    vector<string> dbNames;
+    Lock::GlobalWrite lk(opCtx);
 
-    StorageEngine* storageEngine = opCtx->getServiceContext()->getGlobalStorageEngine();
+    std::vector<std::string> dbNames;
     storageEngine->listDatabases(&dbNames);
 
     // Repair all databases first, so that we do not try to open them if they are in bad shape
     if (storageGlobalParams.repair) {
         invariant(!storageGlobalParams.readOnly);
-        for (vector<string>::const_iterator i = dbNames.begin(); i != dbNames.end(); ++i) {
-            const string dbName = *i;
+        for (const auto& dbName : dbNames) {
             LOG(1) << "    Repairing database: " << dbName;
-
             fassert(18506, repairDatabase(opCtx, storageEngine, dbName));
         }
     }
@@ -376,8 +365,7 @@ void repairDatabasesAndCheckVersion(OperationContext* opCtx) {
     // To print warning later if any database collections have UUIDs in FCV 3.4.
     bool collsHaveUuids = false;
 
-    for (vector<string>::const_iterator i = dbNames.begin(); i != dbNames.end(); ++i) {
-        const string dbName = *i;
+    for (const auto& dbName : dbNames) {
         LOG(1) << "    Recovering database: " << dbName;
 
         Database* db = dbHolder().openDb(opCtx, dbName);
@@ -463,7 +451,7 @@ void repairDatabasesAndCheckVersion(OperationContext* opCtx) {
         PlanExecutor::ExecState state;
         while (PlanExecutor::ADVANCED == (state = exec->getNext(&index, NULL))) {
             const BSONObj key = index.getObjectField("key");
-            const string plugin = IndexNames::findPluginName(key);
+            const auto plugin = IndexNames::findPluginName(key);
 
             if (db->getDatabaseCatalogEntry()->isOlderThan24(opCtx)) {
                 if (IndexNames::existedBefore24(plugin)) {
@@ -520,18 +508,17 @@ ExitCode _initAndListen(int listenPort) {
     Client::initThread("initandlisten");
 
     initWireSpec();
-    auto globalServiceContext = checked_cast<ServiceContextMongoD*>(getGlobalServiceContext());
+    auto serviceContext = checked_cast<ServiceContextMongoD*>(getGlobalServiceContext());
 
-    globalServiceContext->setFastClockSource(FastClockSourceFactory::create(Milliseconds(10)));
-    globalServiceContext->setOpObserver(stdx::make_unique<OpObserverImpl>());
+    serviceContext->setFastClockSource(FastClockSourceFactory::create(Milliseconds(10)));
+    serviceContext->setOpObserver(stdx::make_unique<OpObserverImpl>());
 
-    DBDirectClientFactory::get(globalServiceContext)
-        .registerImplementation([](OperationContext* opCtx) {
-            return std::unique_ptr<DBClientBase>(new DBDirectClient(opCtx));
-        });
+    DBDirectClientFactory::get(serviceContext).registerImplementation([](OperationContext* opCtx) {
+        return std::unique_ptr<DBClientBase>(new DBDirectClient(opCtx));
+    });
 
     const repl::ReplSettings& replSettings =
-        repl::ReplicationCoordinator::get(globalServiceContext)->getSettings();
+        repl::ReplicationCoordinator::get(serviceContext)->getSettings();
 
     {
         ProcessId pid = ProcessId::getCurrent();
@@ -555,24 +542,26 @@ ExitCode _initAndListen(int listenPort) {
 
     logProcessDetails();
 
-    globalServiceContext->createLockFile();
+    serviceContext->createLockFile();
 
-    globalServiceContext->setServiceEntryPoint(
-        stdx::make_unique<ServiceEntryPointMongod>(globalServiceContext));
+    serviceContext->setServiceEntryPoint(
+        stdx::make_unique<ServiceEntryPointMongod>(serviceContext));
 
-    auto tl = transport::TransportLayerManager::createWithConfig(&serverGlobalParams,
-                                                                 globalServiceContext);
-    auto res = tl->setup();
-    if (!res.isOK()) {
-        error() << "Failed to set up listener: " << res;
-        return EXIT_NET_ERROR;
+    {
+        auto tl =
+            transport::TransportLayerManager::createWithConfig(&serverGlobalParams, serviceContext);
+        auto res = tl->setup();
+        if (!res.isOK()) {
+            error() << "Failed to set up listener: " << res;
+            return EXIT_NET_ERROR;
+        }
+        serviceContext->setTransportLayer(std::move(tl));
     }
-    globalServiceContext->setTransportLayer(std::move(tl));
 
-    globalServiceContext->initializeGlobalStorageEngine();
+    serviceContext->initializeGlobalStorageEngine();
 
 #ifdef MONGO_CONFIG_WIREDTIGER_ENABLED
-    if (EncryptionHooks::get(getGlobalServiceContext())->restartRequired()) {
+    if (EncryptionHooks::get(serviceContext)->restartRequired()) {
         exitCleanly(EXIT_CLEAN);
     }
 #endif
@@ -592,7 +581,7 @@ ExitCode _initAndListen(int listenPort) {
             }
 
             // Warn if field name matches non-active registered storage engine.
-            if (globalServiceContext->isRegisteredStorageEngine(e.fieldName())) {
+            if (serviceContext->isRegisteredStorageEngine(e.fieldName())) {
                 warning() << "Detected configuration for non-active storage engine "
                           << e.fieldName() << " when current storage engine is "
                           << storageGlobalParams.engine;
@@ -600,7 +589,7 @@ ExitCode _initAndListen(int listenPort) {
         }
     }
 
-    if (!globalServiceContext->getGlobalStorageEngine()->getSnapshotManager()) {
+    if (!serviceContext->getGlobalStorageEngine()->getSnapshotManager()) {
         if (moe::startupOptionsParsed.count("replication.enableMajorityReadConcern") &&
             moe::startupOptionsParsed["replication.enableMajorityReadConcern"].as<bool>()) {
             // Note: we are intentionally only erroring if the user explicitly requested that we
@@ -615,10 +604,10 @@ ExitCode _initAndListen(int listenPort) {
         }
     }
 
-    logMongodStartupWarnings(storageGlobalParams, serverGlobalParams, globalServiceContext);
+    logMongodStartupWarnings(storageGlobalParams, serverGlobalParams, serviceContext);
 
     {
-        stringstream ss;
+        std::stringstream ss;
         ss << endl;
         ss << "*********************************************************************" << endl;
         ss << " ERROR: dbpath (" << storageGlobalParams.dbpath << ") does not exist." << endl;
@@ -629,7 +618,7 @@ ExitCode _initAndListen(int listenPort) {
     }
 
     {
-        stringstream ss;
+        std::stringstream ss;
         ss << "repairpath (" << storageGlobalParams.repairpath << ") does not exist";
         uassert(12590, ss.str().c_str(), boost::filesystem::exists(storageGlobalParams.repairpath));
     }
@@ -647,7 +636,7 @@ ExitCode _initAndListen(int listenPort) {
         ScriptEngine::setup();
     }
 
-    auto startupOpCtx = globalServiceContext->makeOperationContext(&cc());
+    auto startupOpCtx = serviceContext->makeOperationContext(&cc());
 
     repairDatabasesAndCheckVersion(startupOpCtx.get());
 
@@ -659,7 +648,7 @@ ExitCode _initAndListen(int listenPort) {
     // Start up health log writer thread.
     HealthLog::get(startupOpCtx.get()).startup();
 
-    auto const globalAuthzManager = AuthorizationManager::get(globalServiceContext);
+    auto const globalAuthzManager = AuthorizationManager::get(serviceContext);
     uassertStatusOK(globalAuthzManager->initialize(startupOpCtx.get()));
 
     // This is for security on certain platforms (nonce generation)
@@ -712,7 +701,7 @@ ExitCode _initAndListen(int listenPort) {
               << startupWarningsLog;
     }
 
-    SessionCatalog::create(globalServiceContext);
+    SessionCatalog::create(serviceContext);
 
     // This function may take the global lock.
     auto shardingInitialized =
@@ -772,7 +761,7 @@ ExitCode _initAndListen(int listenPort) {
             storageGlobalParams.engine != "devnull") {
             Lock::GlobalWrite lk(startupOpCtx.get());
             FeatureCompatibilityVersion::setIfCleanStartup(
-                startupOpCtx.get(), repl::StorageInterface::get(globalServiceContext));
+                startupOpCtx.get(), repl::StorageInterface::get(serviceContext));
         }
 
         if (replSettings.usingReplSets() || (!replSettings.isMaster() && replSettings.isSlave()) ||
@@ -788,10 +777,10 @@ ExitCode _initAndListen(int listenPort) {
     // Set up the periodic runner for background job execution
     auto runner = makePeriodicRunner();
     runner->startup().transitional_ignore();
-    globalServiceContext->setPeriodicRunner(std::move(runner));
+    serviceContext->setPeriodicRunner(std::move(runner));
 
-    SessionKiller::set(globalServiceContext,
-                       std::make_shared<SessionKiller>(globalServiceContext, killSessionsLocal));
+    SessionKiller::set(serviceContext,
+                       std::make_shared<SessionKiller>(serviceContext, killSessionsLocal));
 
     // Set up the logical session cache
     LogicalSessionCacheServer kind = LogicalSessionCacheServer::kStandalone;
@@ -801,26 +790,26 @@ ExitCode _initAndListen(int listenPort) {
         kind = LogicalSessionCacheServer::kReplicaSet;
     }
 
-    auto sessionCache = makeLogicalSessionCacheD(globalServiceContext, kind);
-    LogicalSessionCache::set(globalServiceContext, std::move(sessionCache));
+    auto sessionCache = makeLogicalSessionCacheD(serviceContext, kind);
+    LogicalSessionCache::set(serviceContext, std::move(sessionCache));
 
     // MessageServer::run will return when exit code closes its socket and we don't need the
     // operation context anymore
     startupOpCtx.reset();
 
-    auto start = globalServiceContext->getTransportLayer()->start();
+    auto start = serviceContext->getTransportLayer()->start();
     if (!start.isOK()) {
         error() << "Failed to start the listener: " << start.toString();
         return EXIT_NET_ERROR;
     }
 
-    start = globalServiceContext->getServiceExecutor()->start();
+    start = serviceContext->getServiceExecutor()->start();
     if (!start.isOK()) {
         error() << "Failed to start the service executor: " << start;
         return EXIT_NET_ERROR;
     }
 
-    globalServiceContext->notifyStartupComplete();
+    serviceContext->notifyStartupComplete();
 
 #ifndef _WIN32
     mongo::signalForkSuccess();
@@ -879,21 +868,21 @@ void startupConfigActions(const std::vector<std::string>& args) {
     // and "dbppath" command.  The "run" command is the same as just running mongod, so just
     // falls through.
     if (moe::startupOptionsParsed.count("command")) {
-        vector<string> command = moe::startupOptionsParsed["command"].as<vector<string>>();
+        const auto command = moe::startupOptionsParsed["command"].as<std::vector<std::string>>();
 
         if (command[0].compare("dbpath") == 0) {
-            cout << storageGlobalParams.dbpath << endl;
+            std::cout << storageGlobalParams.dbpath << endl;
             quickExit(EXIT_SUCCESS);
         }
 
         if (command[0].compare("run") != 0) {
-            cout << "Invalid command: " << command[0] << endl;
+            std::cout << "Invalid command: " << command[0] << endl;
             printMongodHelp(moe::startupOptions);
             quickExit(EXIT_FAILURE);
         }
 
         if (command.size() > 1) {
-            cout << "Too many parameters to 'run' command" << endl;
+            std::cout << "Too many parameters to 'run' command" << endl;
             printMongodHelp(moe::startupOptions);
             quickExit(EXIT_FAILURE);
         }
@@ -912,13 +901,13 @@ void startupConfigActions(const std::vector<std::string>& args) {
         moe::startupOptionsParsed["shutdown"].as<bool>() == true) {
         bool failed = false;
 
-        string name =
+        std::string name =
             (boost::filesystem::path(storageGlobalParams.dbpath) / "mongod.lock").string();
         if (!boost::filesystem::exists(name) || boost::filesystem::file_size(name) == 0)
             failed = true;
 
         pid_t pid;
-        string procPath;
+        std::string procPath;
         if (!failed) {
             try {
                 std::ifstream f(name.c_str());
@@ -927,7 +916,8 @@ void startupConfigActions(const std::vector<std::string>& args) {
                 if (!boost::filesystem::exists(procPath))
                     failed = true;
             } catch (const std::exception& e) {
-                cerr << "Error reading pid from lock file [" << name << "]: " << e.what() << endl;
+                std::cerr << "Error reading pid from lock file [" << name << "]: " << e.what()
+                          << endl;
                 failed = true;
             }
         }
@@ -938,11 +928,11 @@ void startupConfigActions(const std::vector<std::string>& args) {
             quickExit(EXIT_FAILURE);
         }
 
-        cout << "killing process with pid: " << pid << endl;
+        std::cout << "killing process with pid: " << pid << endl;
         int ret = kill(pid, SIGTERM);
         if (ret) {
             int e = errno;
-            cerr << "failed to kill process: " << errnoWithDescription(e) << endl;
+            std::cerr << "failed to kill process: " << errnoWithDescription(e) << endl;
             quickExit(EXIT_FAILURE);
         }
 
diff --git a/src/mongo/db/service_context.cpp b/src/mongo/db/service_context.cpp
index 2b4eb81c354..48004bf0174 100644
--- a/src/mongo/db/service_context.cpp
+++ b/src/mongo/db/service_context.cpp
@@ -32,6 +32,7 @@
 
 #include "mongo/bson/bsonobj.h"
 #include "mongo/db/client.h"
+#include "mongo/db/op_observer.h"
 #include "mongo/db/operation_context.h"
 #include "mongo/stdx/memory.h"
 #include "mongo/transport/service_entry_point.h"
@@ -188,17 +189,8 @@ transport::ServiceExecutor* ServiceContext::getServiceExecutor() const {
     return _serviceExecutor.get();
 }
 
-
-TickSource* ServiceContext::getTickSource() const {
-    return _tickSource.get();
-}
-
-ClockSource* ServiceContext::getFastClockSource() const {
-    return _fastClockSource.get();
-}
-
-ClockSource* ServiceContext::getPreciseClockSource() const {
-    return _preciseClockSource.get();
+void ServiceContext::setOpObserver(std::unique_ptr<OpObserver> opObserver) {
+    _opObserver = std::move(opObserver);
 }
 
 void ServiceContext::setTickSource(std::unique_ptr<TickSource> newSource) {
diff --git a/src/mongo/db/service_context.h b/src/mongo/db/service_context.h
index 06c7892c504..b33b3aeeb4b 100644
--- a/src/mongo/db/service_context.h
+++ b/src/mongo/db/service_context.h
@@ -66,6 +66,8 @@ class TransportLayer;
  * including limitations on the lifetime of registered listeners.
  */
 class KillOpListenerInterface {
+    MONGO_DISALLOW_COPYING(KillOpListenerInterface);
+
 public:
     /**
      * Will be called *after* ops have been told they should die.
@@ -75,20 +77,23 @@ public:
     virtual void interruptAll() = 0;
 
 protected:
+    KillOpListenerInterface() = default;
+
     // Should not delete through a pointer of this type
-    virtual ~KillOpListenerInterface() {}
+    virtual ~KillOpListenerInterface() = default;
 };
 
 class StorageFactoriesIterator {
     MONGO_DISALLOW_COPYING(StorageFactoriesIterator);
 
 public:
-    virtual ~StorageFactoriesIterator() {}
+    virtual ~StorageFactoriesIterator() = default;
+
     virtual bool more() const = 0;
     virtual const StorageEngine::Factory* next() = 0;
 
 protected:
-    StorageFactoriesIterator() {}
+    StorageFactoriesIterator() = default;
 };
 
 /**
@@ -392,35 +397,39 @@ public:
      */
     void notifyStartupComplete();
 
-    //
-    // Global OpObserver.
-    //
-
     /**
      * Set the OpObserver.
      */
-    virtual void setOpObserver(std::unique_ptr<OpObserver> opObserver) = 0;
+    void setOpObserver(std::unique_ptr<OpObserver> opObserver);
 
     /**
      * Return the OpObserver instance we're using.
      */
-    virtual OpObserver* getOpObserver() = 0;
+    OpObserver* getOpObserver() const {
+        return _opObserver.get();
+    }
 
     /**
      * Returns the tick/clock source set in this context.
      */
-    TickSource* getTickSource() const;
+    TickSource* getTickSource() const {
+        return _tickSource.get();
+    }
 
     /**
      * Get a ClockSource implementation that may be less precise than the _preciseClockSource but
      * may be cheaper to call.
      */
-    ClockSource* getFastClockSource() const;
+    ClockSource* getFastClockSource() const {
+        return _fastClockSource.get();
+    }
 
     /**
      * Get a ClockSource implementation that is very precise but may be expensive to call.
      */
-    ClockSource* getPreciseClockSource() const;
+    ClockSource* getPreciseClockSource() const {
+        return _preciseClockSource.get();
+    }
 
     /**
      * Replaces the current tick/clock source with a new one. In other words, the old source will be
@@ -474,13 +483,6 @@ private:
     virtual std::unique_ptr<OperationContext> _newOpCtx(Client* client, unsigned opId) = 0;
 
     /**
-     * Kills the given operation.
-     *
-     * Caller must own the service context's _mutex.
-     */
-    void _killOperation_inlock(OperationContext* opCtx, ErrorCodes::Error killCode);
-
-    /**
      * The key manager.
      */
     std::shared_ptr<KeysCollectionManager> _keyManager;
@@ -511,6 +513,11 @@ private:
     std::vector<std::unique_ptr<ClientObserver>> _clientObservers;
     ClientSet _clients;
 
+    /**
+     * The registered OpObserver.
+     */
+    std::unique_ptr<OpObserver> _opObserver;
+
     std::unique_ptr<TickSource> _tickSource;
 
     /**
diff --git a/src/mongo/db/service_context_d.cpp b/src/mongo/db/service_context_d.cpp
index c84f60aad55..56f073630de 100644
--- a/src/mongo/db/service_context_d.cpp
+++ b/src/mongo/db/service_context_d.cpp
@@ -32,22 +32,16 @@
 
 #include "mongo/db/service_context_d.h"
 
-#include <boost/optional.hpp>
-
 #include "mongo/base/init.h"
 #include "mongo/base/initializer.h"
 #include "mongo/db/client.h"
 #include "mongo/db/concurrency/lock_state.h"
-#include "mongo/db/op_observer.h"
-#include "mongo/db/service_context.h"
 #include "mongo/db/service_entry_point_mongod.h"
 #include "mongo/db/storage/storage_engine.h"
 #include "mongo/db/storage/storage_engine_lock_file.h"
 #include "mongo/db/storage/storage_engine_metadata.h"
 #include "mongo/db/storage/storage_options.h"
-#include "mongo/scripting/engine.h"
 #include "mongo/stdx/memory.h"
-#include "mongo/stdx/mutex.h"
 #include "mongo/util/log.h"
 #include "mongo/util/map_util.h"
 #include "mongo/util/mongoutils/str.h"
@@ -72,6 +66,8 @@ MONGO_INITIALIZER(SetGlobalEnvironment)(InitializerContext* context) {
 }
 }  // namespace
 
+extern bool _supportsDocLocking;
+
 ServiceContextMongoD::ServiceContextMongoD() = default;
 
 ServiceContextMongoD::~ServiceContextMongoD() = default;
@@ -83,8 +79,6 @@ StorageEngine* ServiceContextMongoD::getGlobalStorageEngine() {
     return _storageEngine;
 }
 
-extern bool _supportsDocLocking;
-
 void ServiceContextMongoD::createLockFile() {
     try {
         _lockFile.reset(new StorageEngineLockFile(storageGlobalParams.dbpath));
@@ -256,9 +250,8 @@ StorageFactoriesIterator* ServiceContextMongoD::makeStorageFactoriesIterator() {
     return new StorageFactoriesIteratorMongoD(_storageFactories.begin(), _storageFactories.end());
 }
 
-StorageFactoriesIteratorMongoD::StorageFactoriesIteratorMongoD(
-    const ServiceContextMongoD::FactoryMap::const_iterator& begin,
-    const ServiceContextMongoD::FactoryMap::const_iterator& end)
+StorageFactoriesIteratorMongoD::StorageFactoriesIteratorMongoD(const FactoryMapIterator& begin,
+                                                               const FactoryMapIterator& end)
     : _curr(begin), _end(end) {}
 
 bool StorageFactoriesIteratorMongoD::more() const {
@@ -284,12 +277,4 @@ std::unique_ptr<OperationContext> ServiceContextMongoD::_newOpCtx(Client* client
     return opCtx;
 }
 
-void ServiceContextMongoD::setOpObserver(std::unique_ptr<OpObserver> opObserver) {
-    _opObserver = std::move(opObserver);
-}
-
-OpObserver* ServiceContextMongoD::getOpObserver() {
-    return _opObserver.get();
-}
-
 }  // namespace mongo
diff --git a/src/mongo/db/service_context_d.h b/src/mongo/db/service_context_d.h
index 0e96996206a..79b2126831a 100644
--- a/src/mongo/db/service_context_d.h
+++ b/src/mongo/db/service_context_d.h
@@ -28,11 +28,9 @@
 
 #pragma once
 
-#include <boost/optional.hpp>
-#include <vector>
+#include <map>
 
 #include "mongo/db/service_context.h"
-#include "mongo/platform/unordered_set.h"
 
 namespace mongo {
 
@@ -41,7 +39,7 @@ class StorageEngineLockFile;
 
 class ServiceContextMongoD final : public ServiceContext {
 public:
-    typedef std::map<std::string, const StorageEngine::Factory*> FactoryMap;
+    using FactoryMap = std::map<std::string, const StorageEngine::Factory*>;
 
     ServiceContextMongoD();
 
@@ -62,10 +60,6 @@ public:
 
     StorageFactoriesIterator* makeStorageFactoriesIterator() override;
 
-    void setOpObserver(std::unique_ptr<OpObserver> opObserver) override;
-
-    OpObserver* getOpObserver() override;
-
 private:
     std::unique_ptr<OperationContext> _newOpCtx(Client* client, unsigned opId) override;
 
@@ -74,20 +68,18 @@ private:
     // logically owned here, but never deleted by anyone.
     StorageEngine* _storageEngine = nullptr;
 
-    // logically owned here.
-    std::unique_ptr<OpObserver> _opObserver;
-
     // All possible storage engines are registered here through MONGO_INIT.
     FactoryMap _storageFactories;
 };
 
-class StorageFactoriesIteratorMongoD : public StorageFactoriesIterator {
+class StorageFactoriesIteratorMongoD final : public StorageFactoriesIterator {
 public:
     typedef ServiceContextMongoD::FactoryMap::const_iterator FactoryMapIterator;
+
     StorageFactoriesIteratorMongoD(const FactoryMapIterator& begin, const FactoryMapIterator& end);
 
-    virtual bool more() const;
-    virtual const StorageEngine::Factory* next();
+    bool more() const override;
+    const StorageEngine::Factory* next() override;
 
 private:
     FactoryMapIterator _curr;
diff --git a/src/mongo/db/service_context_d_test_fixture.cpp b/src/mongo/db/service_context_d_test_fixture.cpp
index eb35a212569..c47e554b633 100644
--- a/src/mongo/db/service_context_d_test_fixture.cpp
+++ b/src/mongo/db/service_context_d_test_fixture.cpp
@@ -52,7 +52,8 @@ namespace mongo {
 
 void ServiceContextMongoDTest::setUp() {
     Client::initThread(getThreadName());
-    ServiceContext* serviceContext = getServiceContext();
+
+    auto const serviceContext = getServiceContext();
 
     auto logicalClock = stdx::make_unique<LogicalClock>(serviceContext);
     LogicalClock::set(serviceContext, std::move(logicalClock));
@@ -64,7 +65,8 @@ void ServiceContextMongoDTest::setUp() {
         storageGlobalParams.dbpath = tempDir.path();
         storageGlobalParams.engine = "ephemeralForTest";
         storageGlobalParams.engineSetByUser = true;
-        checked_cast<ServiceContextMongoD*>(getGlobalServiceContext())->createLockFile();
+
+        checked_cast<ServiceContextMongoD*>(serviceContext)->createLockFile();
         serviceContext->initializeGlobalStorageEngine();
         serviceContext->setOpObserver(stdx::make_unique<OpObserverNoop>());
     }
diff --git a/src/mongo/db/service_context_noop.cpp b/src/mongo/db/service_context_noop.cpp
index 64e1388b80e..2ad4d1f2c3c 100644
--- a/src/mongo/db/service_context_noop.cpp
+++ b/src/mongo/db/service_context_noop.cpp
@@ -30,7 +30,6 @@
 
 #include "mongo/db/service_context_noop.h"
 
-#include "mongo/db/op_observer.h"
 #include "mongo/db/operation_context_noop.h"
 #include "mongo/stdx/memory.h"
 
@@ -71,10 +70,4 @@ std::unique_ptr<OperationContext> ServiceContextNoop::_newOpCtx(Client* client,
     return stdx::make_unique<OperationContextNoop>(client, opId);
 }
 
-void ServiceContextNoop::setOpObserver(std::unique_ptr<OpObserver> opObserver) {}
-
-OpObserver* ServiceContextNoop::getOpObserver() {
-    return nullptr;
-}
-
 }  // namespace mongo
diff --git a/src/mongo/db/service_context_noop.h b/src/mongo/db/service_context_noop.h
index d6e2d12e525..251cd43310a 100644
--- a/src/mongo/db/service_context_noop.h
+++ b/src/mongo/db/service_context_noop.h
@@ -47,10 +47,6 @@ public:
 
     StorageFactoriesIterator* makeStorageFactoriesIterator() override;
 
-    void setOpObserver(std::unique_ptr<OpObserver> opObserver) override;
-
-    OpObserver* getOpObserver() override;
-
 private:
     std::unique_ptr<OperationContext> _newOpCtx(Client* client, unsigned opId) override;
 };
diff --git a/src/mongo/shell/bench.cpp b/src/mongo/shell/bench.cpp
index 613b1a93fa0..eb5f83b1f68 100644
--- a/src/mongo/shell/bench.cpp
+++ b/src/mongo/shell/bench.cpp
@@ -1,6 +1,4 @@
-/** @file bench.cpp */
-
-/*
+/**
  *    Copyright (C) 2010 10gen Inc.
  *
  *    This program is free software: you can redistribute it and/or  modify
@@ -34,7 +32,6 @@
 
 #include "mongo/shell/bench.h"
 
-#include <iostream>
 #include <pcrecpp.h>
 
 #include "mongo/client/dbclientcursor.h"
@@ -51,13 +48,39 @@
 #include "mongo/util/timer.h"
 #include "mongo/util/version.h"
 
-// ---------------------------------
-// ---- benchmarking system --------
-// ---------------------------------
-
-// TODO:  Maybe extract as library to avoid code duplication?
+namespace mongo {
 namespace {
-inline pcrecpp::RE_Options flags2options(const char* flags) {
+
+const std::map<OpType, std::string> opTypeName{{OpType::NONE, "none"},
+                                               {OpType::NOP, "nop"},
+                                               {OpType::FINDONE, "findOne"},
+                                               {OpType::COMMAND, "command"},
+                                               {OpType::FIND, "find"},
+                                               {OpType::UPDATE, "update"},
+                                               {OpType::INSERT, "insert"},
+                                               {OpType::REMOVE, "remove"},
+                                               {OpType::CREATEINDEX, "createIndex"},
+                                               {OpType::DROPINDEX, "dropIndex"},
+                                               {OpType::LET, "let"},
+                                               {OpType::CPULOAD, "cpuload"}};
+
+class BenchRunWorkerStateGuard {
+    MONGO_DISALLOW_COPYING(BenchRunWorkerStateGuard);
+
+public:
+    explicit BenchRunWorkerStateGuard(BenchRunState* brState) : _brState(brState) {
+        _brState->onWorkerStarted();
+    }
+
+    ~BenchRunWorkerStateGuard() {
+        _brState->onWorkerFinished();
+    }
+
+private:
+    BenchRunState* _brState;
+};
+
+pcrecpp::RE_Options flags2options(const char* flags) {
     pcrecpp::RE_Options options;
     options.set_utf8(true);
     while (flags && *flags) {
@@ -71,33 +94,100 @@ inline pcrecpp::RE_Options flags2options(const char* flags) {
     }
     return options;
 }
+
+void appendAverageMicrosIfAvailable(BSONObjBuilder& buf,
+                                    const std::string& name,
+                                    const BenchRunEventCounter& counter) {
+    if (counter.getNumEvents() > 0)
+        buf.append(name,
+                   static_cast<double>(counter.getTotalTimeMicros()) / counter.getNumEvents());
 }
 
-namespace mongo {
+bool hasSpecial(const BSONObj& obj) {
+    BSONObjIterator i(obj);
+    while (i.more()) {
+        BSONElement e = i.next();
+        if (e.fieldName()[0] == '#')
+            return true;
 
-using std::unique_ptr;
-using std::cout;
-using std::map;
+        if (!e.isABSONObj())
+            continue;
 
-const std::map<OpType, std::string> opTypeName{{OpType::NONE, "none"},
-                                               {OpType::NOP, "nop"},
-                                               {OpType::FINDONE, "findOne"},
-                                               {OpType::COMMAND, "command"},
-                                               {OpType::FIND, "find"},
-                                               {OpType::UPDATE, "update"},
-                                               {OpType::INSERT, "insert"},
-                                               {OpType::REMOVE, "remove"},
-                                               {OpType::CREATEINDEX, "createIndex"},
-                                               {OpType::DROPINDEX, "dropIndex"},
-                                               {OpType::LET, "let"},
-                                               {OpType::CPULOAD, "cpuload"}};
+        if (hasSpecial(e.Obj()))
+            return true;
+    }
+    return false;
+}
+
+BSONObj fixQuery(const BSONObj& obj, BsonTemplateEvaluator& btl) {
+    if (!hasSpecial(obj))
+        return obj;
+
+    BSONObjBuilder b(obj.objsize() + 128);
+    verify(BsonTemplateEvaluator::StatusSuccess == btl.evaluate(obj, b));
+    return b.obj();
+}
+
+/**
+ * Issues the query 'qr' against 'conn' using read commands. Returns the size of the result set
+ * returned by the query.
+ *
+ * If 'qr' has the 'wantMore' flag set to false and the 'limit' option set to 1LL, then the caller
+ * may optionally specify a pointer to an object in 'objOut', which will be filled in with the
+ * single object in the query result set (or the empty object, if the result set is empty).
+ * If 'qr' doesn't have these options set, then nullptr must be passed for 'objOut'.
+ *
+ * On error, throws a AssertionException.
+ */
+int runQueryWithReadCommands(DBClientBase* conn,
+                             std::unique_ptr<QueryRequest> qr,
+                             BSONObj* objOut = nullptr) {
+    std::string dbName = qr->nss().db().toString();
+    BSONObj findCommandResult;
+    bool res = conn->runCommand(dbName, qr->asFindCommand(), findCommandResult);
+    uassert(ErrorCodes::CommandFailed,
+            str::stream() << "find command failed; reply was: " << findCommandResult,
+            res);
+
+    CursorResponse cursorResponse =
+        uassertStatusOK(CursorResponse::parseFromBSON(findCommandResult));
+    int count = cursorResponse.getBatch().size();
+
+    if (objOut) {
+        invariant(qr->getLimit() && *qr->getLimit() == 1 && !qr->wantMore());
+        // Since this is a "single batch" query, we can simply grab the first item in the result set
+        // and return here.
+        *objOut = (count > 0) ? cursorResponse.getBatch()[0] : BSONObj();
+        return count;
+    }
+
+    while (cursorResponse.getCursorId() != 0) {
+        GetMoreRequest getMoreRequest(qr->nss(),
+                                      cursorResponse.getCursorId(),
+                                      qr->getBatchSize(),
+                                      boost::none,   // maxTimeMS
+                                      boost::none,   // term
+                                      boost::none);  // lastKnownCommittedOpTime
+        BSONObj getMoreCommandResult;
+        res = conn->runCommand(dbName, getMoreRequest.toBSON(), getMoreCommandResult);
+        uassert(ErrorCodes::CommandFailed,
+                str::stream() << "getMore command failed; reply was: " << getMoreCommandResult,
+                res);
+        cursorResponse = uassertStatusOK(CursorResponse::parseFromBSON(getMoreCommandResult));
+        count += cursorResponse.getBatch().size();
+    }
+
+    return count;
+}
+
+void doNothing(const BSONObj&) {}
+
+}  // namespace
 
 BenchRunEventCounter::BenchRunEventCounter() {
     reset();
 }
 
-BenchRunEventCounter::~BenchRunEventCounter() {}
-
 void BenchRunEventCounter::reset() {
     _numEvents = 0;
     _totalTimeMicros = 0;
@@ -112,8 +202,6 @@ BenchRunStats::BenchRunStats() {
     reset();
 }
 
-BenchRunStats::~BenchRunStats() {}
-
 void BenchRunStats::reset() {
     error = false;
     errCount = 0;
@@ -490,12 +578,12 @@ void BenchRunConfig::initializeFromBson(const BSONObj& args) {
     }
 }
 
-DBClientBase* BenchRunConfig::createConnection() const {
+std::unique_ptr<DBClientBase> BenchRunConfig::createConnection() const {
     const ConnectionString connectionString = uassertStatusOK(ConnectionString::parse(host));
 
     std::string errorMessage;
-    DBClientBase* connection = connectionString.connect("BenchRun", errorMessage);
-    uassert(16158, errorMessage, connection != NULL);
+    std::unique_ptr<DBClientBase> connection(connectionString.connect("BenchRun", errorMessage));
+    uassert(16158, errorMessage, connection);
 
     return connection;
 }
@@ -572,34 +660,6 @@ void BenchRunState::onWorkerFinished() {
     }
 }
 
-BSONObj benchStart(const BSONObj&, void*);
-BSONObj benchFinish(const BSONObj&, void*);
-
-static bool _hasSpecial(const BSONObj& obj) {
-    BSONObjIterator i(obj);
-    while (i.more()) {
-        BSONElement e = i.next();
-        if (e.fieldName()[0] == '#')
-            return true;
-
-        if (!e.isABSONObj())
-            continue;
-
-        if (_hasSpecial(e.Obj()))
-            return true;
-    }
-    return false;
-}
-
-static BSONObj fixQuery(const BSONObj& obj, BsonTemplateEvaluator& btl) {
-    if (!_hasSpecial(obj))
-        return obj;
-    BSONObjBuilder b(obj.objsize() + 128);
-
-    verify(BsonTemplateEvaluator::StatusSuccess == btl.evaluate(obj, b));
-    return b.obj();
-}
-
 BenchRunWorker::BenchRunWorker(size_t id,
                                const BenchRunConfig* config,
                                BenchRunState* brState,
@@ -620,76 +680,22 @@ bool BenchRunWorker::shouldCollectStats() const {
     return _brState->shouldWorkerCollectStats();
 }
 
-void doNothing(const BSONObj&) {}
-
-/**
- * Issues the query 'qr' against 'conn' using read commands. Returns the size of the result set
- * returned by the query.
- *
- * If 'qr' has the 'wantMore' flag set to false and the 'limit' option set to 1LL, then the caller
- * may optionally specify a pointer to an object in 'objOut', which will be filled in with the
- * single object in the query result set (or the empty object, if the result set is empty).
- * If 'qr' doesn't have these options set, then nullptr must be passed for 'objOut'.
- *
- * On error, throws a AssertionException.
- */
-int runQueryWithReadCommands(DBClientBase* conn,
-                             unique_ptr<QueryRequest> qr,
-                             BSONObj* objOut = nullptr) {
-    std::string dbName = qr->nss().db().toString();
-    BSONObj findCommandResult;
-    bool res = conn->runCommand(dbName, qr->asFindCommand(), findCommandResult);
-    uassert(ErrorCodes::CommandFailed,
-            str::stream() << "find command failed; reply was: " << findCommandResult,
-            res);
-
-    CursorResponse cursorResponse =
-        uassertStatusOK(CursorResponse::parseFromBSON(findCommandResult));
-    int count = cursorResponse.getBatch().size();
-
-    if (objOut) {
-        invariant(qr->getLimit() && *qr->getLimit() == 1 && !qr->wantMore());
-        // Since this is a "single batch" query, we can simply grab the first item in the result set
-        // and return here.
-        *objOut = (count > 0) ? cursorResponse.getBatch()[0] : BSONObj();
-        return count;
-    }
-
-    while (cursorResponse.getCursorId() != 0) {
-        GetMoreRequest getMoreRequest(qr->nss(),
-                                      cursorResponse.getCursorId(),
-                                      qr->getBatchSize(),
-                                      boost::none,   // maxTimeMS
-                                      boost::none,   // term
-                                      boost::none);  // lastKnownCommittedOpTime
-        BSONObj getMoreCommandResult;
-        res = conn->runCommand(dbName, getMoreRequest.toBSON(), getMoreCommandResult);
-        uassert(ErrorCodes::CommandFailed,
-                str::stream() << "getMore command failed; reply was: " << getMoreCommandResult,
-                res);
-        cursorResponse = uassertStatusOK(CursorResponse::parseFromBSON(getMoreCommandResult));
-        count += cursorResponse.getBatch().size();
-    }
-
-    return count;
-}
-
 void BenchRunWorker::generateLoadOnConnection(DBClientBase* conn) {
     verify(conn);
     long long count = 0;
-    mongo::Timer timer;
+    Timer timer;
 
     BsonTemplateEvaluator bsonTemplateEvaluator(_randomSeed);
     invariant(bsonTemplateEvaluator.setId(_id) == BsonTemplateEvaluator::StatusSuccess);
 
     if (_config->username != "") {
         std::string errmsg;
-        if (!conn->auth("admin", _config->username, _config->password, errmsg)) {
-            uasserted(15931, "Authenticating to connection for _benchThread failed: " + errmsg);
-        }
+        uassert(15931,
+                str::stream() << "Authenticating to connection for _benchThread failed: " << errmsg,
+                conn->auth("admin", _config->username, _config->password, errmsg));
     }
 
-    unique_ptr<Scope> scope{getGlobalScriptEngine()->newScopeForCurrentThread()};
+    std::unique_ptr<Scope> scope{getGlobalScriptEngine()->newScopeForCurrentThread()};
     verify(scope.get());
 
     while (!shouldStop()) {
@@ -713,6 +719,7 @@ void BenchRunWorker::generateLoadOnConnection(DBClientBase* conn) {
                 scope->init(&scopeObj);
                 invariant(scopeFunc);
             }
+
             try {
                 switch (op.op) {
                     case OpType::NOP:
@@ -868,15 +875,14 @@ void BenchRunWorker::generateLoadOnConnection(DBClientBase* conn) {
                                     op.options | DBClientCursor::QueryOptionLocal_forceOpQuery);
                             } else {
                                 BenchRunEventTrace _bret(&stats.queryCounter);
-                                unique_ptr<DBClientCursor> cursor;
-                                cursor = conn->query(
+                                std::unique_ptr<DBClientCursor> cursor(conn->query(
                                     op.ns,
                                     fixedQuery,
                                     op.limit,
                                     op.skip,
                                     &op.projection,
                                     op.options | DBClientCursor::QueryOptionLocal_forceOpQuery,
-                                    op.batchSize);
+                                    op.batchSize));
                                 count = cursor->itcount();
                             }
                         }
@@ -1152,38 +1158,22 @@ void BenchRunWorker::generateLoadOnConnection(DBClientBase* conn) {
     conn->getLastError();
 }
 
-namespace {
-class BenchRunWorkerStateGuard {
-    MONGO_DISALLOW_COPYING(BenchRunWorkerStateGuard);
-
-public:
-    explicit BenchRunWorkerStateGuard(BenchRunState* brState) : _brState(brState) {
-        _brState->onWorkerStarted();
-    }
-
-    ~BenchRunWorkerStateGuard() {
-        _brState->onWorkerFinished();
-    }
-
-private:
-    BenchRunState* _brState;
-};
-}  // namespace
-
 void BenchRunWorker::run() {
     try {
-        std::unique_ptr<DBClientBase> conn(_config->createConnection());
+        auto conn(_config->createConnection());
+
         if (!_config->username.empty()) {
             std::string errmsg;
             if (!conn->auth("admin", _config->username, _config->password, errmsg)) {
                 uasserted(15932, "Authenticating to connection for benchThread failed: " + errmsg);
             }
         }
+
         BenchRunWorkerStateGuard _workerStateGuard(_brState);
         generateLoadOnConnection(conn.get());
-    } catch (DBException& e) {
+    } catch (const DBException& e) {
         error() << "DBException not handled in benchRun thread" << causedBy(e);
-    } catch (std::exception& e) {
+    } catch (const std::exception& e) {
         error() << "std::exception not handled in benchRun thread" << causedBy(e);
     } catch (...) {
         error() << "Unknown exception not handled in benchRun thread.";
@@ -1229,7 +1219,7 @@ void BenchRunner::start() {
 
         // initial stats
         _brState.tellWorkersToCollectStats();
-        _brTimer = new mongo::Timer();
+        _brTimer = new Timer();
     }
 }
 
@@ -1276,14 +1266,6 @@ void BenchRunner::populateStats(BenchRunStats* stats) {
         stats->updateFrom(_workers[i]->stats());
 }
 
-static void appendAverageMicrosIfAvailable(BSONObjBuilder& buf,
-                                           const std::string& name,
-                                           const BenchRunEventCounter& counter) {
-    if (counter.getNumEvents() > 0)
-        buf.append(name,
-                   static_cast<double>(counter.getTotalTimeMicros()) / counter.getNumEvents());
-}
-
 BSONObj BenchRunner::finish(BenchRunner* runner) {
     runner->stop();
 
@@ -1328,7 +1310,7 @@ BSONObj BenchRunner::finish(BenchRunner* runner) {
 }
 
 stdx::mutex BenchRunner::_staticMutex;
-map<OID, BenchRunner*> BenchRunner::_activeRuns;
+std::map<OID, BenchRunner*> BenchRunner::_activeRuns;
 
 /**
  * benchRun( { ops : [] , host : XXX , db : XXXX , parallel : 5 , seconds : 5 }
diff --git a/src/mongo/shell/bench.h b/src/mongo/shell/bench.h
index 6d3b9721988..41a23512d28 100644
--- a/src/mongo/shell/bench.h
+++ b/src/mongo/shell/bench.h
@@ -119,7 +119,7 @@ public:
     void initializeFromBson(const BSONObj& args);
 
     // Create a new connection to the mongo instance specified by this configuration.
-    DBClientBase* createConnection() const;
+    std::unique_ptr<DBClientBase> createConnection() const;
 
     /**
      * Connection std::string describing the host to which to connect.
@@ -197,7 +197,6 @@ class BenchRunEventCounter {
 public:
     /// Constructs a zeroed out counter.
     BenchRunEventCounter();
-    ~BenchRunEventCounter();
 
     /**
      * Zero out the counter.
@@ -297,7 +296,6 @@ class BenchRunStats {
 
 public:
     BenchRunStats();
-    ~BenchRunStats();
 
     void reset();
 
@@ -443,11 +441,13 @@ private:
     /// Predicate, used to decide whether or not it's time to collect statistics
     bool shouldCollectStats() const;
 
-    size_t _id;
+    const size_t _id;
+
     const BenchRunConfig* _config;
     BenchRunState* _brState;
     BenchRunStats _stats;
-    /// Dummy stats to use before observation period.
+
+    // Dummy stats to use before observation period.
     BenchRunStats _statsBlackHole;
     int64_t _randomSeed;
 };