diff options
author | Gregory Wlodarek <gregory.wlodarek@mongodb.com> | 2021-08-20 17:35:14 +0000 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2021-08-25 21:17:47 +0000 |
commit | 0dd96157fb1fffc8eefe6be4b26aff9805bfbfac (patch) | |
tree | 1384ec27caa18815512d6a8d4362bfb4af6d9ca0 | |
parent | 6dbd76c0f5d2a5fceff2e8ea25bff5be71fc009b (diff) | |
download | mongo-0dd96157fb1fffc8eefe6be4b26aff9805bfbfac.tar.gz |
SERVER-59074 Do not acquire storage tickets just to set/wait on oplog visibility
(cherry picked from commit 64dd4524434f617b480e742e8dc421cccd8231fai)
-rw-r--r-- | src/mongo/db/concurrency/lock_state.h | 29 | ||||
-rw-r--r-- | src/mongo/db/concurrency/locker.h | 9 | ||||
-rw-r--r-- | src/mongo/db/repl/replication_coordinator_external_state_impl.cpp | 13 | ||||
-rw-r--r-- | src/mongo/db/repl/storage_interface_impl.cpp | 9 |
4 files changed, 52 insertions, 8 deletions
diff --git a/src/mongo/db/concurrency/lock_state.h b/src/mongo/db/concurrency/lock_state.h index dfa0e709d77..181f6eb2aae 100644 --- a/src/mongo/db/concurrency/lock_state.h +++ b/src/mongo/db/concurrency/lock_state.h @@ -407,6 +407,35 @@ public: }; /** + * RAII-style class to opt out of the ticket acquisition mechanism when acquiring a global lock. + * + * Operations that acquire the global lock but do not use any storage engine resources are eligible + * to skip ticket acquisition. Otherwise, a ticket acquisition is required to prevent throughput + * from suffering under high load. + */ +class SkipTicketAcquisitionForLock { +public: + SkipTicketAcquisitionForLock(const SkipTicketAcquisitionForLock&) = delete; + SkipTicketAcquisitionForLock& operator=(const SkipTicketAcquisitionForLock&) = delete; + explicit SkipTicketAcquisitionForLock(OperationContext* opCtx) + : _opCtx(opCtx), _shouldAcquireTicket(_opCtx->lockState()->shouldAcquireTicket()) { + if (_shouldAcquireTicket) { + _opCtx->lockState()->skipAcquireTicket(); + } + } + + ~SkipTicketAcquisitionForLock() { + if (_shouldAcquireTicket) { + _opCtx->lockState()->setAcquireTicket(); + } + } + +private: + OperationContext* _opCtx; + const bool _shouldAcquireTicket; +}; + +/** * Retrieves the global lock manager instance. */ LockManager* getGlobalLockManager(); diff --git a/src/mongo/db/concurrency/locker.h b/src/mongo/db/concurrency/locker.h index 746b65fa40d..8fd6a2e39bf 100644 --- a/src/mongo/db/concurrency/locker.h +++ b/src/mongo/db/concurrency/locker.h @@ -499,14 +499,19 @@ public: } /** - * This will opt out of the ticket mechanism. This should be used sparingly for special purpose - * threads, such as FTDC and committing or aborting prepared transactions. + * This will opt in or out of the ticket mechanism. This should be used sparingly for special + * purpose threads, such as FTDC and committing or aborting prepared transactions. */ void skipAcquireTicket() { // Should not hold or wait for the ticket. invariant(isNoop() || getClientState() == Locker::ClientState::kInactive); _shouldAcquireTicket = false; } + void setAcquireTicket() { + // Should hold or wait for the ticket. + invariant(isNoop() || getClientState() == Locker::ClientState::kInactive); + _shouldAcquireTicket = true; + } bool shouldAcquireTicket() const { return _shouldAcquireTicket; diff --git a/src/mongo/db/repl/replication_coordinator_external_state_impl.cpp b/src/mongo/db/repl/replication_coordinator_external_state_impl.cpp index 34303f125cb..d5957c5f3d5 100644 --- a/src/mongo/db/repl/replication_coordinator_external_state_impl.cpp +++ b/src/mongo/db/repl/replication_coordinator_external_state_impl.cpp @@ -410,14 +410,15 @@ Status ReplicationCoordinatorExternalStateImpl::initializeReplSetStorage(Operati const auto msgObj = BSON("msg" << kInitiatingSetMsg); _service->getOpObserver()->onOpMessage(opCtx, msgObj); wuow.commit(); - // ReplSetTest assumes that immediately after the replSetInitiate - // command returns, it can allow other nodes to initial sync with no - // retries and they will succeed. Unfortunately, initial sync will - // fail if it finds its sync source has an empty oplog. Thus, we - // need to wait here until the seed document is visible in our oplog. - _storageInterface->waitForAllEarlierOplogWritesToBeVisible(opCtx); }); + // ReplSetTest assumes that immediately after the replSetInitiate + // command returns, it can allow other nodes to initial sync with no + // retries and they will succeed. Unfortunately, initial sync will + // fail if it finds its sync source has an empty oplog. Thus, we + // need to wait here until the seed document is visible in our oplog. + _storageInterface->waitForAllEarlierOplogWritesToBeVisible(opCtx); + // Update unique index format version for all non-replicated collections. It is possible // for MongoDB to have a "clean startup", i.e., no non-local databases, but still have // unique indexes on collections in the local database. On clean startup, diff --git a/src/mongo/db/repl/storage_interface_impl.cpp b/src/mongo/db/repl/storage_interface_impl.cpp index e0527d461bb..2cf0f9b35c9 100644 --- a/src/mongo/db/repl/storage_interface_impl.cpp +++ b/src/mongo/db/repl/storage_interface_impl.cpp @@ -52,6 +52,7 @@ #include "mongo/db/catalog/index_catalog.h" #include "mongo/db/client.h" #include "mongo/db/concurrency/d_concurrency.h" +#include "mongo/db/concurrency/lock_state.h" #include "mongo/db/concurrency/write_conflict_exception.h" #include "mongo/db/curop.h" #include "mongo/db/db_raii.h" @@ -1150,6 +1151,10 @@ Status StorageInterfaceImpl::isAdminDbValid(OperationContext* opCtx) { void StorageInterfaceImpl::waitForAllEarlierOplogWritesToBeVisible(OperationContext* opCtx, bool primaryOnly) { + // Waiting for oplog writes to be visible in the oplog does not use any storage engine resources + // and must skip ticket acquisition to avoid deadlocks with updating oplog visibility. + SkipTicketAcquisitionForLock skipTicketAcquisition(opCtx); + Lock::GlobalLock lk(opCtx, MODE_IS); if (primaryOnly && !repl::ReplicationCoordinator::get(opCtx)->canAcceptWritesForDatabase(opCtx, "admin")) @@ -1171,6 +1176,10 @@ void StorageInterfaceImpl::waitForAllEarlierOplogWritesToBeVisible(OperationCont void StorageInterfaceImpl::oplogDiskLocRegister(OperationContext* opCtx, const Timestamp& ts, bool orderedCommit) { + // Setting the oplog visibility does not use any storage engine resources and must skip ticket + // acquisition to avoid deadlocks with updating oplog visibility. + SkipTicketAcquisitionForLock skipTicketAcquisition(opCtx); + AutoGetCollection oplog(opCtx, NamespaceString::kRsOplogNamespace, MODE_IS); fassert( 28557, |