SERVER-33272 The DatabaseHolder::close() function no longer requires a global write lock and neither does the dropDatabase command.

7 files changed, 67 insertions, 38 deletions

diff --git a/jstests/noPassthrough/dropdatabase_respect_maxtimems.js b/jstests/noPassthrough/dropdatabase_respect_maxtimems.js
index db93575c993..a6f16456a7a 100644
--- a/jstests/noPassthrough/dropdatabase_respect_maxtimems.js
+++ b/jstests/noPassthrough/dropdatabase_respect_maxtimems.js
@@ -7,17 +7,30 @@ const rst = ReplSetTest({nodes: 1});
 rst.startSet();
 rst.initiate();
 
-const adminDB = rst.getPrimary().getDB("admin");
-const txnDB = rst.getPrimary().getDB("txn");
 const dropDB = rst.getPrimary().getDB("drop");
 
-(function assertColletionDropCanBeInterrupted() {
-    assert.commandWorked(txnDB.foo.insert({}));
+const waitForCommand = function(waitingFor, opFilter) {
+    let opId = -1;
+    assert.soon(function() {
+        print(`Checking for ${waitingFor}`);
+        const curopRes = dropDB.getSiblingDB("admin").currentOp();
+        assert.commandWorked(curopRes);
+        const foundOp = curopRes["inprog"].filter(opFilter);
+
+        if (foundOp.length == 1) {
+            opId = foundOp[0]["opid"];
+        }
+        return (foundOp.length == 1);
+    });
+    return opId;
+};
+
+(function assertCollectionDropCanBeInterrupted() {
     assert.commandWorked(dropDB.bar.insert({}));
-    const session = txnDB.getMongo().startSession({causalConsistency: false});
-    const sessionDB = session.getDatabase("txn");
+    const session = dropDB.getMongo().startSession({causalConsistency: false});
+    const sessionDB = session.getDatabase("drop");
     session.startTransaction();
-    assert.commandWorked(sessionDB.foo.insert({}));
+    assert.commandWorked(sessionDB.bar.insert({}));
     assert.commandFailedWithCode(dropDB.runCommand({dropDatabase: 1, maxTimeMS: 100}),
                                  ErrorCodes.MaxTimeMSExpired);
 
@@ -26,38 +39,47 @@ const dropDB = rst.getPrimary().getDB("drop");
 })();
 
 (function assertDatabaseDropCanBeInterrupted() {
-    assert.commandWorked(txnDB.foo.insert({}));
+    load("jstests/libs/check_log.js");
+
     assert.commandWorked(dropDB.bar.insert({}));
 
     assert.commandWorked(rst.getPrimary().adminCommand(
         {configureFailPoint: "dropDatabaseHangAfterAllCollectionsDrop", mode: "alwaysOn"}));
 
     // This will get blocked by the failpoint when collection drop phase finishes.
-    let dropDatabaseShell = startParallelShell(
-        "assert.commandFailedWithCode(db.getSiblingDB(\"drop\").runCommand({dropDatabase: 1, maxTimeMS: 5000}), ErrorCodes.MaxTimeMSExpired);",
-        rst.getPrimary().port);
+    let dropDatabaseShell = startParallelShell(() => {
+        assert.commandFailedWithCode(
+            db.getSiblingDB("drop").runCommand({dropDatabase: 1, maxTimeMS: 5000}),
+            ErrorCodes.MaxTimeMSExpired);
+    }, rst.getPrimary().port);
 
-    assert.soon(function() {
-        const sessionFilter = {active: true, "command.dropDatabase": 1};
-        const res = adminDB.aggregate([{$currentOp: {}}, {$match: sessionFilter}]);
-        return res.hasNext();
-    }, "Timeout waiting for dropDatabase to start");
+    checkLog.contains(
+        dropDB.getMongo(),
+        "dropDatabase - fail point dropDatabaseHangAfterAllCollectionsDrop enabled. Blocking until fail point is disabled.");
 
-    const session = txnDB.getMongo().startSession({causalConsistency: false});
-    const sessionDB = session.getDatabase("txn");
-    session.startTransaction();
-    assert.commandWorked(sessionDB.foo.insert({}));
+    let sleepCommand = startParallelShell(() => {
+        // Make dropDatabase timeout.
+        assert.commandFailedWithCode(
+            db.getSiblingDB("drop").adminCommand(
+                {sleep: 1, secs: 500, lockTarget: "drop", lock: "ir", $comment: "Lock sleep"}),
+            ErrorCodes.Interrupted);
+    }, rst.getPrimary().port);
+
+    checkLog.contains(dropDB.getMongo(), "test only command sleep invoked");
 
     // dropDatabase now gets unblocked by the failpoint but will immediately
-    // get blocked by acquiring the GlobalWrite lock for dropping the database.
+    // get blocked by acquiring the database lock for dropping the database.
     assert.commandWorked(rst.getPrimary().adminCommand(
         {configureFailPoint: "dropDatabaseHangAfterAllCollectionsDrop", mode: "off"}));
 
-    // This should timeout.
     dropDatabaseShell();
 
-    assert.commandWorked(session.commitTransaction_forTesting());
-    session.endSession();
+    // Interrupt the sleep command.
+    const sleepID = waitForCommand(
+        "sleepCmd", op => (op["ns"] == "admin.$cmd" && op["command"]["$comment"] == "Lock sleep"));
+    assert.commandWorked(dropDB.getSiblingDB("admin").killOp(sleepID));
+
+    sleepCommand();
 })();
 
 rst.stopSet();
diff --git a/jstests/noPassthrough/lock_stats.js b/jstests/noPassthrough/lock_stats.js
index 1274dd326c4..404f5a589ae 100644
--- a/jstests/noPassthrough/lock_stats.js
+++ b/jstests/noPassthrough/lock_stats.js
@@ -6,14 +6,19 @@
 'use strict';
 
 function testBlockTime(blockTimeMillis) {
+    assert.commandWorked(db.getSiblingDB('t1').createCollection('test'));
+
     // Lock the database, and in parallel start an operation that needs the lock, so it blocks.
     assert.commandWorked(db.fsyncLock());
     var startStats = db.serverStatus().locks.Global;
     var startTime = new Date();
     var minBlockedMillis = blockTimeMillis;
+
     // This is just some command that requires a MODE_X global lock that conflicts.
-    var s = startParallelShell(
-        'assert.commandWorked(db.getSiblingDB(\'nonexisting\').dropDatabase());', conn.port);
+    let awaitRename = startParallelShell(() => {
+        assert.commandWorked(
+            db.adminCommand({renameCollection: 't1.test', to: 't2.test', dropTarget: true}));
+    }, conn.port);
 
     // Wait until we see somebody waiting to acquire the lock, defend against unset stats.
     assert.soon((function() {
@@ -32,7 +37,7 @@ function testBlockTime(blockTimeMillis) {
     db.fsyncUnlock();
 
     // Wait for the parallel shell to finish, so its stats will have been recorded.
-    s();
+    awaitRename();
 
     // The fsync command from the shell cannot have possibly been blocked longer than this.
     var maxBlockedMillis = new Date() - startTime;
diff --git a/src/mongo/db/catalog/collection_catalog.cpp b/src/mongo/db/catalog/collection_catalog.cpp
index c32c11405fc..a080463f486 100644
--- a/src/mongo/db/catalog/collection_catalog.cpp
+++ b/src/mongo/db/catalog/collection_catalog.cpp
@@ -212,7 +212,7 @@ void CollectionCatalog::setCollectionNamespace(OperationContext* opCtx,
 }
 
 void CollectionCatalog::onCloseDatabase(OperationContext* opCtx, std::string dbName) {
-    invariant(opCtx->lockState()->isW());
+    invariant(opCtx->lockState()->isDbLockedForMode(dbName, MODE_X));
     auto rid = ResourceId(RESOURCE_DATABASE, dbName);
     removeResource(rid, dbName);
 }
diff --git a/src/mongo/db/catalog/database_holder_impl.cpp b/src/mongo/db/catalog/database_holder_impl.cpp
index 943350b0c89..e528491248d 100644
--- a/src/mongo/db/catalog/database_holder_impl.cpp
+++ b/src/mongo/db/catalog/database_holder_impl.cpp
@@ -198,9 +198,8 @@ void DatabaseHolderImpl::dropDb(OperationContext* opCtx, Database* db) {
 }
 
 void DatabaseHolderImpl::close(OperationContext* opCtx, StringData ns) {
-    invariant(opCtx->lockState()->isW());
-
     const StringData dbName = _todb(ns);
+    invariant(opCtx->lockState()->isDbLockedForMode(dbName, MODE_X));
 
     stdx::lock_guard<SimpleMutex> lk(_m);
 
diff --git a/src/mongo/db/catalog/drop_database.cpp b/src/mongo/db/catalog/drop_database.cpp
index ccaa58bcfc0..65e56764762 100644
--- a/src/mongo/db/catalog/drop_database.cpp
+++ b/src/mongo/db/catalog/drop_database.cpp
@@ -126,7 +126,6 @@ Status dropDatabase(OperationContext* opCtx, const std::string& dbName) {
     repl::OpTime latestDropPendingOpTime;
 
     {
-        Lock::GlobalWrite lk(opCtx);
         AutoGetDb autoDB(opCtx, dbName, MODE_X);
         Database* const db = autoDB.getDb();
         if (!db) {
@@ -299,7 +298,6 @@ Status dropDatabase(OperationContext* opCtx, const std::string& dbName) {
         MONGO_FAIL_POINT_PAUSE_WHILE_SET(dropDatabaseHangAfterAllCollectionsDrop);
     }
 
-    Lock::GlobalWrite lk(opCtx);
     AutoGetDb autoDB(opCtx, dbName, MODE_X);
     auto db = autoDB.getDb();
     if (!db) {
diff --git a/src/mongo/db/catalog/drop_database_test.cpp b/src/mongo/db/catalog/drop_database_test.cpp
index 16fc13ecb44..4cc9320cd97 100644
--- a/src/mongo/db/catalog/drop_database_test.cpp
+++ b/src/mongo/db/catalog/drop_database_test.cpp
@@ -197,7 +197,6 @@ void _createCollection(OperationContext* opCtx, const NamespaceString& nss) {
  * Removes database from catalog, bypassing dropDatabase().
  */
 void _removeDatabaseFromCatalog(OperationContext* opCtx, StringData dbName) {
-    Lock::GlobalWrite lk(opCtx);
     AutoGetDb autoDB(opCtx, dbName, MODE_X);
     auto db = autoDB.getDb();
     // dropDatabase can call awaitReplication more than once, so do not attempt to drop the database
@@ -382,7 +381,7 @@ TEST_F(DropDatabaseTest,
         });
 
     {
-        Lock::GlobalWrite lk(_opCtx.get());
+        Lock::DBLock lk(_opCtx.get(), _nss.db(), MODE_X);
         _testDropDatabase(_opCtx.get(), _opObserver, _nss, true);
     }
 
@@ -410,7 +409,7 @@ TEST_F(DropDatabaseTest,
     _createCollection(_opCtx.get(), dpns);
 
     {
-        Lock::GlobalWrite lk(_opCtx.get());
+        Lock::DBLock lk(_opCtx.get(), _nss.db(), MODE_X);
         ASSERT_OK(dropDatabase(_opCtx.get(), _nss.db().toString()));
     }
 
diff --git a/src/mongo/db/commands/sleep_command.cpp b/src/mongo/db/commands/sleep_command.cpp
index 594422fb77e..d2fd98c3180 100644
--- a/src/mongo/db/commands/sleep_command.cpp
+++ b/src/mongo/db/commands/sleep_command.cpp
@@ -58,7 +58,8 @@ public:
                "If neither 'secs' nor 'millis' is set, command will sleep for 10 seconds. "
                "If both are set, command will sleep for the sum of 'secs' and 'millis.'\n"
                "   w:<bool> (deprecated: use 'lock' instead) if true, takes a write lock.\n"
-               "   lock: r, w, none. If r or w, db will block under a lock. Defaults to r."
+               "   lock: r, ir, w, iw, none. If r or w, db will block under a lock.\n"
+               "   If ir or iw, db will block under an intent lock. Defaults to ir."
                " 'lock' and 'w' may not both be set.\n"
                "   secs:<seconds> Amount of time to sleep, in seconds.\n"
                "   millis:<milliseconds> Amount of time to sleep, in ms.\n"
@@ -159,9 +160,14 @@ public:
                     opCtx->sleepFor(Milliseconds(msRemaining));
                 } else if (lock == "w") {
                     _sleepInLock(opCtx, msRemaining.count(), MODE_X, lockTarget);
-                } else {
-                    uassert(34347, "'lock' must be one of 'r', 'w', 'none'.", lock == "r");
+                } else if (lock == "iw") {
+                    _sleepInLock(opCtx, msRemaining.count(), MODE_IX, lockTarget);
+                } else if (lock == "r") {
                     _sleepInLock(opCtx, msRemaining.count(), MODE_S, lockTarget);
+                } else {
+                    uassert(
+                        34347, "'lock' must be one of 'r', 'ir', 'w', 'iw', 'none'.", lock == "ir");
+                    _sleepInLock(opCtx, msRemaining.count(), MODE_IS, lockTarget);
                 }
             }
         }