diff options
author | Marcos José Grillo Ramírez <marcos.grillo@10gen.com> | 2019-10-29 12:07:54 +0000 |
---|---|---|
committer | evergreen <evergreen@mongodb.com> | 2019-10-29 12:07:54 +0000 |
commit | 81f52a69c92d27034ef3a9bf411fb463cab7643e (patch) | |
tree | 37a57eaa120a6a3c71a515a7f39dfceb21ee8cee /src | |
parent | af1a00179bd00083fd6d4c3af34140e4ba2aac6d (diff) | |
download | mongo-81f52a69c92d27034ef3a9bf411fb463cab7643e.tar.gz |
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
Diffstat (limited to 'src')
-rw-r--r-- | src/mongo/db/logical_session_cache_impl.cpp | 15 | ||||
-rw-r--r-- | src/mongo/db/sessions_collection.h | 2 | ||||
-rw-r--r-- | src/mongo/db/sessions_collection_config_server.cpp | 4 | ||||
-rw-r--r-- | src/mongo/db/sessions_collection_config_server.h | 2 | ||||
-rw-r--r-- | src/mongo/db/sessions_collection_mock.h | 4 | ||||
-rw-r--r-- | src/mongo/db/sessions_collection_rs.cpp | 89 | ||||
-rw-r--r-- | src/mongo/db/sessions_collection_rs.h | 5 | ||||
-rw-r--r-- | src/mongo/db/sessions_collection_sharded.cpp | 6 | ||||
-rw-r--r-- | src/mongo/db/sessions_collection_sharded.h | 2 | ||||
-rw-r--r-- | src/mongo/db/sessions_collection_standalone.cpp | 71 | ||||
-rw-r--r-- | src/mongo/db/sessions_collection_standalone.h | 2 |
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 |