diff options
-rw-r--r-- | etc/backports_required_for_multiversion_tests.yml | 4 | ||||
-rw-r--r-- | jstests/sharding/write_commands_read_concern_validation.js | 180 | ||||
-rw-r--r-- | src/mongo/s/commands/cluster_write_cmd.cpp | 5 |
3 files changed, 184 insertions, 5 deletions
diff --git a/etc/backports_required_for_multiversion_tests.yml b/etc/backports_required_for_multiversion_tests.yml index e1af6eab413..f6a31c71d8d 100644 --- a/etc/backports_required_for_multiversion_tests.yml +++ b/etc/backports_required_for_multiversion_tests.yml @@ -106,6 +106,8 @@ last-continuous: test_file: jstests/sharding/resharding_temp_ns_routing_info_unsharded.js - ticket: SERVER-68728 test_file: jstests/sharding/move_chunk_interrupt_postimage.js + - ticket: SERVER-58176 + test_file: jstests/sharding/write_commands_read_concern_validation.js # Tests that should only be excluded from particular suites should be listed under that suite. suites: @@ -372,6 +374,8 @@ last-lts: test_file: jstests/sharding/resharding_temp_ns_routing_info_unsharded.js - ticket: SERVER-68728 test_file: jstests/sharding/move_chunk_interrupt_postimage.js + - ticket: SERVER-58176 + test_file: jstests/sharding/write_commands_read_concern_validation.js # Tests that should only be excluded from particular suites should be listed under that suite. suites: diff --git a/jstests/sharding/write_commands_read_concern_validation.js b/jstests/sharding/write_commands_read_concern_validation.js new file mode 100644 index 00000000000..1c314ed29e5 --- /dev/null +++ b/jstests/sharding/write_commands_read_concern_validation.js @@ -0,0 +1,180 @@ +/** + * Tests that mongod and mongos + * - Do not throw an error if a client specifies readConcern for an insert, update, delete, and + * findAndModify command running as the first command in a transaction. + * - Throw InvalidOptions if a client specifies readConcern in all other cases. + * @tags: [requires_fcv_51, uses_transactions, uses_multi_shard_transaction] + */ +(function() { +'use strict'; + +const st = new ShardingTest({ + mongos: 1, + shards: 1, +}); +const shard0Primary = st.rs0.getPrimary(); + +const kDbName = "testDb"; +assert.commandWorked(st.s.adminCommand({enableSharding: kDbName})); + +function testWriteCommandOutsideSessionAndTransaction( + conn, cmdObj, readConcern, shouldBypassCheck) { + const cmdObjWithReadConcern = Object.assign({}, cmdObj, { + readConcern: readConcern, + }); + const res = conn.getDB(kDbName).runCommand(cmdObjWithReadConcern); + if (shouldBypassCheck) { + assert.commandWorked(res); + } else { + assert.commandFailedWithCode(res, ErrorCodes.InvalidOptions); + } +} + +function testWriteCommandOutsideTransaction(conn, cmdObj, readConcern, shouldBypassCheck) { + const lsid = {id: UUID()}; + const cmdObjWithReadConcern = Object.assign({}, cmdObj, {readConcern: readConcern, lsid: lsid}); + const res = conn.getDB(kDbName).runCommand(cmdObjWithReadConcern); + if (shouldBypassCheck) { + assert.commandWorked(res); + } else { + assert.commandFailedWithCode(res, ErrorCodes.InvalidOptions); + } +} + +function testWriteCommandInsideTransactionFirstCommand(conn, cmdObj, readConcern) { + const lsid = {id: UUID()}; + const txnNumber = NumberLong(1); + const cmdObjWithReadConcern = Object.assign({}, cmdObj, { + readConcern: readConcern, + lsid: lsid, + txnNumber: txnNumber, + startTransaction: true, + autocommit: false + }); + + assert.commandWorked(conn.getDB(kDbName).runCommand(cmdObjWithReadConcern)); + assert.commandWorked(conn.getDB(kDbName).adminCommand({ + commitTransaction: 1, + lsid: lsid, + txnNumber: txnNumber, + autocommit: false, + writeConcern: {w: "majority"} + })); +} + +function testWriteCommandInsideTransactionNotFirstCommand(conn, cmdObj, readConcern, kCollName) { + const lsid = {id: UUID()}; + const txnNumber = NumberLong(1); + + assert.commandWorked(conn.getDB(kDbName).runCommand({ + findAndModify: kCollName, + query: {x: -10}, + update: {$set: {z: -10}}, + lsid: lsid, + txnNumber: txnNumber, + startTransaction: true, + autocommit: false + })); + const cmdObjWithReadConcern = Object.assign( + {}, + cmdObj, + {readConcern: readConcern, lsid: lsid, txnNumber: txnNumber, autocommit: false}); + const res = conn.getDB(kDbName).runCommand(cmdObjWithReadConcern); + assert.commandFailedWithCode(res, ErrorCodes.InvalidOptions); + assert.commandWorked(conn.getDB(kDbName).adminCommand({ + abortTransaction: 1, + lsid: lsid, + txnNumber: txnNumber, + autocommit: false, + writeConcern: {w: "majority"} + })); +} + +function runTest(conn, cmdObj, mongosConn, kCollName) { + const defaultReadConcerns = [ + {}, + {"level": "local"}, + {"level": "majority"}, + {"level": "available"}, + ]; + + defaultReadConcerns.forEach(defaultReadConcern => { + if ("level" in defaultReadConcern) { + assert.commandWorked(mongosConn.adminCommand({ + setDefaultRWConcern: 1, + defaultReadConcern: defaultReadConcern, + })); + jsTest.log("Testing defaultReadConcern " + tojson(defaultReadConcern)); + } else { + jsTest.log("Testing without specifying defaultReadConcern"); + } + + const testCases = [ + {supportedInTransaction: true, readConcern: {level: "snapshot"}}, + {supportedInTransaction: true, readConcern: {level: "majority"}}, + {supportedInTransaction: false, readConcern: {level: "linearizable"}}, + {supportedInTransaction: false, readConcern: {level: "available"}}, + // "local" is the default readConcern so it is exempt from the check. + {supportedInTransaction: true, readConcern: {level: "local"}, shouldBypassCheck: true} + ]; + testCases.forEach(testCase => { + jsTest.log("Testing readConcern " + tojson(testCase.readConcern)); + testWriteCommandOutsideSessionAndTransaction( + conn, cmdObj, testCase.readConcern, testCase.shouldBypassCheck); + testWriteCommandOutsideTransaction( + conn, cmdObj, testCase.readConcern, testCase.shouldBypassCheck); + if (testCase.supportedInTransaction) { + testWriteCommandInsideTransactionFirstCommand(conn, cmdObj, testCase.readConcern); + testWriteCommandInsideTransactionNotFirstCommand( + conn, cmdObj, testCase.readConcern, kCollName); + } + }); + }); +} + +function runTests(conn, mongosConn, kCollName) { + const kNs = kDbName + "." + kCollName; + + // Do an insert to force a refresh so the first transaction doesn't fail due to StaleConfig. + assert.commandWorked(st.s.getCollection(kNs).insert({x: 0})); + + jsTest.log("Running insert"); + const insertCmdObj = { + insert: kCollName, + documents: [{x: -10}], + }; + runTest(conn, insertCmdObj, mongosConn, kCollName); + + jsTest.log("Running update"); + const updateCmdObj = { + update: kCollName, + updates: [{q: {x: -10}, u: {$set: {y: -10}}}], + }; + runTest(conn, updateCmdObj, mongosConn, kCollName); + + jsTest.log("Running delete"); + const deleteCmdObj = { + delete: kCollName, + deletes: [{q: {x: -10}, limit: 1}], + }; + runTest(conn, deleteCmdObj, mongosConn, kCollName); + + jsTest.log("Running findAndModify"); + const findAndModifyCmdObj = { + findAndModify: kCollName, + query: {x: -10}, + update: {$set: {z: -10}}, + }; + runTest(conn, findAndModifyCmdObj, mongosConn, kCollName); +} + +jsTest.log("Running tests against mongod"); +const kCollName0 = "testColl0"; +runTests(shard0Primary, st.s, kCollName0); + +jsTest.log("Running tests against mongos"); +const kCollName1 = "testColl1"; +runTests(st.s, st.s, kCollName1); + +st.stop(); +})(); diff --git a/src/mongo/s/commands/cluster_write_cmd.cpp b/src/mongo/s/commands/cluster_write_cmd.cpp index 189a7b273c3..fc668b87ad7 100644 --- a/src/mongo/s/commands/cluster_write_cmd.cpp +++ b/src/mongo/s/commands/cluster_write_cmd.cpp @@ -613,11 +613,6 @@ private: return true; } - ReadConcernSupportResult supportsReadConcern(repl::ReadConcernLevel level, - bool isImplicitDefault) const final { - return ReadConcernSupportResult::allSupportedAndDefaultPermitted(); - } - void doCheckAuthorization(OperationContext* opCtx) const final { try { doCheckAuthorizationHook(AuthorizationSession::get(opCtx->getClient())); |