SERVER-33004 Add two-phase locking for shared locks for snapshot reads

5 files changed, 111 insertions, 11 deletions

diff --git a/src/mongo/db/concurrency/lock_state.cpp b/src/mongo/db/concurrency/lock_state.cpp
index 49d4031506d..34645189977 100644
--- a/src/mongo/db/concurrency/lock_state.cpp
+++ b/src/mongo/db/concurrency/lock_state.cpp
@@ -125,13 +125,10 @@ AtomicUInt64 idCounter(0);
 // Partitioned global lock statistics, so we don't hit the same bucket
 PartitionedInstanceWideLockStats globalStats;
 
+}  // namespace
 
-/**
- * Whether the particular lock's release should be held until the end of the operation. We
- * delay release of exclusive locks (locks that are for write operations) in order to ensure
- * that the data they protect is committed successfully.
- */
-bool shouldDelayUnlock(ResourceId resId, LockMode mode) {
+template <bool IsForMMAPV1>
+bool LockerImpl<IsForMMAPV1>::_shouldDelayUnlock(ResourceId resId, LockMode mode) const {
     switch (resId.getType()) {
         // The flush lock must not participate in two-phase locking because we need to temporarily
         // yield it while blocked waiting to acquire other locks.
@@ -156,16 +153,13 @@ bool shouldDelayUnlock(ResourceId resId, LockMode mode) {
 
         case MODE_IS:
         case MODE_S:
-            return false;
+            return _sharedLocksShouldTwoPhaseLock;
 
         default:
             MONGO_UNREACHABLE;
     }
 }
 
-}  // namespace
-
-
 template <bool IsForMMAPV1>
 bool LockerImpl<IsForMMAPV1>::isW() const {
     return getLockMode(resourceIdGlobal) == MODE_X;
@@ -455,7 +449,7 @@ void LockerImpl<IsForMMAPV1>::downgrade(ResourceId resId, LockMode newMode) {
 template <bool IsForMMAPV1>
 bool LockerImpl<IsForMMAPV1>::unlock(ResourceId resId) {
     LockRequestsMap::Iterator it = _requests.find(resId);
-    if (inAWriteUnitOfWork() && shouldDelayUnlock(it.key(), (it->mode))) {
+    if (inAWriteUnitOfWork() && _shouldDelayUnlock(it.key(), (it->mode))) {
         _resourcesToUnlockAtEndOfUnitOfWork.push(it.key());
         return false;
     }
diff --git a/src/mongo/db/concurrency/lock_state.h b/src/mongo/db/concurrency/lock_state.h
index 5df6137b035..de93aa65f0c 100644
--- a/src/mongo/db/concurrency/lock_state.h
+++ b/src/mongo/db/concurrency/lock_state.h
@@ -101,6 +101,10 @@ public:
 
     stdx::thread::id getThreadId() const override;
 
+    void setSharedLocksShouldTwoPhaseLock(bool sharedLocksShouldTwoPhaseLock) override {
+        _sharedLocksShouldTwoPhaseLock = sharedLocksShouldTwoPhaseLock;
+    }
+
     virtual LockResult lockGlobal(LockMode mode);
     virtual LockResult lockGlobalBegin(LockMode mode, Date_t deadline) {
         return _lockGlobalBegin(mode, deadline);
@@ -199,6 +203,14 @@ private:
      */
     LockMode _getModeForMMAPV1FlushLock() const;
 
+    /**
+     * Whether the particular lock's release should be held until the end of the operation. We delay
+     * release of exclusive locks (locks that are for write operations) in order to ensure that the
+     * data they protect is committed successfully. Shared locks will also participate in two-phase
+     * locking if '_sharedLocksShouldTwoPhaseLock' is true.
+     */
+    bool _shouldDelayUnlock(ResourceId resId, LockMode mode) const;
+
     // Used to disambiguate different lockers
     const LockerId _id;
 
@@ -232,6 +244,9 @@ private:
     // Track the thread who owns the lock for debugging purposes
     stdx::thread::id _threadId;
 
+    // If true, shared locks will participate in two-phase locking.
+    bool _sharedLocksShouldTwoPhaseLock = false;
+
     //////////////////////////////////////////////////////////////////////////////////////////
     //
     // Methods merged from LockState, which should eventually be removed or changed to methods
diff --git a/src/mongo/db/concurrency/lock_state_test.cpp b/src/mongo/db/concurrency/lock_state_test.cpp
index 9d4a15ef1af..a244378fbfe 100644
--- a/src/mongo/db/concurrency/lock_state_test.cpp
+++ b/src/mongo/db/concurrency/lock_state_test.cpp
@@ -304,6 +304,88 @@ TEST(LockerImpl, CanceledDeadlockUnblocks) {
     ASSERT(locker3.unlockGlobal());
 }
 
+TEST(LockerImpl, MODE_ISLocksUseTwoPhaseLockingWhenSharedLocksShouldTwoPhaseLockIsTrue) {
+    const ResourceId resId(RESOURCE_COLLECTION, "TestDB.collection"_sd);
+
+    DefaultLockerImpl locker;
+    locker.setSharedLocksShouldTwoPhaseLock(true);
+    locker.lockGlobal(MODE_IS);
+
+    ASSERT_EQ(LOCK_OK, locker.lock(resId, MODE_IS));
+    ASSERT_TRUE(locker.isLockHeldForMode(resId, MODE_IS));
+
+    locker.beginWriteUnitOfWork();
+
+    ASSERT_FALSE(locker.unlock(resId));
+    ASSERT_TRUE(locker.isLockHeldForMode(resId, MODE_IS));
+
+    locker.endWriteUnitOfWork();
+
+    ASSERT_TRUE(locker.isLockHeldForMode(resId, MODE_NONE));
+
+    locker.unlockGlobal();
+}
+
+TEST(LockerImpl, MODE_ISLocksDoNotUseTwoPhaseLockingWhenSharedLocksShouldTwoPhaseLockIsFalse) {
+    const ResourceId resId(RESOURCE_COLLECTION, "TestDB.collection"_sd);
+
+    DefaultLockerImpl locker;
+    locker.lockGlobal(MODE_IS);
+
+    ASSERT_EQ(LOCK_OK, locker.lock(resId, MODE_IS));
+    ASSERT_TRUE(locker.isLockHeldForMode(resId, MODE_IS));
+
+    locker.beginWriteUnitOfWork();
+
+    ASSERT_TRUE(locker.unlock(resId));
+    ASSERT_TRUE(locker.isLockHeldForMode(resId, MODE_NONE));
+
+    locker.endWriteUnitOfWork();
+
+    locker.unlockGlobal();
+}
+
+TEST(LockerImpl, MODE_SLocksUseTwoPhaseLockingWhenSharedLocksShouldTwoPhaseLockIsTrue) {
+    const ResourceId resId(RESOURCE_COLLECTION, "TestDB.collection"_sd);
+
+    DefaultLockerImpl locker;
+    locker.setSharedLocksShouldTwoPhaseLock(true);
+    locker.lockGlobal(MODE_IS);
+
+    ASSERT_EQ(LOCK_OK, locker.lock(resId, MODE_S));
+    ASSERT_TRUE(locker.isLockHeldForMode(resId, MODE_S));
+
+    locker.beginWriteUnitOfWork();
+
+    ASSERT_FALSE(locker.unlock(resId));
+    ASSERT_TRUE(locker.isLockHeldForMode(resId, MODE_S));
+
+    locker.endWriteUnitOfWork();
+
+    ASSERT_TRUE(locker.isLockHeldForMode(resId, MODE_NONE));
+
+    locker.unlockGlobal();
+}
+
+TEST(LockerImpl, MODE_SLocksDoNotUseTwoPhaseLockingWhenSharedLocksShouldTwoPhaseLockIsFalse) {
+    const ResourceId resId(RESOURCE_COLLECTION, "TestDB.collection"_sd);
+
+    DefaultLockerImpl locker;
+    locker.lockGlobal(MODE_IS);
+
+    ASSERT_EQ(LOCK_OK, locker.lock(resId, MODE_S));
+    ASSERT_TRUE(locker.isLockHeldForMode(resId, MODE_S));
+
+    locker.beginWriteUnitOfWork();
+
+    ASSERT_TRUE(locker.unlock(resId));
+    ASSERT_TRUE(locker.isLockHeldForMode(resId, MODE_NONE));
+
+    locker.endWriteUnitOfWork();
+
+    locker.unlockGlobal();
+}
+
 namespace {
 /**
  * Helper function to determine if 'lockerInfo' contains a lock with ResourceId 'resourceId' and
diff --git a/src/mongo/db/concurrency/locker.h b/src/mongo/db/concurrency/locker.h
index a8690ce2d37..3e70d000fe8 100644
--- a/src/mongo/db/concurrency/locker.h
+++ b/src/mongo/db/concurrency/locker.h
@@ -90,6 +90,11 @@ public:
     virtual stdx::thread::id getThreadId() const = 0;
 
     /**
+     * Indicate that shared locks should participate in two-phase locking for this Locker instance.
+     */
+    virtual void setSharedLocksShouldTwoPhaseLock(bool sharedLocksShouldTwoPhaseLock) = 0;
+
+    /**
      * This should be the first method invoked for a particular Locker object. It acquires the
      * Global lock in the specified mode and effectively indicates the mode of the operation.
      * This is what the lock modes on the global lock mean:
diff --git a/src/mongo/db/concurrency/locker_noop.h b/src/mongo/db/concurrency/locker_noop.h
index 15ae5a62ba6..208c76fd673 100644
--- a/src/mongo/db/concurrency/locker_noop.h
+++ b/src/mongo/db/concurrency/locker_noop.h
@@ -57,6 +57,10 @@ public:
         invariant(false);
     }
 
+    void setSharedLocksShouldTwoPhaseLock(bool sharedLocksShouldTwoPhaseLock) override {
+        invariant(false);
+    }
+
     virtual LockResult lockGlobal(LockMode mode) {
         invariant(false);
     }