SERVER-15946 Get rid of GlobalSharedLockAcquirer in favor of taking the global lock directly

8 files changed, 17 insertions, 148 deletions

diff --git a/src/mongo/db/repl/repl_coordinator_external_state.cpp b/src/mongo/db/repl/repl_coordinator_external_state.cpp
index 28392676ee7..4ee3533e272 100644
--- a/src/mongo/db/repl/repl_coordinator_external_state.cpp
+++ b/src/mongo/db/repl/repl_coordinator_external_state.cpp
@@ -36,20 +36,5 @@ namespace repl {
     ReplicationCoordinatorExternalState::ReplicationCoordinatorExternalState() {}
     ReplicationCoordinatorExternalState::~ReplicationCoordinatorExternalState() {}
 
-    ReplicationCoordinatorExternalState::ScopedLocker::ScopedLocker(
-            OperationContext* txn,
-            ReplicationCoordinatorExternalState::GlobalSharedLockAcquirer* locker,
-            const Milliseconds& timeout) : _locker(locker), _gotLock(false) {
-        _gotLock = _locker->try_lock(txn, timeout);
-    }
-
-    ReplicationCoordinatorExternalState::GlobalSharedLockAcquirer::~GlobalSharedLockAcquirer() {}
-
-    ReplicationCoordinatorExternalState::ScopedLocker::~ScopedLocker() {}
-
-    bool ReplicationCoordinatorExternalState::ScopedLocker::gotLock() const {
-        return _gotLock;
-    }
-
 } // namespace repl
 } // namespace mongo
diff --git a/src/mongo/db/repl/repl_coordinator_external_state.h b/src/mongo/db/repl/repl_coordinator_external_state.h
index 03ae82304c9..ee96eeba65c 100644
--- a/src/mongo/db/repl/repl_coordinator_external_state.h
+++ b/src/mongo/db/repl/repl_coordinator_external_state.h
@@ -55,9 +55,6 @@ namespace repl {
         MONGO_DISALLOW_COPYING(ReplicationCoordinatorExternalState);
     public:
 
-        class GlobalSharedLockAcquirer;
-        class ScopedLocker;
-
         ReplicationCoordinatorExternalState();
         virtual ~ReplicationCoordinatorExternalState();
 
@@ -146,12 +143,6 @@ namespace repl {
         virtual void signalApplierToChooseNewSyncSource() = 0;
 
         /**
-         * Returns an instance of GlobalSharedLockAcquirer that can be used to acquire the global
-         * shared lock.
-         */
-        virtual GlobalSharedLockAcquirer* getGlobalSharedLockAcquirer() = 0;
-
-        /**
          * Returns an OperationContext, owned by the caller, that may be used in methods of
          * the same instance that require an OperationContext.
          */
@@ -166,39 +157,5 @@ namespace repl {
         virtual void dropAllTempCollections(OperationContext* txn) = 0;
     };
 
-    /**
-     * Interface that encapsulates acquiring the global shared lock.
-     */
-    class ReplicationCoordinatorExternalState::GlobalSharedLockAcquirer {
-    public:
-
-        virtual ~GlobalSharedLockAcquirer();
-
-        virtual bool try_lock(OperationContext* txn, const Milliseconds& timeout) = 0;
-    };
-
-    /**
-     * Class used to acquire the global shared lock, using a given implementation of
-     * GlobalSharedLockAcquirer.
-     */
-    class ReplicationCoordinatorExternalState::ScopedLocker {
-    public:
-
-        /**
-         * Takes ownership of the passed in GlobalSharedLockAcquirer.
-         */
-        ScopedLocker(OperationContext* txn,
-                     GlobalSharedLockAcquirer* locker,
-                     const Milliseconds& timeout);
-        ~ScopedLocker();
-
-        bool gotLock() const;
-
-    private:
-
-        boost::scoped_ptr<ReplicationCoordinatorExternalState::GlobalSharedLockAcquirer> _locker;
-        bool _gotLock;
-    };
-
 } // namespace repl
 } // namespace mongo
