diff options
12 files changed, 197 insertions, 76 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 e83f7635c82..64c70dcae67 100644 --- a/buildscripts/resmokeconfig/suites/sharded_causally_consistent_jscore_txns_passthrough.yml +++ b/buildscripts/resmokeconfig/suites/sharded_causally_consistent_jscore_txns_passthrough.yml @@ -42,6 +42,8 @@ selector: - jstests/core/txns/non_transactional_operations_on_session_with_transaction.js exclude_with_any_tags: + # Transactions are not allowed to operate on capped collections on shard servers. + - requires_capped # Prepare is not a command on mongos. - uses_prepare_transaction executor: 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 9888854a5af..d201ab10a5c 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 @@ -33,9 +33,6 @@ selector: # Uses hangAfterCollectionInserts failpoint not available on mongos. - jstests/core/txns/speculative_snapshot_includes_all_writes.js - # Cannot shard a capped collection. - - jstests/core/txns/ban_tailable_cursor.js - # View tests aren't expected to work when collections are implicitly sharded. - jstests/core/txns/view_reads_in_transaction.js @@ -50,6 +47,8 @@ selector: - assumes_no_implicit_index_creation - assumes_unsharded_collection - cannot_create_unique_index_when_using_hashed_shard_key + # Transactions are not allowed to operate on capped collections on shard servers. + - requires_capped # Prepare is not a command on mongos. - uses_prepare_transaction executor: diff --git a/buildscripts/resmokeconfig/suites/sharded_jscore_txns.yml b/buildscripts/resmokeconfig/suites/sharded_jscore_txns.yml index df13cb042e3..44fd20dd3d0 100644 --- a/buildscripts/resmokeconfig/suites/sharded_jscore_txns.yml +++ b/buildscripts/resmokeconfig/suites/sharded_jscore_txns.yml @@ -38,6 +38,8 @@ selector: - jstests/core/txns/speculative_snapshot_includes_all_writes.js exclude_with_any_tags: + # Transactions are not allowed to operate on capped collections on shard servers. + - requires_capped # Prepare is not a command on mongos. - uses_prepare_transaction executor: diff --git a/buildscripts/resmokeconfig/suites/sharded_jscore_txns_sharded_collections.yml b/buildscripts/resmokeconfig/suites/sharded_jscore_txns_sharded_collections.yml index fe39ad11ea9..b667f103685 100644 --- a/buildscripts/resmokeconfig/suites/sharded_jscore_txns_sharded_collections.yml +++ b/buildscripts/resmokeconfig/suites/sharded_jscore_txns_sharded_collections.yml @@ -33,9 +33,6 @@ selector: # Uses hangAfterCollectionInserts failpoint not available on mongos. - jstests/core/txns/speculative_snapshot_includes_all_writes.js - # Cannot shard a capped collection. - - jstests/core/txns/ban_tailable_cursor.js - # View tests aren't expected to work when collections are implicitly sharded. - jstests/core/txns/view_reads_in_transaction.js @@ -46,6 +43,8 @@ selector: - assumes_no_implicit_index_creation - assumes_unsharded_collection - cannot_create_unique_index_when_using_hashed_shard_key + # Transactions are not allowed to operate on capped collections on shard servers. + - requires_capped # Prepare is not a command on mongos. - uses_prepare_transaction executor: diff --git a/buildscripts/resmokeconfig/suites/sharded_multi_stmt_txn_jscore_passthrough.yml b/buildscripts/resmokeconfig/suites/sharded_multi_stmt_txn_jscore_passthrough.yml index 6c1999c77f9..6d90c023a5d 100644 --- a/buildscripts/resmokeconfig/suites/sharded_multi_stmt_txn_jscore_passthrough.yml +++ b/buildscripts/resmokeconfig/suites/sharded_multi_stmt_txn_jscore_passthrough.yml @@ -12,10 +12,6 @@ selector: - jstests/core/apitest_db.js # serverStatus output doesn't have storageEngine. - jstests/core/apitest_db_profile_level.js # profiling. - jstests/core/apply_ops*.js # applyOps, SERVER-1439. - - jstests/core/capped6.js # captrunc. - - jstests/core/capped_convertToCapped1.js # cloneCollectionAsCapped. - - jstests/core/capped_empty.js # emptycapped. - - jstests/core/capped_update.js # uses godinsert and can't run under replication. - jstests/core/check_shard_index.js # checkShardingIndex. - jstests/core/collection_truncate.js # emptycapped. - jstests/core/collmod_without_uuid.js # applyOps, SERVER-1439 @@ -70,7 +66,6 @@ selector: - jstests/core/bulk_api_ordered.js - jstests/core/bulk_api_unordered.js - jstests/core/bulk_legacy_enforce_gle.js - - jstests/core/capped5.js - jstests/core/commands_with_uuid.js - jstests/core/dbcase.js - jstests/core/dbcase2.js @@ -197,7 +192,6 @@ selector: - jstests/core/bulk_api_ordered.js - jstests/core/bulk_api_unordered.js - jstests/core/bulk_legacy_enforce_gle.js - - jstests/core/cappeda.js - jstests/core/doc_validation.js - jstests/core/doc_validation_options.js - jstests/core/geo_multinest0.js @@ -244,15 +238,6 @@ selector: - jstests/core/tailable_cursor_invalidation.js - jstests/core/tailable_getmore_batch_size.js - # SERVER-34918 The "max" option of a capped collection can be exceeded until the next insert. - # The reason is that we don't update the count of a collection until a transaction commits, - # by which point it is too late to complain that "max" has been exceeded. - - jstests/core/capped_max1.js - - # The "max" option of a capped collection can be temporarily exceeded before a - # txn is committed. - - jstests/core/bulk_insert_capped.js - # Wrong count for top info (WriteLock) - jstests/core/top.js @@ -331,6 +316,8 @@ selector: # "writeConcern is not allowed within a multi-statement transaction" - assumes_write_concern_unchanged - assumes_against_mongod_not_mongos + # Transactions are not allowed to operate on capped collections on shard servers. + - requires_capped executor: config: diff --git a/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml b/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml index f0d71ccc5ab..4487e309cad 100644 --- a/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml +++ b/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml @@ -31,6 +31,9 @@ selector: # Enable when 4.2 becomes last-stable. - jstests/sharding/aggregation_internal_parameters.js - jstests/sharding/agg_error_reports_shard_host_and_port.js + - jstests/sharding/now_variable_replset.js + - jstests/sharding/now_variable_sharding.js + - jstests/sharding/transaction_ops_fail_against_capped_collection_on_shards.js # mongos in 4.0 doesn't like an aggregation explain without stages for optimized away pipelines, # so blacklisting the test until 4.2 becomes last-stable. - jstests/sharding/agg_explain_fmt.js @@ -117,9 +120,6 @@ selector: - jstests/sharding/sharding_statistics_server_status.js # Enable if SERVER-36966 is backported or 4.2 becomes last-stable - jstests/sharding/mr_output_sharded_validation.js - # Enable when 4.2 becomes last-stable - - jstests/sharding/now_variable_replset.js - - jstests/sharding/now_variable_sharding.js executor: config: diff --git a/jstests/core/txns/ban_tailable_cursor.js b/jstests/core/txns/ban_tailable_cursor.js index e228c1b4c60..6751496ac36 100644 --- a/jstests/core/txns/ban_tailable_cursor.js +++ b/jstests/core/txns/ban_tailable_cursor.js @@ -1,5 +1,10 @@ // Check that opening a tailable cursor within a transaction is not allowed. -// @tags: [uses_transactions] +// +// This test cannot run on shards because transactions are not allowed to operate on a capped +// collection on a shard. +// +// @tags: [requires_capped, uses_transactions] + (function() { const dbName = 'test'; const collName = 'tailable-cursor-ban'; diff --git a/jstests/core/txns/multi_statement_transaction_write_error.js b/jstests/core/txns/multi_statement_transaction_write_error.js index d01e8afa351..293d28d93e5 100644 --- a/jstests/core/txns/multi_statement_transaction_write_error.js +++ b/jstests/core/txns/multi_statement_transaction_write_error.js @@ -1,7 +1,7 @@ /** * Test that write errors in transactions are reported in the writeErrors array, except for * TransientTransactionErrors. - * @tags: [uses_transactions] + * @tags: [requires_capped, uses_transactions] */ (function() { "use strict"; @@ -19,9 +19,6 @@ assert.commandWorked(testDB.createCollection(testColl.getName())); assert.commandWorked(testDB.createCollection(cappedCollName, {capped: true, size: 1000})); - assert.commandWorked(testColl.insertOne({_id: 0, x: "string"})); - assert.commandWorked(cappedColl.insertOne({_id: 0, x: "string"})); - // Assert that "cmd" fails with error "code" after "nExpected" operations, or fail with "msg" function runInTxn({cmd, msg, code, nExpected, expectedErrorIndex}) { const session = db.getMongo().startSession(); @@ -151,17 +148,20 @@ } } + // Set up a document so we can get a DuplicateKey error trying to insert it again. + assert.commandWorked(testColl.insert({_id: 5})); exerciseWriteInTxn({ - collNames: [testCollName, cappedCollName], + collNames: [testCollName], cmdName: "insert", goodOp: {}, - badOp: {_id: 0}, + badOp: {_id: 5}, code: ErrorCodes.DuplicateKey }); - // The good op updates "string" to "STRING", preserving doc size, required in capped collection + // Set up a document with a string field so we can update it but fail to increment it. + assert.commandWorked(testColl.insertOne({_id: 0, x: "string"})); exerciseWriteInTxn({ - collNames: [testCollName, cappedCollName], + collNames: [testCollName], cmdName: "update", goodOp: {q: {_id: 0}, u: {$set: {x: "STRING"}}}, badOp: {q: {_id: 0}, u: {$inc: {x: 1}}}, @@ -169,7 +169,7 @@ }); // Give the good delete operation some documents to delete - testColl.insertMany([{}, {}, {}, {}]); + assert.commandWorked(testColl.insertMany([{}, {}, {}, {}])); exerciseWriteInTxn({ collNames: [testCollName], cmdName: "delete", diff --git a/jstests/core/txns/prepare_transaction_fails_on_temp_collections.js b/jstests/core/txns/prepare_transaction_fails_on_temp_collections.js index 3b23266daf9..091665d2509 100644 --- a/jstests/core/txns/prepare_transaction_fails_on_temp_collections.js +++ b/jstests/core/txns/prepare_transaction_fails_on_temp_collections.js @@ -4,7 +4,7 @@ * Transactions should not operate on temporary collections because they are for internal use only * and are deleted on both repl stepup and server startup. * - * @tags: [uses_transactions, uses_prepare_transaction, requires_replication] + * @tags: [uses_transactions, uses_prepare_transaction] */ (function() { diff --git a/jstests/core/txns/transaction_ops_succeed_against_capped_collection.js b/jstests/core/txns/transaction_ops_succeed_against_capped_collection.js new file mode 100644 index 00000000000..6f98daabcab --- /dev/null +++ b/jstests/core/txns/transaction_ops_succeed_against_capped_collection.js @@ -0,0 +1,45 @@ +/** + * Tests that transaction CRUD operations on a capped collection against a non-shard replica set is + * allowed and succeeds. + * + * 'requires_capped' tagged tests are excluded from sharding txn passthrough suites. + * @tags: [requires_capped, uses_transactions] + */ +(function() { + "use strict"; + + const dbName = "test"; + const cappedCollName = "transaction_ops_succeed_against_capped_collection"; + const testDB = db.getSiblingDB(dbName); + const cappedTestColl = testDB.getCollection(cappedCollName); + const testDocument = {"a": "docToTryToUpdateThenDelete"}; + + cappedTestColl.drop({writeConcern: {w: "majority"}}); + + jsTest.log("Creating a capped collection '" + dbName + "." + cappedCollName + "'."); + assert.commandWorked(testDB.createCollection(cappedCollName, {capped: true, size: 500})); + + jsTest.log("Adding a document to the capped collection so that the update op can be tested " + + "in the subsequent transaction attempts"); + assert.commandWorked(cappedTestColl.insert(testDocument)); + + jsTest.log("Setting up a transaction in which to execute transaction ops."); + const session = db.getMongo().startSession(); + const sessionDB = session.getDatabase(dbName); + const sessionCappedColl = sessionDB.getCollection(cappedCollName); + session.startTransaction(); + + jsTest.log("Running an insert op in the transaction against a capped collection that should " + + "succeed"); + assert.commandWorked(sessionCappedColl.insert({x: 55})); + + jsTest.log("Running an update op in the transaction against a capped collection that should " + + "succeed"); + assert.commandWorked( + sessionCappedColl.update(testDocument, {"a": "docIsGettingUpdatedAndSize"})); + + // Deletes do not work against capped collections so we will not test it in a transaction. + + assert.commandWorked(session.commitTransaction_forTesting()); + session.endSession(); +})(); diff --git a/jstests/sharding/transaction_ops_fail_against_capped_collection_on_shards.js b/jstests/sharding/transaction_ops_fail_against_capped_collection_on_shards.js new file mode 100644 index 00000000000..614b7cb7aae --- /dev/null +++ b/jstests/sharding/transaction_ops_fail_against_capped_collection_on_shards.js @@ -0,0 +1,55 @@ +/** + * Tests that transaction CRUD operations against a shard on a capped collection fail. + * + * Capped collection writes take collection X locks and cannot guarantee successful application + * across all replica set members (a write could succeed on a primary and fail on one of the + * secondaries), so they are not allowed in prepared transactions or transactions on shards more + * generally. + * + * @tags: [uses_transactions, uses_prepare_transaction] + */ +(function() { + "use strict"; + + const st = new ShardingTest({shards: 1}); + + const dbName = "test_db"; + const cappedCollName = "test_capped_coll"; + const testDB = st.shard0.getDB('test_db'); + + jsTest.log("Creating a capped collection '" + dbName + "." + cappedCollName + "'."); + assert.commandWorked(testDB.createCollection(cappedCollName, {capped: true, size: 500})); + + jsTest.log("Adding data to the capped collection so that the update op can be tested in " + + "the subsequent transaction attempts"); + const testCappedColl = testDB.getCollection(cappedCollName); + const testDocument = {"a": "docToTryToUpdateThenDelete"}; + assert.writeOK(testCappedColl.insert(testDocument)); + + jsTest.log("Setting up a session in which to execute transaction ops."); + const session = testDB.getMongo().startSession(); + const sessionDB = session.getDatabase(dbName); + const sessionCappedColl = sessionDB.getCollection(cappedCollName); + + jsTest.log("Starting a transaction for an insert op against a capped collection that should " + + "fail"); + session.startTransaction(); + assert.commandFailedWithCode(sessionCappedColl.insert({"x": 55}), + ErrorCodes.OperationNotSupportedInTransaction); + assert.commandFailedWithCode(session.abortTransaction_forTesting(), + ErrorCodes.NoSuchTransaction); + + jsTest.log("Starting a transaction for an update op against a capped collection that should " + + "fail"); + session.startTransaction(); + assert.commandFailedWithCode(sessionCappedColl.update(testDocument, {"a": 1000}), + ErrorCodes.OperationNotSupportedInTransaction); + assert.commandFailedWithCode(session.abortTransaction_forTesting(), + ErrorCodes.NoSuchTransaction); + + // Deletes do not work against capped collections so we will not test it in a transaction. + + session.endSession(); + + st.stop(); +})(); diff --git a/src/mongo/db/ops/write_ops_exec.cpp b/src/mongo/db/ops/write_ops_exec.cpp index cc6d2fcd041..a39b60aa903 100644 --- a/src/mongo/db/ops/write_ops_exec.cpp +++ b/src/mongo/db/ops/write_ops_exec.cpp @@ -321,6 +321,28 @@ void insertDocuments(OperationContext* opCtx, } /** + * Returns a OperationNotSupportedInTransaction error Status if we are in a transaction and + * operating on a capped collection on a shard. + * + * The behavior of an operation against a capped collection may differ across replica set members, + * where it can succeed on one member and fail on another, crashing the failing member. Prepared + * transactions are not allowed to fail, so capped collections will not be allowed on shards. + * Furthermore, capped collections only allow one operation at a time because they enforce + * sequential insertion order with a MODE_X collection lock, which we cannot hold in transactions. + */ +Status checkIfTransactionOnCappedCollOnShard(OperationContext* opCtx, Collection* collection) { + auto txnParticipant = TransactionParticipant::get(opCtx); + if (txnParticipant && txnParticipant.inMultiDocumentTransaction() && collection->isCapped() && + serverGlobalParams.clusterRole == ClusterRole::ShardServer) { + return {ErrorCodes::OperationNotSupportedInTransaction, + str::stream() << "Collection '" << collection->ns() + << "' is a capped collection. Transactions are not allowed on capped " + "collections on shards."}; + } + return Status::OK(); +} + +/** * Returns true if caller should try to insert more documents. Does nothing else if batch is empty. */ bool insertBatchAndHandleErrors(OperationContext* opCtx, @@ -334,26 +356,25 @@ bool insertBatchAndHandleErrors(OperationContext* opCtx, auto& curOp = *CurOp::get(opCtx); + CurOpFailpointHelpers::waitWhileFailPointEnabled( + &hangDuringBatchInsert, + opCtx, + "hangDuringBatchInsert", + [&wholeOp]() { + log() << "batch insert - hangDuringBatchInsert fail point enabled for namespace " + << wholeOp.getNamespace() << ". Blocking " + "until fail point is disabled."; + }, + true, // Check for interrupt periodically. + wholeOp.getNamespace()); + + if (MONGO_FAIL_POINT(failAllInserts)) { + uasserted(ErrorCodes::InternalError, "failAllInserts failpoint active!"); + } + boost::optional<AutoGetCollection> collection; auto acquireCollection = [&] { while (true) { - CurOpFailpointHelpers::waitWhileFailPointEnabled( - &hangDuringBatchInsert, - opCtx, - "hangDuringBatchInsert", - [&wholeOp]() { - log() - << "batch insert - hangDuringBatchInsert fail point enabled for namespace " - << wholeOp.getNamespace() << ". Blocking " - "until fail point is disabled."; - }, - true, // Check for interrupt periodically. - wholeOp.getNamespace()); - - if (MONGO_FAIL_POINT(failAllInserts)) { - uasserted(ErrorCodes::InternalError, "failAllInserts failpoint active!"); - } - collection.emplace( opCtx, wholeOp.getNamespace(), @@ -410,6 +431,9 @@ bool insertBatchAndHandleErrors(OperationContext* opCtx, try { if (!collection) acquireCollection(); + // Transactions are not allowed to operate on capped collections on shards. + uassertStatusOK( + checkIfTransactionOnCappedCollOnShard(opCtx, collection->getCollection())); lastOpFixer->startingOp(); insertDocuments(opCtx, collection->getCollection(), it, it + 1, fromMigrate); lastOpFixer->finishedOpSuccessfully(); @@ -565,29 +589,28 @@ static SingleWriteResult performSingleUpdateOp(OperationContext* opCtx, ParsedUpdate parsedUpdate(opCtx, &updateRequest); uassertStatusOK(parsedUpdate.parseRequest()); + CurOpFailpointHelpers::waitWhileFailPointEnabled( + &hangDuringBatchUpdate, + opCtx, + "hangDuringBatchUpdate", + [&ns]() { + log() << "batch update - hangDuringBatchUpdate fail point enabled for nss " << ns + << ". Blocking until " + "fail point is disabled."; + }, + false /*checkForInterrupt*/, + ns); + + if (MONGO_FAIL_POINT(failAllUpdates)) { + uasserted(ErrorCodes::InternalError, "failAllUpdates failpoint active!"); + } + boost::optional<AutoGetCollection> collection; while (true) { - const auto checkForInterrupt = false; - CurOpFailpointHelpers::waitWhileFailPointEnabled( - &hangDuringBatchUpdate, - opCtx, - "hangDuringBatchUpdate", - [&ns]() { - log() << "batch update - hangDuringBatchUpdate fail point enabled for nss " << ns - << ". Blocking until " - "fail point is disabled."; - }, - checkForInterrupt, - ns); - - if (MONGO_FAIL_POINT(failAllUpdates)) { - uasserted(ErrorCodes::InternalError, "failAllUpdates failpoint active!"); - } + collection.emplace(opCtx, ns, MODE_IX, fixLockModeForSystemDotViewsChanges(ns, MODE_IX)); - collection.emplace(opCtx, - ns, - MODE_IX, // DB is always IX, even if collection is X. - fixLockModeForSystemDotViewsChanges(ns, MODE_IX)); + // If this is an upsert, which is an insert, we must have a collection. + // An update on a non-existant collection is okay and handled later. if (collection->getCollection() || !updateRequest.isUpsert()) break; @@ -595,6 +618,11 @@ static SingleWriteResult performSingleUpdateOp(OperationContext* opCtx, makeCollection(opCtx, ns); } + if (auto coll = collection->getCollection()) { + // Transactions are not allowed to operate on capped collections on shards. + uassertStatusOK(checkIfTransactionOnCappedCollOnShard(opCtx, coll)); + } + CurOpFailpointHelpers::waitWhileFailPointEnabled( &hangWithLockDuringBatchUpdate, opCtx, "hangWithLockDuringBatchUpdate"); @@ -838,10 +866,9 @@ static SingleWriteResult performSingleDeleteOp(OperationContext* opCtx, uasserted(ErrorCodes::InternalError, "failAllRemoves failpoint active!"); } - AutoGetCollection collection(opCtx, - ns, - MODE_IX, // DB is always IX, even if collection is X. - fixLockModeForSystemDotViewsChanges(ns, MODE_IX)); + AutoGetCollection collection( + opCtx, ns, MODE_IX, fixLockModeForSystemDotViewsChanges(ns, MODE_IX)); + if (collection.getDb()) { curOp.raiseDbProfileLevel(collection.getDb()->getProfilingLevel()); } |