summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--jstests/replsets/noop_writes_wait_for_write_concern.js14
-rw-r--r--src/mongo/db/commands/apply_ops_cmd.cpp23
-rw-r--r--src/mongo/db/commands/create_indexes.cpp8
-rw-r--r--src/mongo/db/commands/find_and_modify.cpp19
-rw-r--r--src/mongo/db/ops/delete.cpp8
-rw-r--r--src/mongo/db/ops/update.cpp17
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 c252b8fd073..262e8ef93d3 100644
--- a/src/mongo/db/commands/apply_ops_cmd.cpp
+++ b/src/mongo/db/commands/apply_ops_cmd.cpp
@@ -116,22 +116,17 @@ public:
}
}
- auto client = txn->getClient();
- auto lastOpAtOperationStart = repl::ReplClientInfo::forClient(client).getLastOp();
- ScopeGuard lastOpSetterGuard =
- MakeObjGuard(repl::ReplClientInfo::forClient(client),
- &repl::ReplClientInfo::setLastOpToSystemLastOpTime,
- txn);
-
+ // 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(txn, 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 489e080f7a6..ee0133da46b 100644
--- a/src/mongo/db/commands/create_indexes.cpp
+++ b/src/mongo/db/commands/create_indexes.cpp
@@ -290,12 +290,6 @@ public:
const int numIndexesBefore = collection->getIndexCatalog()->numIndexesTotal(txn);
result.append("numIndexesBefore", numIndexesBefore);
- auto client = txn->getClient();
- ScopeGuard lastOpSetterGuard =
- MakeObjGuard(repl::ReplClientInfo::forClient(client),
- &repl::ReplClientInfo::setLastOpToSystemLastOpTime,
- txn);
-
MultiIndexBlock indexer(txn, collection);
indexer.allowBackgroundBuilding();
indexer.allowInterruption();
@@ -403,8 +397,6 @@ public:
result.append("numIndexesAfter", collection->getIndexCatalog()->numIndexesTotal(txn));
- 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 79ee34203ce..61dbc4f6509 100644
--- a/src/mongo/db/commands/find_and_modify.cpp
+++ b/src/mongo/db/commands/find_and_modify.cpp
@@ -364,18 +364,6 @@ public:
if (shouldBypassDocumentValidationForCommand(cmdObj))
maybeDisableValidation.emplace(txn);
- auto client = txn->getClient();
- auto lastOpAtOperationStart = repl::ReplClientInfo::forClient(client).getLastOp();
- ScopeGuard lastOpSetterGuard =
- MakeObjGuard(repl::ReplClientInfo::forClient(client),
- &repl::ReplClientInfo::setLastOpToSystemLastOpTime,
- txn);
-
- // If this is the local database, don't set last op.
- if (dbName == "local") {
- lastOpSetterGuard.Dismiss();
- }
-
auto curOp = CurOp::get(txn);
OpDebug* opDebug = &curOp->debug();
@@ -567,13 +555,6 @@ public:
}
MONGO_WRITE_CONFLICT_RETRY_LOOP_END(txn, "findAndModify", nsString.ns());
- 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 7f509308ad4..6a7da15ee74 100644
--- a/src/mongo/db/ops/delete.cpp
+++ b/src/mongo/db/ops/delete.cpp
@@ -63,19 +63,11 @@ long long deleteObjects(OperationContext* txn,
ParsedDelete parsedDelete(txn, &request);
uassertStatusOK(parsedDelete.parseRequest());
- auto client = txn->getClient();
- auto lastOpAtOperationStart = repl::ReplClientInfo::forClient(client).getLastOp();
-
std::unique_ptr<PlanExecutor> exec = uassertStatusOK(
getExecutorDelete(txn, &CurOp::get(txn)->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(txn);
- }
-
return DeleteStage::getNumDeleted(*exec);
}
diff --git a/src/mongo/db/ops/update.cpp b/src/mongo/db/ops/update.cpp
index 5e0763f9eac..bc3c595dc6c 100644
--- a/src/mongo/db/ops/update.cpp
+++ b/src/mongo/db/ops/update.cpp
@@ -63,20 +63,9 @@ UpdateResult update(OperationContext* txn, Database* db, const UpdateRequest& re
// Explain should never use this helper.
invariant(!request.isExplain());
- auto client = txn->getClient();
- auto lastOpAtOperationStart = repl::ReplClientInfo::forClient(client).getLastOp();
- ScopeGuard lastOpSetterGuard = MakeObjGuard(repl::ReplClientInfo::forClient(client),
- &repl::ReplClientInfo::setLastOpToSystemLastOpTime,
- txn);
-
const NamespaceString& nsString = request.getNamespaceString();
Collection* collection = db->getCollection(nsString.ns());
- // 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()) {
@@ -116,12 +105,6 @@ UpdateResult update(OperationContext* txn, Database* db, const UpdateRequest& re
uassertStatusOK(getExecutorUpdate(txn, 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());