diff --git a/src/mongo/db/repl/repl_coordinator_external_state_impl.cpp b/src/mongo/db/repl/repl_coordinator_external_state_impl.cpp
index fbd2b03d04b..d9aef7887db 100644
--- a/src/mongo/db/repl/repl_coordinator_external_state_impl.cpp
+++ b/src/mongo/db/repl/repl_coordinator_external_state_impl.cpp
@@ -244,34 +244,5 @@ namespace {
         }
     }
 
-namespace {
-    class GlobalSharedLockAcquirerImpl :
-            public ReplicationCoordinatorExternalState::GlobalSharedLockAcquirer {
-    public:
-
-        GlobalSharedLockAcquirerImpl() {};
-        virtual ~GlobalSharedLockAcquirerImpl() {};
-
-        virtual bool try_lock(OperationContext* txn, const Milliseconds& timeout) {
-            try {
-                _rlock.reset(new Lock::GlobalRead(txn->lockState(), timeout.total_milliseconds()));
-            }
-            catch (const DBTryLockTimeoutException&) {
-                return false;
-            }
-            return true;
-        }
-
-    private:
-
-        boost::scoped_ptr<Lock::GlobalRead> _rlock;
-    };
-} // namespace
-
-    ReplicationCoordinatorExternalState::GlobalSharedLockAcquirer*
-            ReplicationCoordinatorExternalStateImpl::getGlobalSharedLockAcquirer() {
-        return new GlobalSharedLockAcquirerImpl();
-    }
-
 } // namespace repl
 } // namespace mongo
