SERVER-45370 Fix oplog hole and test parallel createCollection txns

Testing parallel transactions with createCollections found a hang
scenario caused by reserving oplog slots before transaction commit.
This patch adds this testing and fixes this bug.

8 files changed, 100 insertions, 14 deletions

diff --git a/buildscripts/resmokeconfig/suites/sharded_causally_consistent_jscore_txns_passthrough.yml b/buildscripts/resmokeconfig/suites/sharded_causally_consistent_jscore_txns_passthrough.yml
index 0951c6549a7..aa36c2234a3 100644
--- a/buildscripts/resmokeconfig/suites/sharded_causally_consistent_jscore_txns_passthrough.yml
+++ b/buildscripts/resmokeconfig/suites/sharded_causally_consistent_jscore_txns_passthrough.yml
@@ -32,6 +32,7 @@ selector:
 
   # TODO(SERVER-45368) re-enable once collection creation is permitted in cross-shard transactions.
   - jstests/core/txns/create_collection.js
+  - jstests/core/txns/create_collection_parallel.js
 
   exclude_with_any_tags:
   - assumes_against_mongod_not_mongos
diff --git a/buildscripts/resmokeconfig/suites/sharded_collections_causally_consistent_jscore_txns_passthrough.yml b/buildscripts/resmokeconfig/suites/sharded_collections_causally_consistent_jscore_txns_passthrough.yml
index 41441209150..79cf9b8dde7 100644
--- a/buildscripts/resmokeconfig/suites/sharded_collections_causally_consistent_jscore_txns_passthrough.yml
+++ b/buildscripts/resmokeconfig/suites/sharded_collections_causally_consistent_jscore_txns_passthrough.yml
@@ -31,6 +31,7 @@ selector:
 
   # TODO(SERVER-45368) re-enable once collection creation is permitted in cross-shard transactions.
   - jstests/core/txns/create_collection.js
+  - jstests/core/txns/create_collection_parallel.js
 
   exclude_with_any_tags:
   - assumes_against_mongod_not_mongos
diff --git a/buildscripts/resmokeconfig/suites/sharded_jscore_txns.yml b/buildscripts/resmokeconfig/suites/sharded_jscore_txns.yml
index 6ef0bc600f9..6e4ba3a3368 100644
--- a/buildscripts/resmokeconfig/suites/sharded_jscore_txns.yml
+++ b/buildscripts/resmokeconfig/suites/sharded_jscore_txns.yml
@@ -28,6 +28,7 @@ selector:
 
   # TODO(SERVER-45368) re-enable once collection creation is permitted in cross-shard transactions.
   - jstests/core/txns/create_collection.js
+  - jstests/core/txns/create_collection_parallel.js
 
 
   exclude_with_any_tags:
diff --git a/buildscripts/resmokeconfig/suites/sharded_jscore_txns_sharded_collections.yml b/buildscripts/resmokeconfig/suites/sharded_jscore_txns_sharded_collections.yml
index b1334268870..5cf115e842f 100644
--- a/buildscripts/resmokeconfig/suites/sharded_jscore_txns_sharded_collections.yml
+++ b/buildscripts/resmokeconfig/suites/sharded_jscore_txns_sharded_collections.yml
@@ -27,6 +27,7 @@ selector:
 
   # TODO(SERVER-45368) re-enable once collection creation is permitted in cross-shard transactions.
   - jstests/core/txns/create_collection.js
+  - jstests/core/txns/create_collection_parallel.js
 
   exclude_with_any_tags:
   - assumes_against_mongod_not_mongos
