diff options
author | Judah Schvimer <judah@mongodb.com> | 2017-07-26 11:43:32 -0400 |
---|---|---|
committer | Judah Schvimer <judah@mongodb.com> | 2017-07-26 11:43:32 -0400 |
commit | 50cbe6d35a3b61ce674eb0d8fae173f70f1dddd5 (patch) | |
tree | 3ed5b2402f96838b1d92e5881e6f9decfd8478e9 | |
parent | ee7376eec1c60210e5ec6301bfb65f415d54cc9d (diff) | |
download | mongo-50cbe6d35a3b61ce674eb0d8fae173f70f1dddd5.tar.gz |
SERVER-27067 Remove unnecessary references to setLastOpToSystemLastOpTime
-rw-r--r-- | jstests/replsets/noop_writes_wait_for_write_concern.js | 14 | ||||
-rw-r--r-- | src/mongo/db/commands/apply_ops_cmd.cpp | 23 | ||||
-rw-r--r-- | src/mongo/db/commands/create_indexes.cpp | 8 | ||||
-rw-r--r-- | src/mongo/db/commands/find_and_modify.cpp | 19 | ||||
-rw-r--r-- | src/mongo/db/ops/delete.cpp | 8 | ||||
-rw-r--r-- | src/mongo/db/ops/update.cpp | 17 |
6 files changed, 23 insertions, 66 deletions
diff --git a/jstests/replsets/noop_writes_wait_for_write_concern.js b/jstests/replsets/noop_writes_wait_for_write_concern.js index e525d68503b..61ffc518df4 100644 --- a/jstests/replsets/noop_writes_wait_for_write_concern.js +++ b/jstests/replsets/noop_writes_wait_for_write_concern.js @@ -48,6 +48,20 @@ // } var commands = []; + commands.push({ + req: {applyOps: [{op: "i", ns: coll.getFullName(), o: {_id: 1}}]}, + setupFunc: function() { + assert.writeOK(coll.insert({_id: 1})); + }, + confirmFunc: function(res) { + assert.commandWorked(res); + assert.eq(res.applied, 1); + assert.eq(res.results[0], true); + assert.eq(coll.find().itcount(), 1); + assert.eq(coll.count({_id: 1}), 1); + } + }); + // 'update' where the document to update does not exist. commands.push({ req: {update: collName, updates: [{q: {a: 1}, u: {b: 2}}]}, diff --git a/src/mongo/db/commands/apply_ops_cmd.cpp b/src/mongo/db/commands/apply_ops_cmd.cpp index 3c09e1110aa..504abd19079 100644 --- a/src/mongo/db/commands/apply_ops_cmd.cpp +++ b/src/mongo/db/commands/apply_ops_cmd.cpp @@ -287,22 +287,17 @@ public: } } - auto client = opCtx->getClient(); - auto lastOpAtOperationStart = repl::ReplClientInfo::forClient(client).getLastOp(); - ScopeGuard lastOpSetterGuard = - MakeObjGuard(repl::ReplClientInfo::forClient(client), - &repl::ReplClientInfo::setLastOpToSystemLastOpTime, - opCtx); - + // TODO (SERVER-30217): When a write concern is provided to the applyOps command, we + // normally wait on the OpTime of whichever operation successfully completed last. This is + // erroneous, however, if the last operation in the array happens to be a write no-op and + // thus isn’t assigned an OpTime. Let the second to last operation in the applyOps be write + // A, the last operation in applyOps be write B. Let B do a no-op write and let the + // operation that caused B to be a no-op be C. If C has an OpTime after A but before B, + // then we won’t wait for C to be replicated and it could be rolled back, even though B + // was acknowledged. To fix this, we should wait for replication of the node’s last applied + // OpTime if the last write operation was a no-op write. auto applyOpsStatus = appendCommandStatus(result, applyOps(opCtx, dbname, cmdObj, &result)); - if (repl::ReplClientInfo::forClient(client).getLastOp() != lastOpAtOperationStart) { - // If this operation has already generated a new lastOp, don't bother setting it - // here. No-op applyOps will not generate a new lastOp, so we still need the guard to - // fire in that case. - lastOpSetterGuard.Dismiss(); - } - return applyOpsStatus; } diff --git a/src/mongo/db/commands/create_indexes.cpp b/src/mongo/db/commands/create_indexes.cpp index 230ca733659..16b2724f3ba 100644 --- a/src/mongo/db/commands/create_indexes.cpp +++ b/src/mongo/db/commands/create_indexes.cpp @@ -291,12 +291,6 @@ public: const int numIndexesBefore = collection->getIndexCatalog()->numIndexesTotal(opCtx); result.append("numIndexesBefore", numIndexesBefore); - auto client = opCtx->getClient(); - ScopeGuard lastOpSetterGuard = - MakeObjGuard(repl::ReplClientInfo::forClient(client), - &repl::ReplClientInfo::setLastOpToSystemLastOpTime, - opCtx); - MultiIndexBlock indexer(opCtx, collection); indexer.allowBackgroundBuilding(); indexer.allowInterruption(); @@ -401,8 +395,6 @@ public: result.append("numIndexesAfter", collection->getIndexCatalog()->numIndexesTotal(opCtx)); - lastOpSetterGuard.Dismiss(); - return true; } diff --git a/src/mongo/db/commands/find_and_modify.cpp b/src/mongo/db/commands/find_and_modify.cpp index 8014c67ecd0..c11466cff22 100644 --- a/src/mongo/db/commands/find_and_modify.cpp +++ b/src/mongo/db/commands/find_and_modify.cpp @@ -360,18 +360,6 @@ public: if (shouldBypassDocumentValidationForCommand(cmdObj)) maybeDisableValidation.emplace(opCtx); - auto client = opCtx->getClient(); - auto lastOpAtOperationStart = repl::ReplClientInfo::forClient(client).getLastOp(); - ScopeGuard lastOpSetterGuard = - MakeObjGuard(repl::ReplClientInfo::forClient(client), - &repl::ReplClientInfo::setLastOpToSystemLastOpTime, - opCtx); - - // If this is the local database, don't set last op. - if (dbName == "local") { - lastOpSetterGuard.Dismiss(); - } - auto curOp = CurOp::get(opCtx); OpDebug* opDebug = &curOp->debug(); @@ -577,13 +565,6 @@ public: return false; } - if (repl::ReplClientInfo::forClient(client).getLastOp() != lastOpAtOperationStart) { - // If this operation has already generated a new lastOp, don't bother setting it here. - // No-op updates will not generate a new lastOp, so we still need the guard to fire in - // that case. - lastOpSetterGuard.Dismiss(); - } - return true; } diff --git a/src/mongo/db/ops/delete.cpp b/src/mongo/db/ops/delete.cpp index 1ac92749e3f..d0c1e3e5fce 100644 --- a/src/mongo/db/ops/delete.cpp +++ b/src/mongo/db/ops/delete.cpp @@ -55,19 +55,11 @@ long long deleteObjects(OperationContext* opCtx, ParsedDelete parsedDelete(opCtx, &request); uassertStatusOK(parsedDelete.parseRequest()); - auto client = opCtx->getClient(); - auto lastOpAtOperationStart = repl::ReplClientInfo::forClient(client).getLastOp(); - auto exec = uassertStatusOK( getExecutorDelete(opCtx, &CurOp::get(opCtx)->debug(), collection, &parsedDelete)); uassertStatusOK(exec->executePlan()); - // No-ops need to reset lastOp in the client, for write concern. - if (repl::ReplClientInfo::forClient(client).getLastOp() == lastOpAtOperationStart) { - repl::ReplClientInfo::forClient(client).setLastOpToSystemLastOpTime(opCtx); - } - return DeleteStage::getNumDeleted(*exec); } diff --git a/src/mongo/db/ops/update.cpp b/src/mongo/db/ops/update.cpp index 575df5d35df..9c661c03925 100644 --- a/src/mongo/db/ops/update.cpp +++ b/src/mongo/db/ops/update.cpp @@ -63,20 +63,9 @@ UpdateResult update(OperationContext* opCtx, Database* db, const UpdateRequest& // Explain should never use this helper. invariant(!request.isExplain()); - auto client = opCtx->getClient(); - auto lastOpAtOperationStart = repl::ReplClientInfo::forClient(client).getLastOp(); - ScopeGuard lastOpSetterGuard = MakeObjGuard(repl::ReplClientInfo::forClient(client), - &repl::ReplClientInfo::setLastOpToSystemLastOpTime, - opCtx); - const NamespaceString& nsString = request.getNamespaceString(); Collection* collection = db->getCollection(opCtx, nsString); - // If this is the local database, don't set last op. - if (db->name() == "local") { - lastOpSetterGuard.Dismiss(); - } - // The update stage does not create its own collection. As such, if the update is // an upsert, create the collection that the update stage inserts into beforehand. if (!collection && request.isUpsert()) { @@ -113,12 +102,6 @@ UpdateResult update(OperationContext* opCtx, Database* db, const UpdateRequest& auto exec = uassertStatusOK(getExecutorUpdate(opCtx, nullOpDebug, collection, &parsedUpdate)); uassertStatusOK(exec->executePlan()); - if (repl::ReplClientInfo::forClient(client).getLastOp() != lastOpAtOperationStart) { - // If this operation has already generated a new lastOp, don't bother setting it here. - // No-op updates will not generate a new lastOp, so we still need the guard to fire in that - // case. - lastOpSetterGuard.Dismiss(); - } const UpdateStats* updateStats = UpdateStage::getUpdateStats(exec.get()); |