diff --git a/src/mongo/db/repl/repl_coordinator_external_state_impl.h b/src/mongo/db/repl/repl_coordinator_external_state_impl.h
index 401ec1fbfb2..d336b30f37b 100644
--- a/src/mongo/db/repl/repl_coordinator_external_state_impl.h
+++ b/src/mongo/db/repl/repl_coordinator_external_state_impl.h
@@ -59,8 +59,6 @@ namespace repl {
         virtual void closeConnections();
         virtual void clearShardingState();
         virtual void signalApplierToChooseNewSyncSource();
-        virtual ReplicationCoordinatorExternalState::GlobalSharedLockAcquirer*
-                getGlobalSharedLockAcquirer();
         virtual OperationContext* createOperationContext(const std::string& threadName);
         virtual void dropAllTempCollections(OperationContext* txn);
 
diff --git a/src/mongo/db/repl/repl_coordinator_external_state_mock.cpp b/src/mongo/db/repl/repl_coordinator_external_state_mock.cpp
index b535515f607..3b555ccd0f9 100644
--- a/src/mongo/db/repl/repl_coordinator_external_state_mock.cpp
+++ b/src/mongo/db/repl/repl_coordinator_external_state_mock.cpp
@@ -138,27 +138,6 @@ namespace repl {
 
     void ReplicationCoordinatorExternalStateMock::signalApplierToChooseNewSyncSource() {}
 
-    void ReplicationCoordinatorExternalStateMock::setCanAcquireGlobalSharedLock(bool canAcquire) {
-        _canAcquireGlobalSharedLock = canAcquire;
-    }
-
-    ReplicationCoordinatorExternalState::GlobalSharedLockAcquirer*
-            ReplicationCoordinatorExternalStateMock::getGlobalSharedLockAcquirer() {
-        return new ReplicationCoordinatorExternalStateMock::GlobalSharedLockAcquirer(
-                _canAcquireGlobalSharedLock);
-    }
-
-    ReplicationCoordinatorExternalStateMock::GlobalSharedLockAcquirer::GlobalSharedLockAcquirer(
-            bool canAcquireLock) : _canAcquireLock(canAcquireLock) {}
-
-    ReplicationCoordinatorExternalStateMock::GlobalSharedLockAcquirer::~GlobalSharedLockAcquirer() {
-    }
-
-    bool ReplicationCoordinatorExternalStateMock::GlobalSharedLockAcquirer::try_lock(
-            OperationContext* txn, const Milliseconds& timeout) {
-        return _canAcquireLock;
-    }
-
     OperationContext* ReplicationCoordinatorExternalStateMock::createOperationContext(
             const std::string& threadName) {
         return new OperationContextReplMock;
diff --git a/src/mongo/db/repl/repl_coordinator_external_state_mock.h b/src/mongo/db/repl/repl_coordinator_external_state_mock.h
index a242d333957..5c1511d16bc 100644
--- a/src/mongo/db/repl/repl_coordinator_external_state_mock.h
+++ b/src/mongo/db/repl/repl_coordinator_external_state_mock.h
@@ -62,8 +62,6 @@ namespace repl {
         virtual void closeConnections();
         virtual void clearShardingState();
         virtual void signalApplierToChooseNewSyncSource();
-        virtual ReplicationCoordinatorExternalState::GlobalSharedLockAcquirer*
-                getGlobalSharedLockAcquirer();
         virtual OperationContext* createOperationContext(const std::string& threadName);
         virtual void dropAllTempCollections(OperationContext* txn);
 
@@ -84,12 +82,6 @@ namespace repl {
         void setClientHostAndPort(const HostAndPort& clientHostAndPort);
 
         /**
-         * Sets the value that will be passed to the constructor of any future
-         * GlobalSharedLockAcuirers created and returned by getGlobalSharedLockAcquirer().
-         */
-        void setCanAcquireGlobalSharedLock(bool canAcquire);
-
-        /**
          * Sets the return value for subsequent calls to loadLastOpTimeApplied.
          */
         void setLastOpTime(const StatusWith<OpTime>& lastApplied);
@@ -120,24 +112,5 @@ namespace repl {
         HostAndPort _clientHostAndPort;
     };
 
-    class ReplicationCoordinatorExternalStateMock::GlobalSharedLockAcquirer :
-            public ReplicationCoordinatorExternalState::GlobalSharedLockAcquirer {
-    public:
-
-        /**
-         * The canAcquireLock argument determines what the return value of calls to try_lock will
-         * be.
-         */
-        GlobalSharedLockAcquirer(bool canAcquireLock);
-        virtual ~GlobalSharedLockAcquirer();
-
-        virtual bool try_lock(OperationContext* txn, const Milliseconds& timeout);
-
-    private:
-
-        const bool _canAcquireLock;
-    };
-
-
 } // namespace repl
 } // namespace mongo
diff --git a/src/mongo/db/repl/repl_coordinator_impl.cpp b/src/mongo/db/repl/repl_coordinator_impl.cpp
index b4813d6dc47..eb7dd095b44 100644
--- a/src/mongo/db/repl/repl_coordinator_impl.cpp
+++ b/src/mongo/db/repl/repl_coordinator_impl.cpp
@@ -36,6 +36,7 @@
 #include <boost/thread.hpp>
 
 #include "mongo/base/status.h"
+#include "mongo/db/concurrency/d_concurrency.h"
 #include "mongo/db/global_optime.h"
 #include "mongo/db/index/index_descriptor.h"
 #include "mongo/db/operation_context_noop.h"
@@ -1007,9 +1008,11 @@ namespace {
         const Date_t stepDownUntil(startTime.millis + stepdownTime.total_milliseconds());
         const Date_t waitUntil(startTime.millis + waitTime.total_milliseconds());
 
-        ReplicationCoordinatorExternalState::ScopedLocker lk(
-                txn, _externalState->getGlobalSharedLockAcquirer(), stepdownTime);
-        if (!lk.gotLock()) {
+        boost::scoped_ptr<Lock::GlobalRead> lk;
+        try {
+            lk.reset(new Lock::GlobalRead(txn->lockState(), stepdownTime.total_milliseconds()));
+        }
+        catch (const DBTryLockTimeoutException&) {
             return Status(ErrorCodes::ExceededTimeLimit,
                           "Could not acquire the global shared lock within the amount of time "
                                   "specified that we should step down for");
diff --git a/src/mongo/db/repl/repl_coordinator_impl_test.cpp b/src/mongo/db/repl/repl_coordinator_impl_test.cpp
index cf6a2208ba1..a45b06fa21f 100644
--- a/src/mongo/db/repl/repl_coordinator_impl_test.cpp
+++ b/src/mongo/db/repl/repl_coordinator_impl_test.cpp
@@ -40,6 +40,7 @@
 #include "mongo/db/repl/handshake_args.h"
 #include "mongo/db/repl/is_master_response.h"
 #include "mongo/db/repl/network_interface_mock.h"
+#include "mongo/db/repl/operation_context_repl_mock.h"
 #include "mongo/db/repl/repl_coordinator.h" // ReplSetReconfigArgs
 #include "mongo/db/repl/repl_coordinator_external_state_mock.h"
 #include "mongo/db/repl/repl_coordinator_impl.h"
@@ -862,7 +863,7 @@ namespace {
     TEST_F(ReplCoordTest, AwaitReplicationStepDown) {
         // Test that a thread blocked in awaitReplication will be woken up and return NotMaster
         // if the node steps down while it is waiting.
-        OperationContextNoop txn;
+        OperationContextReplMock txn;
         assertStartSuccess(
                 BSON("_id" << "mySet" <<
                      "version" << 2 <<
@@ -1031,7 +1032,7 @@ namespace {
     };
 
     TEST_F(StepDownTest, StepDownNotPrimary) {
-        OperationContextNoop txn;
+        OperationContextReplMock txn;
         OpTime optime1(100, 1);
         // All nodes are caught up
         ASSERT_OK(getReplCoord()->setMyLastOptime(&txn, optime1));
@@ -1044,7 +1045,7 @@ namespace {
     }
 
     TEST_F(StepDownTest, StepDownTimeoutAcquiringGlobalLock) {
-        OperationContextNoop txn;
+        OperationContextReplMock txn;
         OpTime optime1(100, 1);
         // All nodes are caught up
         ASSERT_OK(getReplCoord()->setMyLastOptime(&txn, optime1));
@@ -1053,14 +1054,16 @@ namespace {
 
         simulateSuccessfulElection();
 
-        getExternalState()->setCanAcquireGlobalSharedLock(false);
+        // Make sure stepDown cannot grab the global shared lock
+        Lock::GlobalWrite lk(txn.lockState());
+
         Status status = getReplCoord()->stepDown(&txn, false, Milliseconds(0), Milliseconds(1000));
         ASSERT_EQUALS(ErrorCodes::ExceededTimeLimit, status);
         ASSERT_TRUE(getReplCoord()->getCurrentMemberState().primary());
     }
 
     TEST_F(StepDownTest, StepDownNoWaiting) {
-        OperationContextNoop txn;
+        OperationContextReplMock txn;
         OpTime optime1(100, 1);
         // All nodes are caught up
         ASSERT_OK(getReplCoord()->setMyLastOptime(&txn, optime1));
@@ -1111,7 +1114,7 @@ namespace {
                      "version" << 1 <<
                      "members" << BSON_ARRAY(BSON("_id" << 0 << "host" << "test1:1234"))),
                 HostAndPort("test1", 1234));
-        OperationContextNoop txn;
+        OperationContextReplMock txn;
         getReplCoord()->setFollowerMode(MemberState::RS_SECONDARY);
 
         ASSERT_TRUE(getReplCoord()->getCurrentMemberState().primary());
@@ -1196,7 +1199,7 @@ namespace {
     };
 
     TEST_F(StepDownTest, StepDownNotCaughtUp) {
-        OperationContextNoop txn;
+        OperationContextReplMock txn;
         OpTime optime1(100, 1);
         OpTime optime2(100, 2);
         // No secondary is caught up
@@ -1238,7 +1241,7 @@ namespace {
     }
 
     TEST_F(StepDownTest, StepDownCatchUp) {
-        OperationContextNoop txn;
+        OperationContextReplMock txn;
         OpTime optime1(100, 1);
         OpTime optime2(100, 2);
         // No secondary is caught up