SERVER-42508 Convert SessionsCollection to throw instead of return status

Change the return value of checkSessionsCollectionExists from Status to void and throws exception on error

11 files changed, 93 insertions, 109 deletions

diff --git a/src/mongo/db/logical_session_cache_impl.cpp b/src/mongo/db/logical_session_cache_impl.cpp
index 83671cdd039..a623b15e856 100644
--- a/src/mongo/db/logical_session_cache_impl.cpp
+++ b/src/mongo/db/logical_session_cache_impl.cpp
@@ -166,22 +166,20 @@ Status LogicalSessionCacheImpl::_reap(Client* client) {
 
     try {
         ON_BLOCK_EXIT([&opCtx] { clearShardingOperationFailedStatus(opCtx); });
-
-        auto existsStatus = _sessionsColl->checkSessionsCollectionExists(opCtx);
-        if (!existsStatus.isOK()) {
+        try {
+            _sessionsColl->checkSessionsCollectionExists(opCtx);
+        } catch (const DBException& ex) {
             StringData notSetUpWarning =
                 "Sessions collection is not set up; "
                 "waiting until next sessions reap interval";
-            if (existsStatus.code() != ErrorCodes::NamespaceNotFound ||
-                existsStatus.code() != ErrorCodes::NamespaceNotSharded) {
-                log() << notSetUpWarning << ": " << existsStatus.reason();
+            if (ex.code() != ErrorCodes::NamespaceNotFound ||
+                ex.code() != ErrorCodes::NamespaceNotSharded) {
+                log() << notSetUpWarning << ": " << ex.reason();
             } else {
                 log() << notSetUpWarning;
             }
-
             return Status::OK();
         }
-
         numReaped = _reapSessionsOlderThanFn(opCtx,
                                              *_sessionsColl,
                                              _service->now() -
@@ -244,7 +242,6 @@ void LogicalSessionCacheImpl::_refresh(Client* client) {
 
     try {
         _sessionsColl->setupSessionsCollection(opCtx);
-
     } catch (DBException& ex) {
         log() << "Failed to refresh session cache: " << ex.reason()
               << ", will try again at the next refresh interval";
diff --git a/src/mongo/db/sessions_collection.h b/src/mongo/db/sessions_collection.h
index a632e5094d5..96cbcb80694 100644
--- a/src/mongo/db/sessions_collection.h
+++ b/src/mongo/db/sessions_collection.h
@@ -58,7 +58,7 @@ public:
     /**
      * Checks if the sessions collection exists and has the proper indexes.
      */
-    virtual Status checkSessionsCollectionExists(OperationContext* opCtx) = 0;
+    virtual void checkSessionsCollectionExists(OperationContext* opCtx) = 0;
 
     /**
      * Updates the last-use times on the given sessions to be greater than
diff --git a/src/mongo/db/sessions_collection_config_server.cpp b/src/mongo/db/sessions_collection_config_server.cpp
index 30431007ba7..6ed8792c8d8 100644
--- a/src/mongo/db/sessions_collection_config_server.cpp
+++ b/src/mongo/db/sessions_collection_config_server.cpp
@@ -104,8 +104,8 @@ void SessionsCollectionConfigServer::setupSessionsCollection(OperationContext* o
     _generateIndexesIfNeeded(opCtx);
 }
 
-Status SessionsCollectionConfigServer::checkSessionsCollectionExists(OperationContext* opCtx) {
-    return _checkCacheForSessionsCollection(opCtx);
+void SessionsCollectionConfigServer::checkSessionsCollectionExists(OperationContext* opCtx) {
+    uassertStatusOK(_checkCacheForSessionsCollection(opCtx));
 }
 
 }  // namespace mongo
diff --git a/src/mongo/db/sessions_collection_config_server.h b/src/mongo/db/sessions_collection_config_server.h
index d7ee8d3fb63..619d8811f3a 100644
--- a/src/mongo/db/sessions_collection_config_server.h
+++ b/src/mongo/db/sessions_collection_config_server.h
@@ -58,7 +58,7 @@ public:
     /**
      * Checks if the sessions collection exists.
      */
-    Status checkSessionsCollectionExists(OperationContext* opCtx) override;
+    void checkSessionsCollectionExists(OperationContext* opCtx) override;
 
 private:
     void _shardCollectionIfNeeded(OperationContext* opCtx);
diff --git a/src/mongo/db/sessions_collection_mock.h b/src/mongo/db/sessions_collection_mock.h
index e4352249d48..ae118648650 100644
--- a/src/mongo/db/sessions_collection_mock.h
+++ b/src/mongo/db/sessions_collection_mock.h
@@ -108,9 +108,7 @@ public:
 
     void setupSessionsCollection(OperationContext* opCtx) override {}
 
-    Status checkSessionsCollectionExists(OperationContext* opCtx) override {
-        return Status::OK();
-    }
+    void checkSessionsCollectionExists(OperationContext* opCtx) override {}
 
     Status refreshSessions(OperationContext* opCtx,
                            const LogicalSessionRecordSet& sessions) override {
diff --git a/src/mongo/db/sessions_collection_rs.cpp b/src/mongo/db/sessions_collection_rs.cpp
index 281f80c4c1e..c67de981876 100644
--- a/src/mongo/db/sessions_collection_rs.cpp
+++ b/src/mongo/db/sessions_collection_rs.cpp
@@ -99,43 +99,36 @@ auto SessionsCollectionRS::_dispatch(const NamespaceString& ns,
         return std::forward<LocalCallback>(localCallback)();
     }
 
+    // There is a window here where we may transition from Primary to Secondary after we release
+    // the locks we take in _isStandaloneOrPrimary(). In this case, the callback we run below
+    // may throw a NotMaster error, or a stale read. However, this is preferable to running the
+    // callback while we hold locks, since that can lead to a deadlock.
+
+    auto conn = _makePrimaryConnection(opCtx);
+    DBClientBase* client = conn->get();
+    auto guard = makeGuard([&] { conn->done(); });
     try {
-        // There is a window here where we may transition from Primary to Secondary after we release
-        // the locks we take in _isStandaloneOrPrimary(). In this case, the callback we run below
-        // may throw a NotMaster error, or a stale read. However, this is preferable to running the
-        // callback while we hold locks, since that can lead to a deadlock.
-
-        auto conn = _makePrimaryConnection(opCtx);
-        DBClientBase* client = conn->get();
-
-        auto sosw = std::forward<RemoteCallback>(remoteCallback)(client);
-        if (!sosw.isOK()) {
-            conn->kill();
-            return sosw;
-        }
-
-        conn->done();
-        return sosw;
+        return std::forward<RemoteCallback>(remoteCallback)(client);
     } catch (...) {
-        return exceptionToStatus();
+        guard.dismiss();
+        conn->kill();
+        throw;
     }
 }
 
 void SessionsCollectionRS::setupSessionsCollection(OperationContext* opCtx) {
-    uassertStatusOKWithContext(
-        _dispatch(
-            NamespaceString::kLogicalSessionsNamespace,
-            opCtx,
-            [&] {
-                auto existsStatus = checkSessionsCollectionExists(opCtx);
-                if (existsStatus.isOK()) {
-                    return Status::OK();
-                }
+    _dispatch(
+        NamespaceString::kLogicalSessionsNamespace,
+        opCtx,
+        [&] {
+            try {
+                checkSessionsCollectionExists(opCtx);
+            } catch (const DBException& ex) {
 
                 DBDirectClient client(opCtx);
                 BSONObj cmd;
 
-                if (existsStatus.code() == ErrorCodes::IndexOptionsConflict) {
+                if (ex.code() == ErrorCodes::IndexOptionsConflict) {
                     cmd = generateCollModCmd();
                 } else {
                     // Creating the TTL index will auto-generate the collection.
@@ -145,41 +138,37 @@ void SessionsCollectionRS::setupSessionsCollection(OperationContext* opCtx) {
                 BSONObj info;
                 if (!client.runCommand(
                         NamespaceString::kLogicalSessionsNamespace.db().toString(), cmd, info)) {
-                    return getStatusFromCommandResult(info);
+                    uassertStatusOK(getStatusFromCommandResult(info));
                 }
-
-                return Status::OK();
-            },
-            [&](DBClientBase*) { return checkSessionsCollectionExists(opCtx); }),
-        str::stream() << "Failed to create " << NamespaceString::kLogicalSessionsNamespace);
+            }
+        },
+        [&](DBClientBase*) { checkSessionsCollectionExists(opCtx); });
 }
 
-Status SessionsCollectionRS::checkSessionsCollectionExists(OperationContext* opCtx) {
+void SessionsCollectionRS::checkSessionsCollectionExists(OperationContext* opCtx) {
     DBDirectClient client(opCtx);
 
     auto indexes = client.getIndexSpecs(NamespaceString::kLogicalSessionsNamespace);
 
-    if (indexes.size() == 0u) {
-        return Status{ErrorCodes::NamespaceNotFound, "config.system.sessions does not exist"};
-    }
+    uassert(ErrorCodes::NamespaceNotFound,
+            str::stream() << NamespaceString::kLogicalSessionsNamespace << " does not exist",
+            indexes.size() != 0u);
 
     auto index = std::find_if(indexes.begin(), indexes.end(), [](const BSONObj& index) {
         return index.getField("name").String() == kSessionsTTLIndex;
     });
 
-    if (index == indexes.end()) {
-        return Status{ErrorCodes::IndexNotFound,
-                      "config.system.sessions does not have the required TTL index"};
-    }
-
-    if (!index->hasField("expireAfterSeconds") ||
-        index->getField("expireAfterSeconds").Int() != (localLogicalSessionTimeoutMinutes * 60)) {
-        return Status{
-            ErrorCodes::IndexOptionsConflict,
-            "config.system.sessions currently has the incorrect timeout for the TTL index"};
-    }
-
-    return Status::OK();
+    uassert(ErrorCodes::IndexNotFound,
+            str::stream() << NamespaceString::kLogicalSessionsNamespace
+                          << " does not have the required TTL index",
+            index != indexes.end());
+
+    uassert(ErrorCodes::IndexOptionsConflict,
+            str::stream() << NamespaceString::kLogicalSessionsNamespace
+                          << " currently has the incorrect timeout for the TTL index",
+            index->hasField("expireAfterSeconds") &&
+                index->getField("expireAfterSeconds").Int() ==
+                    (localLogicalSessionTimeoutMinutes * 60));
 }
 
 Status SessionsCollectionRS::refreshSessions(OperationContext* opCtx,
diff --git a/src/mongo/db/sessions_collection_rs.h b/src/mongo/db/sessions_collection_rs.h
index 93971948e2e..f87f48c217d 100644
--- a/src/mongo/db/sessions_collection_rs.h
+++ b/src/mongo/db/sessions_collection_rs.h
@@ -57,7 +57,7 @@ public:
     /**
      * Checks if the sessions collection exists and has the proper indexes.
      */
-    Status checkSessionsCollectionExists(OperationContext* opCtx) override;
+    void checkSessionsCollectionExists(OperationContext* opCtx) override;
 
     /**
      * Updates the last-use times on the given sessions to be greater than
@@ -97,7 +97,8 @@ private:
         static_assert(std::is_same_v<LocalReturnType, RemoteReturnType>,
                       "LocalCallback and RemoteCallback must have the same return type");
 
-        using Type = RemoteReturnType;
+        using Type =
+            std::conditional_t<std::is_void<LocalReturnType>::value, void, LocalReturnType>;
     };
     template <typename LocalCallback, typename RemoteCallback>
     using CommonResultT = typename CommonResult<LocalCallback, RemoteCallback>::Type;
diff --git a/src/mongo/db/sessions_collection_sharded.cpp b/src/mongo/db/sessions_collection_sharded.cpp
index fad42e13abb..0e91b8e8f71 100644
--- a/src/mongo/db/sessions_collection_sharded.cpp
+++ b/src/mongo/db/sessions_collection_sharded.cpp
@@ -136,11 +136,11 @@ std::vector<LogicalSessionRecord> SessionsCollectionSharded::_groupSessionRecord
 }
 
 void SessionsCollectionSharded::setupSessionsCollection(OperationContext* opCtx) {
-    uassertStatusOK(checkSessionsCollectionExists(opCtx));
+    checkSessionsCollectionExists(opCtx);
 }
 
-Status SessionsCollectionSharded::checkSessionsCollectionExists(OperationContext* opCtx) {
-    return _checkCacheForSessionsCollection(opCtx);
+void SessionsCollectionSharded::checkSessionsCollectionExists(OperationContext* opCtx) {
+    uassertStatusOK(_checkCacheForSessionsCollection(opCtx));
 }
 
 Status SessionsCollectionSharded::refreshSessions(OperationContext* opCtx,
diff --git a/src/mongo/db/sessions_collection_sharded.h b/src/mongo/db/sessions_collection_sharded.h
index bef89cc368e..55cd728c891 100644
--- a/src/mongo/db/sessions_collection_sharded.h
+++ b/src/mongo/db/sessions_collection_sharded.h
@@ -54,7 +54,7 @@ public:
      * Checks if the sessions collection exists. Does not check if the index exists in the sharded
      * version of this function.
      */
-    virtual Status checkSessionsCollectionExists(OperationContext* opCtx) override;
+    virtual void checkSessionsCollectionExists(OperationContext* opCtx) override;
 
     /**
      * Updates the last-use times on the given sessions to be greater than
diff --git a/src/mongo/db/sessions_collection_standalone.cpp b/src/mongo/db/sessions_collection_standalone.cpp
index c994d7694ad..9d1bb415641 100644
--- a/src/mongo/db/sessions_collection_standalone.cpp
+++ b/src/mongo/db/sessions_collection_standalone.cpp
@@ -46,54 +46,53 @@ BSONObj lsidQuery(const LogicalSessionId& lsid) {
 }  // namespace
 
 void SessionsCollectionStandalone::setupSessionsCollection(OperationContext* opCtx) {
-    auto existsStatus = checkSessionsCollectionExists(opCtx);
-    if (existsStatus.isOK()) {
-        return;
-    }
-
-    DBDirectClient client(opCtx);
-    BSONObj cmd;
-
-    if (existsStatus.code() == ErrorCodes::IndexOptionsConflict) {
-        cmd = generateCollModCmd();
-    } else {
-        cmd = generateCreateIndexesCmd();
-    }
-
-    BSONObj info;
-    if (!client.runCommand(NamespaceString::kLogicalSessionsNamespace.db().toString(), cmd, info)) {
-        uassertStatusOKWithContext(getStatusFromCommandResult(info),
-                                   str::stream() << "Failed to create "
-                                                 << NamespaceString::kLogicalSessionsNamespace);
+    try {
+        checkSessionsCollectionExists(opCtx);
+    } catch (DBException& ex) {
+
+        DBDirectClient client(opCtx);
+        BSONObj cmd;
+
+        if (ex.code() == ErrorCodes::IndexOptionsConflict) {
+            cmd = generateCollModCmd();
+        } else {
+            cmd = generateCreateIndexesCmd();
+        }
+
+        BSONObj info;
+        if (!client.runCommand(
+                NamespaceString::kLogicalSessionsNamespace.db().toString(), cmd, info)) {
+            uassertStatusOKWithContext(getStatusFromCommandResult(info),
+                                       str::stream() << "Failed to create "
+                                                     << NamespaceString::kLogicalSessionsNamespace);
+        }
     }
 }
 
-Status SessionsCollectionStandalone::checkSessionsCollectionExists(OperationContext* opCtx) {
+void SessionsCollectionStandalone::checkSessionsCollectionExists(OperationContext* opCtx) {
     DBDirectClient client(opCtx);
 
     auto indexes = client.getIndexSpecs(NamespaceString::kLogicalSessionsNamespace);
 
-    if (indexes.size() == 0u) {
-        return Status{ErrorCodes::NamespaceNotFound, "config.system.sessions does not exist"};
-    }
+    uassert(ErrorCodes::NamespaceNotFound,
+            str::stream() << NamespaceString::kLogicalSessionsNamespace << " does not exist",
+            indexes.size() != 0u);
 
     auto index = std::find_if(indexes.begin(), indexes.end(), [](const BSONObj& index) {
         return index.getField("name").String() == kSessionsTTLIndex;
     });
 
-    if (index == indexes.end()) {
-        return Status{ErrorCodes::IndexNotFound,
-                      "config.system.sessions does not have the required TTL index"};
-    };
-
-    if (!index->hasField("expireAfterSeconds") ||
-        index->getField("expireAfterSeconds").Int() != (localLogicalSessionTimeoutMinutes * 60)) {
-        return Status{
-            ErrorCodes::IndexOptionsConflict,
-            "config.system.sessions currently has the incorrect timeout for the TTL index"};
-    }
-
-    return Status::OK();
+    uassert(ErrorCodes::IndexNotFound,
+            str::stream() << NamespaceString::kLogicalSessionsNamespace
+                          << " does not have the required TTL index",
+            index != indexes.end());
+
+    uassert(ErrorCodes::IndexOptionsConflict,
+            str::stream() << NamespaceString::kLogicalSessionsNamespace
+                          << " currently has the incorrect timeout for the TTL index",
+            index->hasField("expireAfterSeconds") &&
+                index->getField("expireAfterSeconds").Int() ==
+                    (localLogicalSessionTimeoutMinutes * 60));
 }
 
 Status SessionsCollectionStandalone::refreshSessions(OperationContext* opCtx,
diff --git a/src/mongo/db/sessions_collection_standalone.h b/src/mongo/db/sessions_collection_standalone.h
index 04e7eea9ed6..8c6f8d1f91a 100644
--- a/src/mongo/db/sessions_collection_standalone.h
+++ b/src/mongo/db/sessions_collection_standalone.h
@@ -51,7 +51,7 @@ public:
     /**
      * Checks if the sessions collection exists and has the proper indexes.
      */
-    Status checkSessionsCollectionExists(OperationContext* opCtx) override;
+    void checkSessionsCollectionExists(OperationContext* opCtx) override;
 
     /**
      * Updates the last-use times on the given sessions to be greater than