diff --git a/jstests/core/txns/create_collection.js b/jstests/core/txns/create_collection.js
index 1fc6f1089c1..ad1b7f14bc1 100644
--- a/jstests/core/txns/create_collection.js
+++ b/jstests/core/txns/create_collection.js
@@ -9,18 +9,12 @@
 (function() {
 "use strict";
 
+load("jstests/libs/create_collection_txn_helpers.js");
+
 const session = db.getMongo().startSession({causalConsistency: false});
 const collName = "create_new_collection";
 const secondCollName = collName + "_second";
 
-let doTxnOps = function(sessionDB, collName) {
-    assert.commandWorked(sessionDB.runCommand({create: collName}));
-    let sessionColl = sessionDB[collName];
-    assert.commandWorked(sessionColl.insert({a: 1}));
-    assert.eq(sessionColl.find({a: 1}).itcount(), 1);
-    assert.eq(sessionColl.find({}).itcount(), 1);
-};
-
 let sessionDB = session.getDatabase("test");
 let sessionColl = sessionDB[collName];
 let secondSessionColl = sessionDB[secondCollName];
@@ -29,15 +23,15 @@ secondSessionColl.drop({writeConcern: {w: "majority"}});
 
 jsTest.log("Testing createCollection in a transaction");
 session.startTransaction({writeConcern: {w: "majority"}});
-doTxnOps(sessionDB, collName);
+createCollAndCRUDInTxn(sessionDB, collName);
 session.commitTransaction();
 assert.eq(sessionColl.find({}).itcount(), 1);
 
 sessionColl.drop({writeConcern: {w: "majority"}});
 jsTest.log("Testing multiple createCollections in a transaction");
 session.startTransaction({writeConcern: {w: "majority"}});
-doTxnOps(sessionDB, collName);
-doTxnOps(sessionDB, secondCollName);
+createCollAndCRUDInTxn(sessionDB, collName);
+createCollAndCRUDInTxn(sessionDB, secondCollName);
 session.commitTransaction();
 assert.eq(sessionColl.find({}).itcount(), 1);
 assert.eq(secondSessionColl.find({}).itcount(), 1);
@@ -47,7 +41,7 @@ secondSessionColl.drop({writeConcern: {w: "majority"}});
 
 jsTest.log("Testing createCollection in a transaction that aborts");
 session.startTransaction({writeConcern: {w: "majority"}});
-doTxnOps(sessionDB, collName);
+createCollAndCRUDInTxn(sessionDB, collName);
 assert.commandWorked(session.abortTransaction_forTesting());
 
 assert.eq(sessionColl.find({}).itcount(), 0);
@@ -56,7 +50,7 @@ assert.commandWorked(sessionDB.runCommand({create: collName, writeConcern: {w: "
 jsTest.log(
     "Testing createCollection on an existing collection in a transaction that aborts (SHOULD FAIL)");
 session.startTransaction({writeConcern: {w: "majority"}});
-doTxnOps(sessionDB, secondCollName);
+createCollAndCRUDInTxn(sessionDB, secondCollName);
 assert.commandFailedWithCode(sessionDB.runCommand({create: collName}), ErrorCodes.NamespaceExists);
 assert.commandFailedWithCode(session.abortTransaction_forTesting(), ErrorCodes.NoSuchTransaction);
 assert.eq(sessionColl.find({}).itcount(), 0);
diff --git a/jstests/core/txns/create_collection_parallel.js b/jstests/core/txns/create_collection_parallel.js
new file mode 100644
index 00000000000..a3ee0b94087
--- /dev/null
+++ b/jstests/core/txns/create_collection_parallel.js
@@ -0,0 +1,74 @@
+/* Tests parallel transactions with createCollections.
+ *
+ * @tags: [uses_transactions,
+ *         # Creating collections inside multi-document transactions is supported only in v4.4
+ *         # onwards.
+ *         requires_fcv_44]
+ */
+(function() {
+"use strict";
+
+load("jstests/libs/create_collection_txn_helpers.js");
+
+const dbName = "test";
+const collName = "create_new_collection";
+const distinctCollName = collName + "_second";
+const session = db.getMongo().getDB(dbName).getMongo().startSession({causalConsistency: false});
+const secondSession =
+    db.getMongo().getDB(dbName).getMongo().startSession({causalConsistency: false});
+
+let sessionDB = session.getDatabase("test");
+let secondSessionDB = secondSession.getDatabase("test");
+let sessionColl = sessionDB[collName];
+sessionColl.drop({writeConcern: {w: "majority"}});
+let distinctSessionColl = sessionDB[distinctCollName];
+distinctSessionColl.drop({writeConcern: {w: "majority"}});
+
+jsTest.log("Testing duplicate createCollections, second createCollection fails");
+
+session.startTransaction({writeConcern: {w: "majority"}});        // txn 1
+secondSession.startTransaction({writeConcern: {w: "majority"}});  // txn 2
+
+createCollAndCRUDInTxn(sessionDB, collName);
+jsTest.log("Committing transaction 1");
+session.commitTransaction();
+assert.eq(sessionColl.find({}).itcount(), 1);
+
+assert.commandFailedWithCode(secondSessionDB.runCommand({create: collName}),
+                             ErrorCodes.NamespaceExists);
+assert.commandFailedWithCode(secondSession.abortTransaction_forTesting(),
+                             ErrorCodes.NoSuchTransaction);
+
+sessionColl.drop({writeConcern: {w: "majority"}});
+
+jsTest.log(
+    "Testing duplicate createCollections in parallel, both attempt to commit, second to commit fails");
+
+secondSession.startTransaction({writeConcern: {w: "majority"}});  // txn 2
+createCollAndCRUDInTxn(secondSession.getDatabase("test"), collName);
+
+session.startTransaction({writeConcern: {w: "majority"}});  // txn 1
+createCollAndCRUDInTxn(sessionDB, collName);
+
+jsTest.log("Committing transaction 2");
+secondSession.commitTransaction();
+jsTest.log("Committing transaction 1 (SHOULD FAIL)");
+assert.commandFailedWithCode(session.commitTransaction_forTesting(), ErrorCodes.WriteConflict);
+assert.eq(sessionColl.find({}).itcount(), 1);
+
+sessionColl.drop({writeConcern: {w: "majority"}});
+distinctSessionColl.drop({writeConcern: {w: "majority"}});
+
+jsTest.log("Testing distinct createCollections in parallel, both successfully commit.");
+session.startTransaction({writeConcern: {w: "majority"}});  // txn 1
+createCollAndCRUDInTxn(sessionDB, collName);
+
+secondSession.startTransaction({writeConcern: {w: "majority"}});  // txn 2
+createCollAndCRUDInTxn(secondSessionDB, distinctCollName);
+
+session.commitTransaction();
+secondSession.commitTransaction();
+
+secondSession.endSession();
+session.endSession();
+}());
diff --git a/jstests/libs/create_collection_txn_helpers.js b/jstests/libs/create_collection_txn_helpers.js
new file mode 100644
index 00000000000..2465b1db210
--- /dev/null
+++ b/jstests/libs/create_collection_txn_helpers.js
@@ -0,0 +1,10 @@
+/**
+ * Helper function shared by createCollection inside txns tests.
+ */
+const createCollAndCRUDInTxn = function(sessionDB, collName) {
+    assert.commandWorked(sessionDB.runCommand({create: collName}));
+    let sessionColl = sessionDB[collName];
+    assert.commandWorked(sessionColl.insert({a: 1}));
+    assert.eq(sessionColl.find({a: 1}).itcount(), 1);
+    assert.eq(sessionColl.find({}).itcount(), 1);
+};
diff --git a/src/mongo/db/catalog/database_impl.cpp b/src/mongo/db/catalog/database_impl.cpp
index efb175b079b..46f2b931f26 100644
--- a/src/mongo/db/catalog/database_impl.cpp
+++ b/src/mongo/db/catalog/database_impl.cpp
@@ -634,9 +634,13 @@ Collection* DatabaseImpl::createCollection(OperationContext* opCtx,
     // collection create always correct even when other operations are present in the same storage
     // transaction, we reserve an opTime before the collection creation, then pass it to the
     // opObserver.  Reserving the optime automatically sets the storage timestamp.
+    // In order to ensure isolation of multi-document transactions, createCollection should only
+    // reserve oplog slots here if it is run outside of a multi-document transaction. Multi-
+    // document transactions reserve the appropriate oplog slots at commit time.
     OplogSlot createOplogSlot;
     Timestamp createTime;
-    if (canAcceptWrites && supportsDocLocking() && !coordinator->isOplogDisabledFor(opCtx, nss)) {
+    if (canAcceptWrites && supportsDocLocking() && !coordinator->isOplogDisabledFor(opCtx, nss) &&
+        !opCtx->inMultiDocumentTransaction()) {
         createOplogSlot = repl::getNextOpTime(opCtx);
         createTime = createOplogSlot.getTimestamp();
     } else {