diff options
| -rw-r--r-- | jstests/replsets/noop_writes_wait_for_write_concern.js | 221 | ||||
| -rw-r--r-- | src/mongo/db/service_entry_point_mongod.cpp | 22 |
2 files changed, 238 insertions, 5 deletions
diff --git a/jstests/replsets/noop_writes_wait_for_write_concern.js b/jstests/replsets/noop_writes_wait_for_write_concern.js new file mode 100644 index 00000000000..e525d68503b --- /dev/null +++ b/jstests/replsets/noop_writes_wait_for_write_concern.js @@ -0,0 +1,221 @@ +/** + * This file tests that if a user initiates a write that becomes a noop due to being a duplicate + * operation, that we still wait for write concern. This is because we must wait for write concern + * on the write that made this a noop so that we can be sure it doesn't get rolled back if we + * acknowledge it. + */ + +(function() { + "use strict"; + load('jstests/libs/write_concern_util.js'); + + var name = 'noop_writes_wait_for_write_concern'; + var replTest = new ReplSetTest({ + name: name, + nodes: [{}, {rsConfig: {priority: 0}}, {rsConfig: {priority: 0}}], + }); + replTest.startSet(); + replTest.initiate(); + // Stops node 1 so that all w:3 write concerns time out. We have 3 data bearing nodes so that + // 'dropDatabase' can satisfy its implicit writeConcern: majority but still time out from the + // explicit w:3 write concern. + replTest.stop(1); + + var primary = replTest.getPrimary(); + assert.eq(primary, replTest.nodes[0]); + var dbName = 'testDB'; + var db = primary.getDB(dbName); + var collName = 'testColl'; + var coll = db[collName]; + + function dropTestCollection() { + coll.drop(); + assert.eq(0, coll.find().itcount(), "test collection not empty"); + } + + // Each entry in this array contains a command whose noop write concern behavior needs to be + // tested. Entries have the following structure: + // { + // req: <object>, // Command request object that will result in a noop + // // write after the setup function is called. + // + // setupFunc: <function()>, // Function to run to ensure that the request is a + // // noop. + // + // confirmFunc: <function(res)>, // Function to run after the command is run to ensure + // // that it executed properly. Accepts the result of + // // the noop request to validate it. + // } + var commands = []; + + // 'update' where the document to update does not exist. + commands.push({ + req: {update: collName, updates: [{q: {a: 1}, u: {b: 2}}]}, + setupFunc: function() { + assert.writeOK(coll.insert({a: 1})); + assert.writeOK(coll.update({a: 1}, {b: 2})); + }, + confirmFunc: function(res) { + assert.commandWorked(res); + assert.eq(res.n, 0); + assert.eq(res.nModified, 0); + assert.eq(coll.find().itcount(), 1); + assert.eq(coll.count({b: 2}), 1); + } + }); + + // 'update' where the update has already been done. + commands.push({ + req: {update: collName, updates: [{q: {a: 1}, u: {$set: {b: 2}}}]}, + setupFunc: function() { + assert.writeOK(coll.insert({a: 1})); + assert.writeOK(coll.update({a: 1}, {$set: {b: 2}})); + }, + confirmFunc: function(res) { + assert.commandWorked(res); + assert.eq(res.n, 1); + assert.eq(res.nModified, 0); + assert.eq(coll.find().itcount(), 1); + assert.eq(coll.count({a: 1, b: 2}), 1); + } + }); + + commands.push({ + req: {delete: collName, deletes: [{q: {a: 1}, limit: 1}]}, + setupFunc: function() { + assert.writeOK(coll.insert({a: 1})); + assert.writeOK(coll.remove({a: 1})); + }, + confirmFunc: function(res) { + assert.commandWorked(res); + assert.eq(res.n, 0); + assert.eq(coll.count({a: 1}), 0); + } + }); + + commands.push({ + req: {createIndexes: collName, indexes: [{key: {a: 1}, name: "a_1"}]}, + setupFunc: function() { + assert.writeOK(coll.insert({a: 1})); + assert.commandWorked( + db.runCommand({createIndexes: collName, indexes: [{key: {a: 1}, name: "a_1"}]})); + }, + confirmFunc: function(res) { + assert.commandWorked(res); + assert.eq(res.numIndexesBefore, res.numIndexesAfter); + } + }); + + // 'findAndModify' where the document to update does not exist. + commands.push({ + req: {findAndModify: collName, query: {a: 1}, update: {b: 2}}, + setupFunc: function() { + assert.writeOK(coll.insert({a: 1})); + assert.commandWorked( + db.runCommand({findAndModify: collName, query: {a: 1}, update: {b: 2}})); + }, + confirmFunc: function(res) { + assert.commandWorked(res); + assert.eq(res.lastErrorObject.updatedExisting, false); + assert.eq(coll.find().itcount(), 1); + assert.eq(coll.count({b: 2}), 1); + } + }); + + // 'findAndModify' where the update has already been done. + commands.push({ + req: {findAndModify: collName, query: {a: 1}, update: {$set: {b: 2}}}, + setupFunc: function() { + assert.writeOK(coll.insert({a: 1})); + assert.commandWorked( + db.runCommand({findAndModify: collName, query: {a: 1}, update: {$set: {b: 2}}})); + }, + confirmFunc: function(res) { + assert.commandWorked(res); + assert.eq(res.lastErrorObject.updatedExisting, true); + assert.eq(coll.find().itcount(), 1); + assert.eq(coll.count({a: 1, b: 2}), 1); + } + }); + + commands.push({ + req: {dropDatabase: 1}, + setupFunc: function() { + assert.writeOK(coll.insert({a: 1})); + assert.commandWorked(db.runCommand({dropDatabase: 1})); + }, + confirmFunc: function(res) { + assert.commandWorked(res); + } + }); + + commands.push({ + req: {drop: collName}, + setupFunc: function() { + assert.writeOK(coll.insert({a: 1})); + assert.commandWorked(db.runCommand({drop: collName})); + }, + confirmFunc: function(res) { + assert.commandFailedWithCode(res, ErrorCodes.NamespaceNotFound); + } + }); + + commands.push({ + req: {create: collName}, + setupFunc: function() { + assert.commandWorked(db.runCommand({create: collName})); + }, + confirmFunc: function(res) { + assert.commandFailedWithCode(res, ErrorCodes.NamespaceExists); + } + }); + + commands.push({ + req: {insert: collName, documents: [{_id: 1}]}, + setupFunc: function() { + assert.writeOK(coll.insert({_id: 1})); + }, + confirmFunc: function(res) { + assert.commandWorked(res); + assert.eq(res.n, 0); + assert.eq(res.writeErrors[0].code, ErrorCodes.DuplicateKey); + assert.eq(coll.count({_id: 1}), 1); + } + }); + + function testCommandWithWriteConcern(cmd) { + // Provide a small wtimeout that we expect to time out. + cmd.req.writeConcern = {w: 3, wtimeout: 1000}; + jsTest.log("Testing " + tojson(cmd.req)); + + dropTestCollection(); + + cmd.setupFunc(); + + // We run the command on a different connection. If the the command were run on the + // same connection, then the client last op for the noop write would be set by the setup + // operation. By using a fresh connection the client last op begins as null. + // This test explicitly tests that write concern for noop writes works when the + // client last op has not already been set by a duplicate operation. + var shell2 = new Mongo(primary.host); + + // We check the error code of 'res' in the 'confirmFunc'. + var res = shell2.getDB(dbName).runCommand(cmd.req); + + try { + // Tests that the command receives a write concern error. If we don't wait for write + // concern on noop writes then we won't get a write concern error. + assertWriteConcernError(res); + cmd.confirmFunc(res); + } catch (e) { + // Make sure that we print out the response. + printjson(res); + throw e; + } + } + + commands.forEach(function(cmd) { + testCommandWithWriteConcern(cmd); + }); + +})();
\ No newline at end of file diff --git a/src/mongo/db/service_entry_point_mongod.cpp b/src/mongo/db/service_entry_point_mongod.cpp index 67790e417fc..a7cbe6a3a5c 100644 --- a/src/mongo/db/service_entry_point_mongod.cpp +++ b/src/mongo/db/service_entry_point_mongod.cpp @@ -38,6 +38,7 @@ #include "mongo/db/client.h" #include "mongo/db/commands.h" #include "mongo/db/commands/fsync.h" +#include "mongo/db/concurrency/global_lock_acquisition_tracker.h" #include "mongo/db/curop.h" #include "mongo/db/curop_metrics.h" #include "mongo/db/cursor_manager.h" @@ -53,6 +54,7 @@ #include "mongo/db/ops/write_ops_exec.h" #include "mongo/db/query/find.h" #include "mongo/db/read_concern.h" +#include "mongo/db/repl/optime.h" #include "mongo/db/repl/repl_client_info.h" #include "mongo/db/repl/replication_coordinator_global.h" #include "mongo/db/s/operation_sharding_state.h" @@ -307,13 +309,21 @@ StatusWith<repl::ReadConcernArgs> _extractReadConcern(const BSONObj& cmdObj, void _waitForWriteConcernAndAddToCommandResponse(OperationContext* opCtx, const std::string& commandName, + const repl::OpTime& lastOpBeforeRun, BSONObjBuilder* commandResponseBuilder) { + auto lastOpAfterRun = repl::ReplClientInfo::forClient(opCtx->getClient()).getLastOp(); + + // Ensures that if we tried to do a write, we wait for write concern, even if that write was + // a noop. + if ((lastOpAfterRun == lastOpBeforeRun) && + GlobalLockAcquisitionTracker::get(opCtx).getGlobalExclusiveLockTaken()) { + repl::ReplClientInfo::forClient(opCtx->getClient()).setLastOpToSystemLastOpTime(opCtx); + lastOpAfterRun = repl::ReplClientInfo::forClient(opCtx->getClient()).getLastOp(); + } + WriteConcernResult res; auto waitForWCStatus = - waitForWriteConcern(opCtx, - repl::ReplClientInfo::forClient(opCtx->getClient()).getLastOp(), - opCtx->getWriteConcern(), - &res); + waitForWriteConcern(opCtx, lastOpAfterRun, opCtx->getWriteConcern(), &res); Command::appendCommandWCStatus(*commandResponseBuilder, waitForWCStatus, res); // SERVER-22421: This code is to ensure error response backwards compatibility with the @@ -446,13 +456,15 @@ bool runCommandImpl(OperationContext* opCtx, return result; } + auto lastOpBeforeRun = repl::ReplClientInfo::forClient(opCtx->getClient()).getLastOp(); + // Change the write concern while running the command. const auto oldWC = opCtx->getWriteConcern(); ON_BLOCK_EXIT([&] { opCtx->setWriteConcern(oldWC); }); opCtx->setWriteConcern(wcResult.getValue()); ON_BLOCK_EXIT([&] { _waitForWriteConcernAndAddToCommandResponse( - opCtx, command->getName(), &inPlaceReplyBob); + opCtx, command->getName(), lastOpBeforeRun, &inPlaceReplyBob); }); result = command->enhancedRun(opCtx, request, inPlaceReplyBob); |