SERVER-32367 Clean up the AutoGet* suite of classes

* Get rid of AutoGetCollectionOrViewForReadCommand
* Get rid of constructors accepting Lock::DBLock
* Always check for shard version, unless the namespace represents a view

1 files changed, 78 insertions, 136 deletions

diff --git a/src/mongo/db/db_raii.cpp b/src/mongo/db/db_raii.cpp
index fb61a0a8adf..52efb4948f8 100644
--- a/src/mongo/db/db_raii.cpp
+++ b/src/mongo/db/db_raii.cpp
@@ -36,6 +36,11 @@
 #include "mongo/db/s/collection_sharding_state.h"
 
 namespace mongo {
+namespace {
+
+const boost::optional<int> kDoNotChangeProfilingLevel = boost::none;
+
+}  // namespace
 
 AutoStatsTracker::AutoStatsTracker(OperationContext* opCtx,
                                    const NamespaceString& nss,
@@ -69,31 +74,11 @@ AutoStatsTracker::~AutoStatsTracker() {
 }
 
 AutoGetCollectionForRead::AutoGetCollectionForRead(OperationContext* opCtx,
-                                                   const NamespaceString& nss,
-                                                   Date_t deadline)
-    : AutoGetCollectionForRead(opCtx,
-                               nss,
-                               AutoGetCollection::ViewMode::kViewsForbidden,
-                               Lock::DBLock(opCtx, nss.db(), getLockModeForQuery(opCtx), deadline),
-                               deadline) {}
-
-AutoGetCollectionForRead::AutoGetCollectionForRead(OperationContext* opCtx,
-                                                   const StringData dbName,
-                                                   const UUID& uuid,
-                                                   Date_t deadline)
-    : AutoGetCollectionForRead(opCtx,
-                               NamespaceStringOrUUID({dbName.toString(), uuid}),
-                               AutoGetCollection::ViewMode::kViewsForbidden,
-                               Lock::DBLock(opCtx, dbName, getLockModeForQuery(opCtx), deadline),
-                               deadline) {}
-
-AutoGetCollectionForRead::AutoGetCollectionForRead(OperationContext* opCtx,
                                                    const NamespaceStringOrUUID& nsOrUUID,
                                                    AutoGetCollection::ViewMode viewMode,
-                                                   Lock::DBLock lock,
                                                    Date_t deadline) {
     const auto collectionLockMode = getLockModeForQuery(opCtx);
-    _autoColl.emplace(opCtx, nsOrUUID, std::move(lock), collectionLockMode, viewMode, deadline);
+    _autoColl.emplace(opCtx, nsOrUUID, collectionLockMode, viewMode, deadline);
 
     while (true) {
         auto coll = _autoColl->getCollection();
@@ -131,124 +116,54 @@ AutoGetCollectionForRead::AutoGetCollectionForRead(OperationContext* opCtx,
 
 AutoGetCollectionForReadCommand::AutoGetCollectionForReadCommand(
     OperationContext* opCtx,
-    const NamespaceString& nss,
-    AutoGetCollection::ViewMode viewMode,
-    Lock::DBLock lock,
-    Date_t deadline) {
-    _autoCollForRead.emplace(opCtx, nss, viewMode, std::move(lock), deadline);
-    const int doNotChangeProfilingLevel = 0;
-    _statsTracker.emplace(opCtx,
-                          nss,
-                          Top::LockType::ReadLocked,
-                          _autoCollForRead->getDb() ? _autoCollForRead->getDb()->getProfilingLevel()
-                                                    : doNotChangeProfilingLevel,
-                          deadline);
-
-    // We have both the DB and collection locked, which is the prerequisite to do a stable shard
-    // version check, but we'd like to do the check after we have a satisfactory snapshot.
-    auto css = CollectionShardingState::get(opCtx, nss);
-    css->checkShardVersionOrThrow(opCtx);
-}
-
-AutoGetCollectionForReadCommand::AutoGetCollectionForReadCommand(
-    OperationContext* opCtx,
-    const NamespaceString& nss,
+    const NamespaceStringOrUUID& nsOrUUID,
     AutoGetCollection::ViewMode viewMode,
     Date_t deadline)
-    : AutoGetCollectionForReadCommand(
-          opCtx,
-          nss,
-          viewMode,
-          Lock::DBLock(opCtx, nss.db(), getLockModeForQuery(opCtx), deadline)) {}
-
-AutoGetCollectionOrViewForReadCommand::AutoGetCollectionOrViewForReadCommand(
-    OperationContext* opCtx, const NamespaceString& nss, Date_t deadline)
-    : AutoGetCollectionForReadCommand(
-          opCtx, nss, AutoGetCollection::ViewMode::kViewsPermitted, deadline),
-      _view(_autoCollForRead->getDb() && !getCollection()
-                ? _autoCollForRead->getDb()->getViewCatalog()->lookup(opCtx, nss.ns())
-                : nullptr) {}
-
-AutoGetCollectionOrViewForReadCommand::AutoGetCollectionOrViewForReadCommand(
-    OperationContext* opCtx, const NamespaceString& nss, Lock::DBLock lock, Date_t deadline)
-    : AutoGetCollectionForReadCommand(
-          opCtx, nss, AutoGetCollection::ViewMode::kViewsPermitted, std::move(lock), deadline),
-      _view(_autoCollForRead->getDb() && !getCollection()
-                ? _autoCollForRead->getDb()->getViewCatalog()->lookup(opCtx, nss.ns())
-                : nullptr) {}
-
-AutoGetCollectionForReadCommand::AutoGetCollectionForReadCommand(OperationContext* opCtx,
-                                                                 const StringData dbName,
-                                                                 const UUID& uuid,
-                                                                 Date_t deadline) {
-    _autoCollForRead.emplace(opCtx, dbName, uuid, deadline);
-    if (_autoCollForRead->getCollection()) {
-        _statsTracker.emplace(opCtx,
-                              _autoCollForRead->getCollection()->ns(),
-                              Top::LockType::ReadLocked,
-                              _autoCollForRead->getDb()->getProfilingLevel(),
-                              deadline);
-
+    : _autoCollForRead(opCtx, nsOrUUID, viewMode, deadline),
+      _statsTracker(opCtx,
+                    _autoCollForRead.getNss(),
+                    Top::LockType::ReadLocked,
+                    _autoCollForRead.getDb() ? _autoCollForRead.getDb()->getProfilingLevel()
+                                             : kDoNotChangeProfilingLevel,
+                    deadline) {
+    if (!_autoCollForRead.getView()) {
         // We have both the DB and collection locked, which is the prerequisite to do a stable shard
         // version check, but we'd like to do the check after we have a satisfactory snapshot.
-        auto css = CollectionShardingState::get(opCtx, _autoCollForRead->getCollection()->ns());
+        auto css = CollectionShardingState::get(opCtx, _autoCollForRead.getNss());
         css->checkShardVersionOrThrow(opCtx);
     }
 }
 
-void AutoGetCollectionOrViewForReadCommand::releaseLocksForView() noexcept {
-    invariant(_view);
-    _view = nullptr;
-    _autoCollForRead = boost::none;
-}
-
-OldClientContext::OldClientContext(OperationContext* opCtx,
-                                   const std::string& ns,
-                                   Database* db,
-                                   bool justCreated)
-    : _justCreated(justCreated), _doVersion(true), _ns(ns), _db(db), _opCtx(opCtx) {
-    _finishInit();
-}
-
-OldClientContext::OldClientContext(OperationContext* opCtx,
-                                   const std::string& ns,
-                                   bool doVersion)
-    : _justCreated(false),  // set for real in finishInit
-      _doVersion(doVersion),
-      _ns(ns),
-      _db(NULL),
-      _opCtx(opCtx) {
-    _finishInit();
-}
+OldClientContext::OldClientContext(OperationContext* opCtx, const std::string& ns, bool doVersion)
+    : OldClientContext(opCtx, ns, doVersion, dbHolder().get(opCtx, ns), false) {}
 
-void OldClientContext::_finishInit() {
-    _db = dbHolder().get(_opCtx, _ns);
-    if (_db) {
-        _justCreated = false;
-    } else {
-        invariant(_opCtx->lockState()->isDbLockedForMode(nsToDatabaseSubstring(_ns), MODE_X));
-        _db = dbHolder().openDb(_opCtx, _ns, &_justCreated);
+OldClientContext::OldClientContext(
+    OperationContext* opCtx, const std::string& ns, bool doVersion, Database* db, bool justCreated)
+    : _opCtx(opCtx), _db(db), _justCreated(justCreated) {
+    if (!_db) {
+        const auto dbName = nsToDatabaseSubstring(ns);
+        invariant(_opCtx->lockState()->isDbLockedForMode(dbName, MODE_X));
+        _db = dbHolder().openDb(_opCtx, dbName, &_justCreated);
         invariant(_db);
     }
 
-    if (_doVersion) {
-        _checkNotStale();
+    auto const currentOp = CurOp::get(_opCtx);
+
+    if (doVersion) {
+        switch (currentOp->getNetworkOp()) {
+            case dbGetMore:  // getMore is special and should be handled elsewhere
+            case dbUpdate:   // update & delete check shard version as part of the write executor
+            case dbDelete:   // path, so no need to check them here as well
+                break;
+            default:
+                auto css = CollectionShardingState::get(_opCtx, ns);
+                css->checkShardVersionOrThrow(_opCtx);
+                break;
+        }
     }
 
     stdx::lock_guard<Client> lk(*_opCtx->getClient());
-    CurOp::get(_opCtx)->enter_inlock(_ns.c_str(), _db->getProfilingLevel());
-}
-
-void OldClientContext::_checkNotStale() const {
-    switch (CurOp::get(_opCtx)->getNetworkOp()) {
-        case dbGetMore:  // getMore is special and should be handled elsewhere.
-        case dbUpdate:   // update & delete check shard version in instance.cpp, so don't check
-        case dbDelete:   // here as well.
-            break;
-        default:
-            auto css = CollectionShardingState::get(_opCtx, _ns);
-            css->checkShardVersionOrThrow(_opCtx);
-    }
+    currentOp->enter_inlock(ns.c_str(), _db->getProfilingLevel());
 }
 
 OldClientContext::~OldClientContext() {
@@ -268,19 +183,46 @@ OldClientContext::~OldClientContext() {
 }
 
 
-OldClientWriteContext::OldClientWriteContext(OperationContext* opCtx, const std::string& ns)
-    : _opCtx(opCtx),
-      _nss(ns),
-      _autodb(opCtx, _nss.db(), MODE_IX),
-      _collk(opCtx->lockState(), ns, MODE_IX),
-      _c(opCtx, ns, _autodb.getDb(), _autodb.justCreated()) {
-    _collection = _c.db()->getCollection(opCtx, ns);
-    if (!_collection && !_autodb.justCreated()) {
-        // relock database in MODE_X to allow collection creation
-        _collk.relockAsDatabaseExclusive(_autodb.lock());
-        Database* db = dbHolder().get(_opCtx, ns);
-        invariant(db == _c.db());
+OldClientWriteContext::OldClientWriteContext(OperationContext* opCtx, StringData ns)
+    : _opCtx(opCtx), _nss(ns) {
+    // Lock the database and collection
+    _autoCreateDb.emplace(opCtx, _nss.db(), MODE_IX);
+    _collLock.emplace(opCtx->lockState(), _nss.ns(), MODE_IX);
+
+    // TODO (Kal): None of the places which use OldClientWriteContext seem to require versioning, so
+    // we should consider defaulting this to false
+    const bool doShardVersionCheck = true;
+    _clientContext.emplace(opCtx,
+                           _nss.ns(),
+                           doShardVersionCheck,
+                           _autoCreateDb->getDb(),
+                           _autoCreateDb->justCreated());
+    invariant(_autoCreateDb->getDb() == _clientContext->db());
+
+    // If the collection exists, there is no need to lock into stronger mode
+    if (getCollection())
+        return;
+
+    // If the database was just created, it is already locked in MODE_X so we can skip the relocking
+    // code below
+    if (_autoCreateDb->justCreated()) {
+        dassert(opCtx->lockState()->isDbLockedForMode(_nss.db(), MODE_X));
+        return;
     }
+
+    // If the collection doesn't exists, put the context in a state where the database is locked in
+    // MODE_X so that the collection can be created
+    _clientContext.reset();
+    _collLock.reset();
+    _autoCreateDb.reset();
+    _autoCreateDb.emplace(opCtx, _nss.db(), MODE_X);
+
+    _clientContext.emplace(opCtx,
+                           _nss.ns(),
+                           doShardVersionCheck,
+                           _autoCreateDb->getDb(),
+                           _autoCreateDb->justCreated());
+    invariant(_autoCreateDb->getDb() == _clientContext->db());
 }
 
 LockMode getLockModeForQuery(OperationContext* opCtx) {