diff options
author | Dianna <dianna.hohensee@10gen.com> | 2019-05-09 08:13:31 -0400 |
---|---|---|
committer | Dianna <dianna.hohensee@10gen.com> | 2019-05-14 18:10:38 -0400 |
commit | 09db7023065f42ccc39dd3309536726814379c86 (patch) | |
tree | 7d7f153b943b1afc0546433b3efae61bd1812cd9 | |
parent | afa6c9136387f07c0bba8dfc33178acfb826fe43 (diff) | |
download | mongo-09db7023065f42ccc39dd3309536726814379c86.tar.gz |
SERVER-39860 Separate reporting of RSTL and PBWM locks metrics in serverStatus and currentOp
-rw-r--r-- | jstests/replsets/dbhash_lock_acquisition.js | 8 | ||||
-rw-r--r-- | src/mongo/db/catalog/rename_collection.cpp | 4 | ||||
-rw-r--r-- | src/mongo/db/concurrency/d_concurrency.cpp | 3 | ||||
-rw-r--r-- | src/mongo/db/concurrency/d_concurrency_test.cpp | 60 | ||||
-rw-r--r-- | src/mongo/db/concurrency/lock_manager_defs.h | 54 | ||||
-rw-r--r-- | src/mongo/db/concurrency/lock_state.cpp | 39 | ||||
-rw-r--r-- | src/mongo/db/concurrency/lock_state_test.cpp | 20 | ||||
-rw-r--r-- | src/mongo/db/concurrency/lock_stats.cpp | 28 | ||||
-rw-r--r-- | src/mongo/db/concurrency/lock_stats.h | 38 | ||||
-rw-r--r-- | src/mongo/db/concurrency/lock_stats_test.cpp | 30 | ||||
-rw-r--r-- | src/mongo/db/stats/fill_locker_info_test.cpp | 25 | ||||
-rw-r--r-- | src/mongo/db/storage/mobile/mobile_recovery_unit.cpp | 6 |
12 files changed, 148 insertions, 167 deletions
diff --git a/jstests/replsets/dbhash_lock_acquisition.js b/jstests/replsets/dbhash_lock_acquisition.js index cf26fb6caea..2376bba6e68 100644 --- a/jstests/replsets/dbhash_lock_acquisition.js +++ b/jstests/replsets/dbhash_lock_acquisition.js @@ -31,7 +31,8 @@ assert.eq(1, ops.length, () => "Failed to find session in currentOp() output: " + tojson(db.currentOp())); - assert.eq(ops[0].locks, {Global: "w", Database: "w", Collection: "w"}); + assert.eq(ops[0].locks, + {ReplicationStateTransition: "w", Global: "w", Database: "w", Collection: "w"}); const threadCaptruncCmd = new ScopedThread(function(host) { try { @@ -78,9 +79,8 @@ if (ops.length === 0) { return false; } - // The lock mode for the Global resource is reported as "w" rather than "r" because of the - // mode taken for the ReplicationStateTransitionLock when acquiring the global lock. - assert.eq(ops[0].locks, {Global: "w", Database: "r", Collection: "r"}); + assert.eq(ops[0].locks, + {ReplicationStateTransition: "w", Global: "r", Database: "r", Collection: "r"}); return true; }, () => "Failed to find create collection in currentOp() output: " + tojson(db.currentOp())); diff --git a/src/mongo/db/catalog/rename_collection.cpp b/src/mongo/db/catalog/rename_collection.cpp index 203db323fd5..7aa953aa7c6 100644 --- a/src/mongo/db/catalog/rename_collection.cpp +++ b/src/mongo/db/catalog/rename_collection.cpp @@ -650,9 +650,7 @@ Status renameBetweenDBs(OperationContext* opCtx, if (opCtx->getServiceContext()->getStorageEngine()->supportsDBLocking()) { if (globalWriteLock) { - const ResourceId globalLockResourceId(RESOURCE_GLOBAL, - ResourceId::SINGLETON_GLOBAL); - opCtx->lockState()->downgrade(globalLockResourceId, MODE_IX); + opCtx->lockState()->downgrade(resourceIdGlobal, MODE_IX); invariant(!opCtx->lockState()->isW()); } else { invariant(opCtx->lockState()->isW()); diff --git a/src/mongo/db/concurrency/d_concurrency.cpp b/src/mongo/db/concurrency/d_concurrency.cpp index c23e3937c2e..37c841594b0 100644 --- a/src/mongo/db/concurrency/d_concurrency.cpp +++ b/src/mongo/db/concurrency/d_concurrency.cpp @@ -215,8 +215,7 @@ void Lock::GlobalLock::waitForLockUntil(Date_t deadline) { throw; } - const ResourceId globalResId(RESOURCE_GLOBAL, ResourceId::SINGLETON_GLOBAL); - auto lockMode = _opCtx->lockState()->getLockMode(globalResId); + auto lockMode = _opCtx->lockState()->getLockMode(resourceIdGlobal); _opCtx->lockState()->setGlobalLockTakenInMode(lockMode); } diff --git a/src/mongo/db/concurrency/d_concurrency_test.cpp b/src/mongo/db/concurrency/d_concurrency_test.cpp index 3d400a9bceb..ddf60cc23a1 100644 --- a/src/mongo/db/concurrency/d_concurrency_test.cpp +++ b/src/mongo/db/concurrency/d_concurrency_test.cpp @@ -288,19 +288,17 @@ TEST_F(DConcurrencyTestFixture, opCtx->swapLockState(stdx::make_unique<LockerImpl>()); auto lockState = opCtx->lockState(); - const ResourceId globalId(RESOURCE_GLOBAL, ResourceId::SINGLETON_GLOBAL); - auto globalWrite = stdx::make_unique<Lock::GlobalWrite>(opCtx.get()); ASSERT(lockState->isW()); - ASSERT(MODE_X == lockState->getLockMode(globalId)) - << "unexpected global lock mode " << modeName(lockState->getLockMode(globalId)); + ASSERT(MODE_X == lockState->getLockMode(resourceIdGlobal)) + << "unexpected global lock mode " << modeName(lockState->getLockMode(resourceIdGlobal)); ASSERT_EQ(lockState->getLockMode(resourceIdReplicationStateTransitionLock), MODE_IX); { Lock::DBLock dbWrite(opCtx.get(), "db", MODE_IX); ASSERT(lockState->isW()); - ASSERT(MODE_X == lockState->getLockMode(globalId)) - << "unexpected global lock mode " << modeName(lockState->getLockMode(globalId)); + ASSERT(MODE_X == lockState->getLockMode(resourceIdGlobal)) + << "unexpected global lock mode " << modeName(lockState->getLockMode(resourceIdGlobal)); // If we destroy the GlobalWrite out of order relative to the DBLock, we will leave the // global lock resource locked in MODE_X. We have to explicitly downgrade this resource to @@ -310,19 +308,19 @@ TEST_F(DConcurrencyTestFixture, ASSERT(lockState->isW()); ASSERT_EQ(lockState->getLockMode(resourceIdReplicationStateTransitionLock), MODE_IX); - lockState->downgrade(globalId, MODE_IX); + lockState->downgrade(resourceIdGlobal, MODE_IX); ASSERT_FALSE(lockState->isW()); ASSERT(lockState->isWriteLocked()); - ASSERT(MODE_IX == lockState->getLockMode(globalId)) - << "unexpected global lock mode " << modeName(lockState->getLockMode(globalId)); + ASSERT(MODE_IX == lockState->getLockMode(resourceIdGlobal)) + << "unexpected global lock mode " << modeName(lockState->getLockMode(resourceIdGlobal)); ASSERT_EQ(lockState->getLockMode(resourceIdReplicationStateTransitionLock), MODE_IX); } ASSERT_FALSE(lockState->isW()); ASSERT_FALSE(lockState->isWriteLocked()); - ASSERT(MODE_NONE == lockState->getLockMode(globalId)) - << "unexpected global lock mode " << modeName(lockState->getLockMode(globalId)); + ASSERT(MODE_NONE == lockState->getLockMode(resourceIdGlobal)) + << "unexpected global lock mode " << modeName(lockState->getLockMode(resourceIdGlobal)); ASSERT_EQ(lockState->getLockMode(resourceIdReplicationStateTransitionLock), MODE_NONE); } @@ -332,28 +330,26 @@ TEST_F(DConcurrencyTestFixture, opCtx->swapLockState(stdx::make_unique<LockerImpl>()); auto lockState = opCtx->lockState(); - const ResourceId globalId(RESOURCE_GLOBAL, ResourceId::SINGLETON_GLOBAL); - auto globalWrite = stdx::make_unique<Lock::GlobalWrite>(opCtx.get()); ASSERT(lockState->isW()); - ASSERT(MODE_X == lockState->getLockMode(globalId)) - << "unexpected global lock mode " << modeName(lockState->getLockMode(globalId)); + ASSERT(MODE_X == lockState->getLockMode(resourceIdGlobal)) + << "unexpected global lock mode " << modeName(lockState->getLockMode(resourceIdGlobal)); ASSERT_EQ(lockState->getLockMode(resourceIdReplicationStateTransitionLock), MODE_IX); { Lock::DBLock dbWrite(opCtx.get(), "db", MODE_IX); ASSERT(lockState->isW()); - ASSERT(MODE_X == lockState->getLockMode(globalId)) - << "unexpected global lock mode " << modeName(lockState->getLockMode(globalId)); + ASSERT(MODE_X == lockState->getLockMode(resourceIdGlobal)) + << "unexpected global lock mode " << modeName(lockState->getLockMode(resourceIdGlobal)); ASSERT_EQ(lockState->getLockMode(resourceIdReplicationStateTransitionLock), MODE_IX); // Downgrade global lock resource to MODE_IX to allow other write operations to make // progress. - lockState->downgrade(globalId, MODE_IX); + lockState->downgrade(resourceIdGlobal, MODE_IX); ASSERT_FALSE(lockState->isW()); ASSERT(lockState->isWriteLocked()); - ASSERT(MODE_IX == lockState->getLockMode(globalId)) - << "unexpected global lock mode " << modeName(lockState->getLockMode(globalId)); + ASSERT(MODE_IX == lockState->getLockMode(resourceIdGlobal)) + << "unexpected global lock mode " << modeName(lockState->getLockMode(resourceIdGlobal)); ASSERT_EQ(lockState->getLockMode(resourceIdReplicationStateTransitionLock), MODE_IX); } @@ -364,8 +360,8 @@ TEST_F(DConcurrencyTestFixture, globalWrite = {}; ASSERT_FALSE(lockState->isW()); ASSERT_FALSE(lockState->isWriteLocked()); - ASSERT(MODE_NONE == lockState->getLockMode(globalId)) - << "unexpected global lock mode " << modeName(lockState->getLockMode(globalId)); + ASSERT(MODE_NONE == lockState->getLockMode(resourceIdGlobal)) + << "unexpected global lock mode " << modeName(lockState->getLockMode(resourceIdGlobal)); ASSERT_EQ(lockState->getLockMode(resourceIdReplicationStateTransitionLock), MODE_NONE); } @@ -375,8 +371,6 @@ TEST_F(DConcurrencyTestFixture, opCtx->swapLockState(stdx::make_unique<LockerImpl>()); auto lockState = opCtx->lockState(); - const ResourceId globalId(RESOURCE_GLOBAL, ResourceId::SINGLETON_GLOBAL); - auto outerGlobalWrite = stdx::make_unique<Lock::GlobalWrite>(opCtx.get()); auto innerGlobalWrite = stdx::make_unique<Lock::GlobalWrite>(opCtx.get()); ASSERT_EQ(lockState->getLockMode(resourceIdReplicationStateTransitionLock), MODE_IX); @@ -384,17 +378,17 @@ TEST_F(DConcurrencyTestFixture, { Lock::DBLock dbWrite(opCtx.get(), "db", MODE_IX); ASSERT(lockState->isW()); - ASSERT(MODE_X == lockState->getLockMode(globalId)) - << "unexpected global lock mode " << modeName(lockState->getLockMode(globalId)); + ASSERT(MODE_X == lockState->getLockMode(resourceIdGlobal)) + << "unexpected global lock mode " << modeName(lockState->getLockMode(resourceIdGlobal)); ASSERT_EQ(lockState->getLockMode(resourceIdReplicationStateTransitionLock), MODE_IX); // Downgrade global lock resource to MODE_IX to allow other write operations to make // progress. - lockState->downgrade(globalId, MODE_IX); + lockState->downgrade(resourceIdGlobal, MODE_IX); ASSERT_FALSE(lockState->isW()); ASSERT(lockState->isWriteLocked()); - ASSERT(MODE_IX == lockState->getLockMode(globalId)) - << "unexpected global lock mode " << modeName(lockState->getLockMode(globalId)); + ASSERT(MODE_IX == lockState->getLockMode(resourceIdGlobal)) + << "unexpected global lock mode " << modeName(lockState->getLockMode(resourceIdGlobal)); ASSERT_EQ(lockState->getLockMode(resourceIdReplicationStateTransitionLock), MODE_IX); } @@ -405,15 +399,15 @@ TEST_F(DConcurrencyTestFixture, innerGlobalWrite = {}; ASSERT_FALSE(lockState->isW()); ASSERT(lockState->isWriteLocked()); - ASSERT(MODE_IX == lockState->getLockMode(globalId)) - << "unexpected global lock mode " << modeName(lockState->getLockMode(globalId)); + ASSERT(MODE_IX == lockState->getLockMode(resourceIdGlobal)) + << "unexpected global lock mode " << modeName(lockState->getLockMode(resourceIdGlobal)); ASSERT_EQ(lockState->getLockMode(resourceIdReplicationStateTransitionLock), MODE_IX); outerGlobalWrite = {}; ASSERT_FALSE(lockState->isW()); ASSERT_FALSE(lockState->isWriteLocked()); - ASSERT(MODE_NONE == lockState->getLockMode(globalId)) - << "unexpected global lock mode " << modeName(lockState->getLockMode(globalId)); + ASSERT(MODE_NONE == lockState->getLockMode(resourceIdGlobal)) + << "unexpected global lock mode " << modeName(lockState->getLockMode(resourceIdGlobal)); ASSERT_EQ(lockState->getLockMode(resourceIdReplicationStateTransitionLock), MODE_NONE); } diff --git a/src/mongo/db/concurrency/lock_manager_defs.h b/src/mongo/db/concurrency/lock_manager_defs.h index 141b3ee4899..4d80fab87c5 100644 --- a/src/mongo/db/concurrency/lock_manager_defs.h +++ b/src/mongo/db/concurrency/lock_manager_defs.h @@ -153,13 +153,18 @@ enum LockResult { * ordering is consistent so deadlock cannot occur. */ enum ResourceType { - /** Types used for special resources, use with a hash id from ResourceId::SingletonHashIds. */ RESOURCE_INVALID = 0, - /** Used for mode changes or global exclusive operations */ + /** Parallel batch writer mutex */ + RESOURCE_PBWM, + + /** Replication state transition lock. */ + RESOURCE_RSTL, + + /** Used for global exclusive operations */ RESOURCE_GLOBAL, - /** Generic resources, used for multi-granularity locking, together with RESOURCE_GLOBAL */ + /** Generic resources, used for multi-granularity locking, together with the above locks */ RESOURCE_DATABASE, RESOURCE_COLLECTION, RESOURCE_METADATA, @@ -174,8 +179,14 @@ enum ResourceType { /** * Maps the resource id to a human-readable string. */ -static const char* ResourceTypeNames[] = { - "Invalid", "Global", "Database", "Collection", "Metadata", "Mutex"}; +static const char* ResourceTypeNames[] = {"Invalid", + "ParallelBatchWriter", + "ReplicationStateTransition", + "Global", + "Database", + "Collection", + "Metadata", + "Mutex"}; // Ensure we do not add new types without updating the names array. MONGO_STATIC_ASSERT((sizeof(ResourceTypeNames) / sizeof(ResourceTypeNames[0])) == @@ -197,17 +208,6 @@ class ResourceId { MONGO_STATIC_ASSERT(ResourceTypesCount <= (1 << resourceTypeBits)); public: - /** - * Assign hash ids for special resources to avoid accidental reuse of ids. For ids used - * with the same ResourceType, the order here must be the same as the locking order. - */ - enum SingletonHashIds { - SINGLETON_INVALID = 0, - SINGLETON_PARALLEL_BATCH_WRITER_MODE, - SINGLETON_REPLICATION_STATE_TRANSITION_LOCK, - SINGLETON_GLOBAL, - }; - ResourceId() : _fullHash(0) {} ResourceId(ResourceType type, StringData ns) : _fullHash(fullHash(type, hashStringData(ns))) {} ResourceId(ResourceType type, const std::string& ns) @@ -283,25 +283,17 @@ extern const ResourceId resourceIdAdminDB; // Global lock. Every server operation, which uses the Locker must acquire this lock at least // once. See comments in the header file (begin/endTransaction) for more information. -// -// There are multiple locks with the "GLOBAL" type. This one is colloquially known as the global -// lock. extern const ResourceId resourceIdGlobal; -// Hardcoded resource id for ParallelBatchWriterMode. We use the same resource type -// as resourceIdGlobal. This will also ensure the waits are reported as global, which -// is appropriate. The lock will never be contended unless the parallel batch writers -// must stop all other accesses globally. This resource must be locked before all other -// resources (including resourceIdGlobal). Replication applier threads don't take this -// lock. -// TODO: Merge this with resourceIdGlobal +// Hardcoded resource id for ParallelBatchWriterMode (PBWM). The lock will never be contended unless +// the parallel batch writers must stop all other accesses globally. This resource must be locked +// before all other resources (including resourceIdGlobal). Replication applier threads don't take +// this lock. extern const ResourceId resourceIdParallelBatchWriterMode; -// Hardcoded resource id for the ReplicationStateTransitionLock (RSTL). We use the same resource -// type as resourceIdGlobal. This will also ensure the waits are reported as global, which is -// appropriate. This lock is acquired in mode X for any replication state transition and is acquired -// by all other reads and writes in mode IX. This lock is acquired after the PBWM but before the -// resourceIdGlobal. +// Hardcoded resource id for the ReplicationStateTransitionLock (RSTL). This lock is acquired in +// mode X for any replication state transition and is acquired by all other reads and writes in mode +// IX. This lock is acquired after the PBWM but before the resourceIdGlobal. extern const ResourceId resourceIdReplicationStateTransitionLock; /** diff --git a/src/mongo/db/concurrency/lock_state.cpp b/src/mongo/db/concurrency/lock_state.cpp index 75a6a771eea..44a5aa20ed0 100644 --- a/src/mongo/db/concurrency/lock_state.cpp +++ b/src/mongo/db/concurrency/lock_state.cpp @@ -56,7 +56,14 @@ MONGO_FAIL_POINT_DEFINE(failNonIntentLocksIfWaitNeeded); namespace { /** - * Partitioned global lock statistics, so we don't hit the same bucket. + * Tracks global (across all clients) lock acquisition statistics, partitioned into multiple + * buckets to minimize concurrent access conflicts. + * + * Each client has a LockerId that monotonically increases across all client instances. The + * LockerId % 8 is used to index into one of 8 LockStats instances. These LockStats objects must be + * atomically accessed, so maintaining 8 that are indexed by LockerId reduces client conflicts and + * improves concurrent write access. A reader, to collect global lock statics for reporting, will + * sum the results of all 8 disjoint 'buckets' of stats. */ class PartitionedInstanceWideLockStats { PartitionedInstanceWideLockStats(const PartitionedInstanceWideLockStats&) = delete; @@ -117,7 +124,8 @@ const Milliseconds MaxWaitTime = Milliseconds(500); // Dispenses unique LockerId identifiers AtomicWord<unsigned long long> idCounter(0); -// Partitioned global lock statistics, so we don't hit the same bucket +// Tracks lock statistics across all Locker instances. Distributes stats across multiple buckets +// indexed by LockerId in order to minimize concurrent access conflicts. PartitionedInstanceWideLockStats globalStats; } // namespace @@ -128,6 +136,8 @@ bool LockerImpl::_shouldDelayUnlock(ResourceId resId, LockMode mode) const { return false; case RESOURCE_GLOBAL: + case RESOURCE_PBWM: + case RESOURCE_RSTL: case RESOURCE_DATABASE: case RESOURCE_COLLECTION: case RESOURCE_METADATA: @@ -395,7 +405,9 @@ bool LockerImpl::unlockGlobal() { // If we're here we should only have one reference to any lock. It is a programming // error for any lock used with multi-granularity locking to have more references than // the global lock, because every scope starts by calling lockGlobal. - if (it.key().getType() == RESOURCE_GLOBAL || it.key().getType() == RESOURCE_MUTEX) { + const auto resType = it.key().getType(); + if (resType == RESOURCE_GLOBAL || resType == RESOURCE_PBWM || resType == RESOURCE_RSTL || + resType == RESOURCE_MUTEX) { it.next(); } else { invariant(_unlockImpl(&it)); @@ -749,9 +761,10 @@ bool LockerImpl::saveLockStateAndUnlock(Locker::LockSnapshot* stateOut) { continue; // We should never have to save and restore metadata locks. - invariant(RESOURCE_DATABASE == resId.getType() || RESOURCE_COLLECTION == resId.getType() || - (RESOURCE_GLOBAL == resId.getType() && isSharedLockMode(it->mode)) || - (resourceIdReplicationStateTransitionLock == resId && it->mode == MODE_IX)); + invariant(RESOURCE_DATABASE == resType || RESOURCE_COLLECTION == resType || + (RESOURCE_GLOBAL == resType && isSharedLockMode(it->mode)) || + (RESOURCE_PBWM == resType && isSharedLockMode(it->mode)) || + (RESOURCE_RSTL == resType && it->mode == MODE_IX)); // And, stuff the info into the out parameter. OneLock info; @@ -837,7 +850,7 @@ LockResult LockerImpl::lockBegin(OperationContext* opCtx, ResourceId resId, Lock // Give priority to the full modes for Global, PBWM, and RSTL resources so we don't stall global // operations such as shutdown or stepdown. const ResourceType resType = resId.getType(); - if (resType == RESOURCE_GLOBAL) { + if (resType == RESOURCE_GLOBAL || resType == RESOURCE_PBWM || resType == RESOURCE_RSTL) { if (mode == MODE_S || mode == MODE_X) { request->enqueueAtFront = true; request->compatibleFirst = true; @@ -976,6 +989,7 @@ void LockerImpl::getFlowControlTicket(OperationContext* opCtx, LockMode lockMode } LockResult LockerImpl::lockRSTLBegin(OperationContext* opCtx, LockMode mode) { + invariant(mode == MODE_IX || mode == MODE_X); return lockBegin(opCtx, resourceIdReplicationStateTransitionLock, mode); } @@ -1063,13 +1077,12 @@ void resetGlobalLockStats() { globalStats.reset(); } -// Definition for the hardcoded localdb and oplog collection info +// Hardcoded resource IDs. const ResourceId resourceIdLocalDB = ResourceId(RESOURCE_DATABASE, StringData("local")); const ResourceId resourceIdOplog = ResourceId(RESOURCE_COLLECTION, StringData("local.oplog.rs")); const ResourceId resourceIdAdminDB = ResourceId(RESOURCE_DATABASE, StringData("admin")); -const ResourceId resourceIdGlobal = ResourceId(RESOURCE_GLOBAL, ResourceId::SINGLETON_GLOBAL); -const ResourceId resourceIdParallelBatchWriterMode = - ResourceId(RESOURCE_GLOBAL, ResourceId::SINGLETON_PARALLEL_BATCH_WRITER_MODE); -const ResourceId resourceIdReplicationStateTransitionLock = - ResourceId(RESOURCE_GLOBAL, ResourceId::SINGLETON_REPLICATION_STATE_TRANSITION_LOCK); +const ResourceId resourceIdGlobal = ResourceId(RESOURCE_GLOBAL, 1ULL); +const ResourceId resourceIdParallelBatchWriterMode = ResourceId(RESOURCE_PBWM, 1ULL); +const ResourceId resourceIdReplicationStateTransitionLock = ResourceId(RESOURCE_RSTL, 1ULL); + } // namespace mongo diff --git a/src/mongo/db/concurrency/lock_state_test.cpp b/src/mongo/db/concurrency/lock_state_test.cpp index 7c7f25418ff..76f51cf9a13 100644 --- a/src/mongo/db/concurrency/lock_state_test.cpp +++ b/src/mongo/db/concurrency/lock_state_test.cpp @@ -668,7 +668,6 @@ TEST_F(LockerImplTest, SharedLocksShouldTwoPhaseLockIsTrue) { // and S locks are postponed until endWriteUnitOfWork() is called. Mode IX and X locks always // participate in two-phased locking, regardless of the setting. - const ResourceId globalResId(RESOURCE_GLOBAL, ResourceId::SINGLETON_GLOBAL); const ResourceId resId1(RESOURCE_DATABASE, "TestDB1"_sd); const ResourceId resId2(RESOURCE_DATABASE, "TestDB2"_sd); const ResourceId resId3(RESOURCE_COLLECTION, "TestDB.collection3"_sd); @@ -678,7 +677,7 @@ TEST_F(LockerImplTest, SharedLocksShouldTwoPhaseLockIsTrue) { locker.setSharedLocksShouldTwoPhaseLock(true); locker.lockGlobal(MODE_IS); - ASSERT_EQ(locker.getLockMode(globalResId), MODE_IS); + ASSERT_EQ(locker.getLockMode(resourceIdGlobal), MODE_IS); locker.lock(resourceIdReplicationStateTransitionLock, MODE_IS); ASSERT_EQ(locker.getLockMode(resourceIdReplicationStateTransitionLock), MODE_IS); @@ -707,7 +706,7 @@ TEST_F(LockerImplTest, SharedLocksShouldTwoPhaseLockIsTrue) { ASSERT_EQ(locker.getLockMode(resourceIdReplicationStateTransitionLock), MODE_IS); ASSERT_FALSE(locker.unlockGlobal()); - ASSERT_EQ(locker.getLockMode(globalResId), MODE_IS); + ASSERT_EQ(locker.getLockMode(resourceIdGlobal), MODE_IS); locker.endWriteUnitOfWork(); @@ -716,14 +715,13 @@ TEST_F(LockerImplTest, SharedLocksShouldTwoPhaseLockIsTrue) { ASSERT_EQ(locker.getLockMode(resId3), MODE_NONE); ASSERT_EQ(locker.getLockMode(resId4), MODE_NONE); ASSERT_EQ(locker.getLockMode(resourceIdReplicationStateTransitionLock), MODE_NONE); - ASSERT_EQ(locker.getLockMode(globalResId), MODE_NONE); + ASSERT_EQ(locker.getLockMode(resourceIdGlobal), MODE_NONE); } TEST_F(LockerImplTest, ModeIXAndXLockParticipatesInTwoPhaseLocking) { // Unlock on mode IX and X locks during a WUOW should always be postponed until // endWriteUnitOfWork() is called. Mode IS and S locks should unlock immediately. - const ResourceId globalResId(RESOURCE_GLOBAL, ResourceId::SINGLETON_GLOBAL); const ResourceId resId1(RESOURCE_DATABASE, "TestDB1"_sd); const ResourceId resId2(RESOURCE_DATABASE, "TestDB2"_sd); const ResourceId resId3(RESOURCE_COLLECTION, "TestDB.collection3"_sd); @@ -732,7 +730,7 @@ TEST_F(LockerImplTest, ModeIXAndXLockParticipatesInTwoPhaseLocking) { LockerImpl locker; locker.lockGlobal(MODE_IX); - ASSERT_EQ(locker.getLockMode(globalResId), MODE_IX); + ASSERT_EQ(locker.getLockMode(resourceIdGlobal), MODE_IX); locker.lock(resourceIdReplicationStateTransitionLock, MODE_IX); ASSERT_EQ(locker.getLockMode(resourceIdReplicationStateTransitionLock), MODE_IX); @@ -761,14 +759,14 @@ TEST_F(LockerImplTest, ModeIXAndXLockParticipatesInTwoPhaseLocking) { ASSERT_EQ(locker.getLockMode(resourceIdReplicationStateTransitionLock), MODE_IX); ASSERT_FALSE(locker.unlockGlobal()); - ASSERT_EQ(locker.getLockMode(globalResId), MODE_IX); + ASSERT_EQ(locker.getLockMode(resourceIdGlobal), MODE_IX); locker.endWriteUnitOfWork(); ASSERT_EQ(locker.getLockMode(resId2), MODE_NONE); ASSERT_EQ(locker.getLockMode(resId4), MODE_NONE); ASSERT_EQ(locker.getLockMode(resourceIdReplicationStateTransitionLock), MODE_NONE); - ASSERT_EQ(locker.getLockMode(globalResId), MODE_NONE); + ASSERT_EQ(locker.getLockMode(resourceIdGlobal), MODE_NONE); } TEST_F(LockerImplTest, RSTLUnlocksWithNestedLock) { @@ -950,7 +948,6 @@ bool lockerInfoContainsLock(const Locker::LockerInfo& lockerInfo, } // namespace TEST_F(LockerImplTest, GetLockerInfoShouldReportHeldLocks) { - const ResourceId globalId(RESOURCE_GLOBAL, ResourceId::SINGLETON_GLOBAL); const ResourceId dbId(RESOURCE_DATABASE, "TestDB"_sd); const ResourceId collectionId(RESOURCE_COLLECTION, "TestDB.collection"_sd); @@ -964,7 +961,7 @@ TEST_F(LockerImplTest, GetLockerInfoShouldReportHeldLocks) { Locker::LockerInfo lockerInfo; locker.getLockerInfo(&lockerInfo, boost::none); - ASSERT(lockerInfoContainsLock(lockerInfo, globalId, MODE_IX)); + ASSERT(lockerInfoContainsLock(lockerInfo, resourceIdGlobal, MODE_IX)); ASSERT(lockerInfoContainsLock(lockerInfo, dbId, MODE_IX)); ASSERT(lockerInfoContainsLock(lockerInfo, collectionId, MODE_X)); ASSERT_EQ(3U, lockerInfo.locks.size()); @@ -975,7 +972,6 @@ TEST_F(LockerImplTest, GetLockerInfoShouldReportHeldLocks) { } TEST_F(LockerImplTest, GetLockerInfoShouldReportPendingLocks) { - const ResourceId globalId(RESOURCE_GLOBAL, ResourceId::SINGLETON_GLOBAL); const ResourceId dbId(RESOURCE_DATABASE, "TestDB"_sd); const ResourceId collectionId(RESOURCE_COLLECTION, "TestDB.collection"_sd); @@ -994,7 +990,7 @@ TEST_F(LockerImplTest, GetLockerInfoShouldReportPendingLocks) { // Assert the held locks show up in the output of getLockerInfo(). Locker::LockerInfo lockerInfo; conflictingLocker.getLockerInfo(&lockerInfo, boost::none); - ASSERT(lockerInfoContainsLock(lockerInfo, globalId, MODE_IS)); + ASSERT(lockerInfoContainsLock(lockerInfo, resourceIdGlobal, MODE_IS)); ASSERT(lockerInfoContainsLock(lockerInfo, dbId, MODE_IS)); ASSERT(lockerInfoContainsLock(lockerInfo, collectionId, MODE_IS)); ASSERT_EQ(3U, lockerInfo.locks.size()); diff --git a/src/mongo/db/concurrency/lock_stats.cpp b/src/mongo/db/concurrency/lock_stats.cpp index 24e04f49450..f724c90a240 100644 --- a/src/mongo/db/concurrency/lock_stats.cpp +++ b/src/mongo/db/concurrency/lock_stats.cpp @@ -45,20 +45,16 @@ void LockStats<CounterType>::report(BSONObjBuilder* builder) const { // All indexing below starts from offset 1, because we do not want to report/account // position 0, which is a sentinel value for invalid resource/no lock. for (int i = 1; i < ResourceTypesCount; i++) { - _report(builder, - resourceTypeName(static_cast<ResourceType>(i)), - _stats[i], - static_cast<ResourceType>(i) == ResourceType::RESOURCE_GLOBAL); + _report(builder, resourceTypeName(static_cast<ResourceType>(i)), _stats[i]); } - _report(builder, "oplog", _oplogStats, false); + _report(builder, "oplog", _oplogStats); } template <typename CounterType> void LockStats<CounterType>::_report(BSONObjBuilder* builder, - const char* sectionName, - const PerModeLockStatCounters& stat, - const bool includeCanonicalGlobal) const { + const char* resourceTypeName, + const PerModeLockStatCounters& stat) const { std::unique_ptr<BSONObjBuilder> section; // All indexing below starts from offset 1, because we do not want to report/account @@ -69,14 +65,11 @@ void LockStats<CounterType>::_report(BSONObjBuilder* builder, std::unique_ptr<BSONObjBuilder> numAcquires; for (int mode = 1; mode < LockModesCount; mode++) { long long value = CounterOps::get(stat.modeStats[mode].numAcquisitions); - if (includeCanonicalGlobal) { - value += CounterOps::get(_resourceIdGlobal.modeStats[mode].numAcquisitions); - } if (value > 0) { if (!numAcquires) { if (!section) { - section.reset(new BSONObjBuilder(builder->subobjStart(sectionName))); + section.reset(new BSONObjBuilder(builder->subobjStart(resourceTypeName))); } numAcquires.reset(new BSONObjBuilder(section->subobjStart("acquireCount"))); @@ -91,13 +84,10 @@ void LockStats<CounterType>::_report(BSONObjBuilder* builder, std::unique_ptr<BSONObjBuilder> numWaits; for (int mode = 1; mode < LockModesCount; mode++) { long long value = CounterOps::get(stat.modeStats[mode].numWaits); - if (includeCanonicalGlobal) { - value += CounterOps::get(_resourceIdGlobal.modeStats[mode].numWaits); - } if (value > 0) { if (!numWaits) { if (!section) { - section.reset(new BSONObjBuilder(builder->subobjStart(sectionName))); + section.reset(new BSONObjBuilder(builder->subobjStart(resourceTypeName))); } numWaits.reset(new BSONObjBuilder(section->subobjStart("acquireWaitCount"))); @@ -112,13 +102,10 @@ void LockStats<CounterType>::_report(BSONObjBuilder* builder, std::unique_ptr<BSONObjBuilder> timeAcquiring; for (int mode = 1; mode < LockModesCount; mode++) { long long value = CounterOps::get(stat.modeStats[mode].combinedWaitTimeMicros); - if (includeCanonicalGlobal) { - value += CounterOps::get(_resourceIdGlobal.modeStats[mode].combinedWaitTimeMicros); - } if (value > 0) { if (!timeAcquiring) { if (!section) { - section.reset(new BSONObjBuilder(builder->subobjStart(sectionName))); + section.reset(new BSONObjBuilder(builder->subobjStart(resourceTypeName))); } timeAcquiring.reset( @@ -140,7 +127,6 @@ void LockStats<CounterType>::reset() { for (int mode = 0; mode < LockModesCount; mode++) { _oplogStats.modeStats[mode].reset(); - _resourceIdGlobal.modeStats[mode].reset(); } } diff --git a/src/mongo/db/concurrency/lock_stats.h b/src/mongo/db/concurrency/lock_stats.h index 6d186f55f65..d45b3b38864 100644 --- a/src/mongo/db/concurrency/lock_stats.h +++ b/src/mongo/db/concurrency/lock_stats.h @@ -38,7 +38,8 @@ class BSONObjBuilder; /** - * Operations for manipulating the lock statistics abstracting whether they are atomic or not. + * Abstraction for manipulating the lock statistics, operating on either AtomicWord<long long> or + * int64_t, which the rest of the code in this file refers to as CounterType. */ struct CounterOps { static int64_t get(const int64_t& counter) { @@ -72,7 +73,10 @@ struct CounterOps { /** - * Bundle of locking statistics values. + * Counts numAcquisitions, numWaits and combinedWaitTimeMicros values. + * + * Additionally supports appending or subtracting other LockStatCounters' values to or from its own; + * and can reset its own values to 0. */ template <typename CounterType> struct LockStatCounters { @@ -96,7 +100,7 @@ struct LockStatCounters { CounterOps::set(combinedWaitTimeMicros, 0); } - + // The lock statistics we track. CounterType numAcquisitions; CounterType numWaits; CounterType combinedWaitTimeMicros; @@ -106,6 +110,8 @@ struct LockStatCounters { /** * Templatized lock statistics management class, which can be specialized with atomic integers * for the global stats and with regular integers for the per-locker stats. + * + * CounterType allows the code to operate on both int64_t and AtomicWord<long long> */ template <typename CounterType> class LockStats { @@ -135,10 +141,6 @@ public: return _oplogStats.modeStats[mode]; } - if (resId == resourceIdGlobal) { - return _resourceIdGlobal.modeStats[mode]; - } - return _stats[resId.getType()].modeStats[mode]; } @@ -161,12 +163,6 @@ public: LockStatCountersType& thisStats = _oplogStats.modeStats[mode]; thisStats.append(otherStats); } - - for (int mode = 0; mode < LockModesCount; ++mode) { - const OtherLockStatCountersType& otherStats = other._resourceIdGlobal.modeStats[mode]; - LockStatCountersType& thisStats = _resourceIdGlobal.modeStats[mode]; - thisStats.append(otherStats); - } } template <typename OtherType> @@ -186,12 +182,6 @@ public: LockStatCountersType& thisStats = _oplogStats.modeStats[mode]; thisStats.subtract(otherStats); } - - for (int mode = 0; mode < LockModesCount; ++mode) { - const OtherLockStatCountersType& otherStats = other._resourceIdGlobal.modeStats[mode]; - LockStatCountersType& thisStats = _resourceIdGlobal.modeStats[mode]; - thisStats.subtract(otherStats); - } } void report(BSONObjBuilder* builder) const; @@ -212,15 +202,13 @@ private: void _report(BSONObjBuilder* builder, - const char* sectionName, - const PerModeLockStatCounters& stat, - const bool includeCanonicalGlobal) const; + const char* resourceTypeName, + const PerModeLockStatCounters& stat) const; - // Split the lock stats per resource type. Special-case the oplog and global lock so we can - // collect more detailed stats for it. + // Split the lock stats per resource type. Special-case the oplog so we can collect more + // detailed stats for it. PerModeLockStatCounters _stats[ResourceTypesCount]; - PerModeLockStatCounters _resourceIdGlobal; PerModeLockStatCounters _oplogStats; }; diff --git a/src/mongo/db/concurrency/lock_stats_test.cpp b/src/mongo/db/concurrency/lock_stats_test.cpp index 5e03af8c3e2..f2aa46e3a5c 100644 --- a/src/mongo/db/concurrency/lock_stats_test.cpp +++ b/src/mongo/db/concurrency/lock_stats_test.cpp @@ -147,7 +147,12 @@ TEST_F(LockStatsTest, Subtraction) { } namespace { -void assertAcquisitionStats(ResourceId rid) { +/** + * Locks 'rid' and then checks the global lock stat is reported correctly. Either the global lock is + * reported locked if 'rid' is the global lock resource, or unlocked if 'rid' is not the global lock + * resource. + */ +void assertGlobalAcquisitionStats(ResourceId rid) { resetGlobalLockStats(); SingleThreadedLockStats stats; @@ -177,20 +182,22 @@ void assertAcquisitionStats(ResourceId rid) { } // namespace TEST_F(LockStatsTest, GlobalRetrievableSeparately) { - assertAcquisitionStats(resourceIdGlobal); - assertAcquisitionStats(resourceIdParallelBatchWriterMode); - assertAcquisitionStats(resourceIdReplicationStateTransitionLock); + assertGlobalAcquisitionStats(resourceIdGlobal); + assertGlobalAcquisitionStats(resourceIdParallelBatchWriterMode); + assertGlobalAcquisitionStats(resourceIdReplicationStateTransitionLock); } -TEST_F(LockStatsTest, ServerStatusAggregatesAllGlobal) { +TEST_F(LockStatsTest, ServerStatus) { resetGlobalLockStats(); + // If there are no locks, nothing is reported. SingleThreadedLockStats stats; reportGlobalLockingStats(&stats); BSONObjBuilder builder; stats.report(&builder); ASSERT_EQUALS(0, builder.done().nFields()); + // Take the global, PBWM and RSTL locks in MODE_IX to create acquisition stats for them. LockerImpl locker; locker.lockGlobal(LockMode::MODE_IX); locker.lock(resourceIdParallelBatchWriterMode, LockMode::MODE_IX); @@ -200,12 +207,21 @@ TEST_F(LockStatsTest, ServerStatusAggregatesAllGlobal) { locker.unlock(resourceIdParallelBatchWriterMode); locker.unlockGlobal(); + // Now the MODE_IX lock acquisitions should be reported, separately for each lock type. reportGlobalLockingStats(&stats); BSONObjBuilder builder2; stats.report(&builder2); + auto lockingStats = builder2.done(); ASSERT_EQUALS( - 3, - builder2.done().getObjectField("Global").getObjectField("acquireCount").getIntField("w")); + 1, lockingStats.getObjectField("Global").getObjectField("acquireCount").getIntField("w")); + ASSERT_EQUALS(1, + lockingStats.getObjectField("ParallelBatchWriter") + .getObjectField("acquireCount") + .getIntField("w")); + ASSERT_EQUALS(1, + lockingStats.getObjectField("ReplicationStateTransition") + .getObjectField("acquireCount") + .getIntField("w")); } } // namespace mongo diff --git a/src/mongo/db/stats/fill_locker_info_test.cpp b/src/mongo/db/stats/fill_locker_info_test.cpp index 8a2f671b873..cd143731f56 100644 --- a/src/mongo/db/stats/fill_locker_info_test.cpp +++ b/src/mongo/db/stats/fill_locker_info_test.cpp @@ -41,11 +41,9 @@ namespace { using LockerInfo = Locker::LockerInfo; using OneLock = Locker::OneLock; -const ResourceId kGlobalId(RESOURCE_GLOBAL, ResourceId::SINGLETON_GLOBAL); - TEST(FillLockerInfo, DoesReportWaitingForLockIfWaiting) { LockerInfo info; - info.waitingResource = kGlobalId; + info.waitingResource = resourceIdGlobal; ASSERT_TRUE(info.waitingResource.isValid()); BSONObjBuilder infoBuilder; @@ -72,7 +70,7 @@ TEST(FillLockerInfo, DoesNotReportWaitingForLockIfNotWaiting) { TEST(FillLockerInfo, DoesReportLockStats) { LockerInfo info; SingleThreadedLockStats stats; - stats.recordAcquisition(kGlobalId, MODE_IX); + stats.recordAcquisition(resourceIdGlobal, MODE_IX); info.stats = stats; BSONObjBuilder infoBuilder; @@ -86,7 +84,7 @@ DEATH_TEST(FillLockerInfo, ShouldFailIfLocksAreNotSortedAppropriately, "Invarian LockerInfo info; // The global lock is supposed to come before the database lock. info.locks = {OneLock{ResourceId(RESOURCE_DATABASE, "TestDB"_sd), MODE_X}, - OneLock{kGlobalId, MODE_IX}}; + OneLock{resourceIdGlobal, MODE_IX}}; BSONObjBuilder infoBuilder; fillLockerInfo(info, infoBuilder); @@ -95,15 +93,16 @@ DEATH_TEST(FillLockerInfo, ShouldFailIfLocksAreNotSortedAppropriately, "Invarian TEST(FillLockerInfo, DoesReportLocksHeld) { const ResourceId dbId(RESOURCE_DATABASE, "TestDB"_sd); LockerInfo info; - info.locks = {OneLock{kGlobalId, MODE_IX}, OneLock{dbId, MODE_IX}}; + info.locks = {OneLock{resourceIdGlobal, MODE_IX}, OneLock{dbId, MODE_IX}}; BSONObjBuilder infoBuilder; fillLockerInfo(info, infoBuilder); const BSONObj infoObj = infoBuilder.done(); ASSERT_EQ(infoObj["locks"].type(), BSONType::Object); - ASSERT_EQ(infoObj["locks"][resourceTypeName(kGlobalId.getType())].type(), BSONType::String); - ASSERT_EQ(infoObj["locks"][resourceTypeName(kGlobalId.getType())].String(), "w"); + ASSERT_EQ(infoObj["locks"][resourceTypeName(resourceIdGlobal.getType())].type(), + BSONType::String); + ASSERT_EQ(infoObj["locks"][resourceTypeName(resourceIdGlobal.getType())].String(), "w"); ASSERT_EQ(infoObj["locks"][resourceTypeName(dbId.getType())].type(), BSONType::String); ASSERT_EQ(infoObj["locks"][resourceTypeName(dbId.getType())].String(), "w"); } @@ -112,8 +111,9 @@ TEST(FillLockerInfo, ShouldReportMaxTypeHeldForResourceType) { const ResourceId firstDbId(RESOURCE_DATABASE, "FirstDB"_sd); const ResourceId secondDbId(RESOURCE_DATABASE, "SecondDB"_sd); LockerInfo info; - info.locks = { - OneLock{kGlobalId, MODE_IX}, OneLock{firstDbId, MODE_IX}, OneLock{secondDbId, MODE_X}}; + info.locks = {OneLock{resourceIdGlobal, MODE_IX}, + OneLock{firstDbId, MODE_IX}, + OneLock{secondDbId, MODE_X}}; BSONObjBuilder infoBuilder; fillLockerInfo(info, infoBuilder); @@ -125,8 +125,9 @@ TEST(FillLockerInfo, ShouldReportMaxTypeHeldForResourceType) { "W"); // One is held in IX, one in X, so X should win and be displayed as "W". // Ensure it still works if locks are reported in the opposite order. - info.locks = { - OneLock{kGlobalId, MODE_IX}, OneLock{secondDbId, MODE_X}, OneLock{firstDbId, MODE_IX}}; + info.locks = {OneLock{resourceIdGlobal, MODE_IX}, + OneLock{secondDbId, MODE_X}, + OneLock{firstDbId, MODE_IX}}; ASSERT_EQ(infoObj["locks"].type(), BSONType::Object); ASSERT_EQ(infoObj["locks"][resourceTypeName(firstDbId.getType())].type(), BSONType::String); diff --git a/src/mongo/db/storage/mobile/mobile_recovery_unit.cpp b/src/mongo/db/storage/mobile/mobile_recovery_unit.cpp index 2d194396feb..d8ff95a9e50 100644 --- a/src/mongo/db/storage/mobile/mobile_recovery_unit.cpp +++ b/src/mongo/db/storage/mobile/mobile_recovery_unit.cpp @@ -222,16 +222,14 @@ void MobileRecoveryUnit::_txnOpen(OperationContext* opCtx, bool readOnly) { // Check for correct locking at higher levels if (readOnly) { // Confirm that this reader has taken a shared lock - if (!opCtx->lockState()->isLockHeldForMode( - ResourceId(RESOURCE_GLOBAL, ResourceId::SINGLETON_GLOBAL), MODE_S)) { + if (!opCtx->lockState()->isLockHeldForMode(resourceIdGlobal, MODE_S)) { opCtx->lockState()->dump(); invariant(!"Reading without a shared lock"); } SqliteStatement::execQuery(_session.get(), "BEGIN"); } else { // Single writer allowed at a time, confirm a global write lock has been taken - if (!opCtx->lockState()->isLockHeldForMode( - ResourceId(RESOURCE_GLOBAL, ResourceId::SINGLETON_GLOBAL), MODE_X)) { + if (!opCtx->lockState()->isLockHeldForMode(resourceIdGlobal, MODE_X)) { opCtx->lockState()->dump(); invariant(!"Writing without an exclusive lock"); } |