From cc39f69e7b2d79a3074a546ca33d30a0307bc238 Mon Sep 17 00:00:00 2001 From: Maria van Keulen Date: Fri, 24 Jan 2020 16:41:52 +0000 Subject: 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. --- ...causally_consistent_jscore_txns_passthrough.yml | 1 + ...causally_consistent_jscore_txns_passthrough.yml | 1 + .../resmokeconfig/suites/sharded_jscore_txns.yml | 1 + .../sharded_jscore_txns_sharded_collections.yml | 1 + jstests/core/txns/create_collection.js | 20 ++---- jstests/core/txns/create_collection_parallel.js | 74 ++++++++++++++++++++++ jstests/libs/create_collection_txn_helpers.js | 10 +++ src/mongo/db/catalog/database_impl.cpp | 6 +- 8 files changed, 100 insertions(+), 14 deletions(-) create mode 100644 jstests/core/txns/create_collection_parallel.js create mode 100644 jstests/libs/create_collection_txn_helpers.js 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 { -- cgit v1.2.1