summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--jstests/sharding/replication_with_undefined_shard_key.js30
-rw-r--r--src/mongo/db/repl/oplog.cpp20
2 files changed, 44 insertions, 6 deletions
diff --git a/jstests/sharding/replication_with_undefined_shard_key.js b/jstests/sharding/replication_with_undefined_shard_key.js
new file mode 100644
index 00000000000..8e37c171735
--- /dev/null
+++ b/jstests/sharding/replication_with_undefined_shard_key.js
@@ -0,0 +1,30 @@
+// Test for SERVER-31953 where secondaries crash when replicating an oplog entry where the document
+// identifier in the oplog entry contains a shard key value that contains an undefined value.
+(function() {
+ "use strict";
+
+ const st = new ShardingTest({mongos: 1, config: 1, shard: 1, rs: {nodes: 2}});
+ const mongosDB = st.s.getDB("test");
+ const mongosColl = mongosDB.mycoll;
+
+ // Shard the test collection on the "x" field.
+ assert.commandWorked(mongosDB.adminCommand({enableSharding: mongosDB.getName()}));
+ assert.commandWorked(mongosDB.adminCommand({
+ shardCollection: mongosColl.getFullName(),
+ key: {x: 1},
+ }));
+
+ // Insert a document with a literal undefined value.
+ assert.writeOK(mongosColl.insert({x: undefined}));
+
+ jsTestLog("Doing writes that generate oplog entries including undefined document key");
+
+ assert.writeOK(mongosColl.update(
+ {},
+ {$set: {a: 1}},
+ {multi: true, writeConcern: {w: 2, wtimeout: ReplSetTest.kDefaultTimeoutMs}}));
+ assert.writeOK(
+ mongosColl.remove({}, {writeConcern: {w: 2, wtimeout: ReplSetTest.kDefaultTimeoutMs}}));
+
+ st.stop();
+})(); \ No newline at end of file
diff --git a/src/mongo/db/repl/oplog.cpp b/src/mongo/db/repl/oplog.cpp
index 909cf659fa8..c849a450f26 100644
--- a/src/mongo/db/repl/oplog.cpp
+++ b/src/mongo/db/repl/oplog.cpp
@@ -1297,12 +1297,15 @@ Status applyOperation_inlock(OperationContext* opCtx,
} else if (*opType == 'u') {
opCounters->gotUpdate();
- BSONObj updateCriteria = o2;
- const bool upsert = valueB || alwaysUpsert;
-
+ auto idField = o2["_id"];
uassert(ErrorCodes::NoSuchKey,
str::stream() << "Failed to apply update due to missing _id: " << op.toString(),
- updateCriteria.hasField("_id"));
+ !idField.eoo());
+
+ // The o2 field may contain additional fields besides the _id (like the shard key fields),
+ // but we want to do the update by just _id so we can take advantage of the IDHACK.
+ BSONObj updateCriteria = idField.wrap();
+ const bool upsert = valueB || alwaysUpsert;
UpdateRequest request(requestNss);
request.setQuery(updateCriteria);
@@ -1379,9 +1382,14 @@ Status applyOperation_inlock(OperationContext* opCtx,
} else if (*opType == 'd') {
opCounters->gotDelete();
+ auto idField = o["_id"];
uassert(ErrorCodes::NoSuchKey,
str::stream() << "Failed to apply delete due to missing _id: " << op.toString(),
- o.hasField("_id"));
+ !idField.eoo());
+
+ // The o field may contain additional fields besides the _id (like the shard key fields),
+ // but we want to do the delete by just _id so we can take advantage of the IDHACK.
+ BSONObj deleteCriteria = idField.wrap();
SnapshotName timestamp;
if (assignOperationTimestamp) {
@@ -1396,7 +1404,7 @@ Status applyOperation_inlock(OperationContext* opCtx,
}
if (opType[1] == 0) {
- deleteObjects(opCtx, collection, requestNss, o, /*justOne*/ valueB);
+ deleteObjects(opCtx, collection, requestNss, deleteCriteria, /*justOne*/ valueB);
} else
verify(opType[1] == 'b'); // "db" advertisement
wuow.commit();