summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorHaley Connelly <haley.connelly@mongodb.com>2020-07-01 18:08:05 +0000
committerEvergreen Agent <no-reply@evergreen.mongodb.com>2020-08-12 00:04:38 +0000
commit3a20a63dc5e96948131dc0393c07a644f49e8109 (patch)
tree39d090af2d3a80e95e724f0dc64b82ef492e4275
parent7e85b69ddc608d80dd4cab301ce4d869c38ecef5 (diff)
downloadmongo-3a20a63dc5e96948131dc0393c07a644f49e8109.tar.gz
SERVER-46811 multi=true updates can modify the shard key of orphan documents and cause them to become owned
(cherry picked from commit 17e468583ec1a7e9755b34b50eaa7cf017a1c96e)
-rw-r--r--jstests/sharding/multi_update_orphan_shard_key.js60
-rw-r--r--src/mongo/db/exec/update_stage.cpp13
2 files changed, 70 insertions, 3 deletions
diff --git a/jstests/sharding/multi_update_orphan_shard_key.js b/jstests/sharding/multi_update_orphan_shard_key.js
new file mode 100644
index 00000000000..6fec3a1653f
--- /dev/null
+++ b/jstests/sharding/multi_update_orphan_shard_key.js
@@ -0,0 +1,60 @@
+/**
+ * Tests it isn't possible to update an orphan document's shard key. Only multi=true updates skip
+ * shard versioning. They are therefore the only case which skips ownership filtering.
+ * @tags: [requires_fcv_44]
+ */
+(function() {
+"use strict";
+
+load("jstests/libs/fail_point_util.js");
+
+const st = new ShardingTest({mongos: 1, config: 1, shards: 2, rs: {nodes: 1}});
+const dbName = "test";
+const collName = "update_orphan_shard_key";
+const collection = st.s.getDB(dbName).getCollection(collName);
+
+// Create a sharded collection with two chunks on shard0, split at the key {x: -1}.
+assert.commandWorked(st.s.adminCommand({enableSharding: dbName}));
+assert.commandWorked(st.s.adminCommand({movePrimary: dbName, to: st.shard0.shardName}));
+assert.commandWorked(st.s.adminCommand({shardCollection: collection.getFullName(), key: {x: 1}}));
+assert.commandWorked(st.s.adminCommand({split: collection.getFullName(), middle: {x: -1}}));
+
+// Insert some documents into the collection, but only into the higher of the two chunks.
+assert.commandWorked(collection.insert(Array.from({length: 100}, (_, i) => ({_id: i, x: i}))));
+
+// Enable the failpoint to cause range deletion to hang indefinitely.
+const suspendRangeDeletionFailpoint = configureFailPoint(st.shard0, "suspendRangeDeletion");
+
+// Note: Use _waitForDelete=false to ensure the command completes since the test intentionally
+// causes range deletion to hang.
+assert.commandWorked(st.s.adminCommand({
+ moveChunk: collection.getFullName(),
+ find: {x: 1},
+ to: st.shard1.shardName,
+ _waitForDelete: false,
+}));
+
+let res = assert.commandWorked(collection.update({x: 0}, {$set: {y: 1}}));
+assert.eq(1, res.nMatched, res);
+assert.eq(1, res.nModified, res);
+
+// Do a multi=true update that will target both shards but not update any documents on the shard
+// which owns the range [-1, MaxKey].
+res = assert.commandFailedWithCode(
+ collection.update({x: {$lte: 0}, y: {$exists: false}}, {$set: {x: -10, y: 2}}, {multi: true}),
+ 31025);
+assert.eq(0, res.nMatched, res);
+assert.eq(0, res.nModified, res);
+assert.eq(0, res.nUpserted, res);
+
+// Wait for range deletion to happen on the donor.
+suspendRangeDeletionFailpoint.off();
+assert.soon(() => {
+ return st.shard0.getCollection(collection.getFullName()).findOne({x: 1}) === null;
+});
+
+// Run a $out aggregation as a simple way to check for duplicate _id values.
+assert.doesNotThrow(() => collection.aggregate([{$out: "output"}]));
+
+st.stop();
+})();
diff --git a/src/mongo/db/exec/update_stage.cpp b/src/mongo/db/exec/update_stage.cpp
index ffd6700c926..1a57e80b856 100644
--- a/src/mongo/db/exec/update_stage.cpp
+++ b/src/mongo/db/exec/update_stage.cpp
@@ -708,9 +708,10 @@ bool UpdateStage::checkUpdateChangesShardKeyFields(ScopedCollectionDescription c
auto oldShardKey = shardKeyPattern.extractShardKeyFromDoc(oldObj.value());
auto newShardKey = shardKeyPattern.extractShardKeyFromDoc(newObj);
- // If the shard key fields remain unchanged by this update or if this document is an orphan and
- // so does not belong to this shard, we can skip the rest of the checks.
- if ((newShardKey.woCompare(oldShardKey) == 0) || !collDesc->keyBelongsToMe(oldShardKey)) {
+ // If the shard key fields remain unchanged by this update we can skip the rest of the checks.
+ // Using BSONObj::binaryEqual() still allows a missing shard key field to be filled in with an
+ // explicit null value.
+ if (newShardKey.binaryEqual(oldShardKey)) {
return false;
}
@@ -748,6 +749,12 @@ bool UpdateStage::checkUpdateChangesShardKeyFields(ScopedCollectionDescription c
"retryWrites: true.",
opCtx()->getTxnNumber() || !opCtx()->writesAreReplicated());
+ // If the shard key of an orphan document is allowed to change, and the document is allowed to
+ // become owned by the shard, the global uniqueness assumption for _id values would be violated.
+ uassert(468110,
+ "Changing the shard key of an orphan document is not allowed",
+ collDesc->keyBelongsToMe(oldShardKey));
+
if (!collDesc->keyBelongsToMe(newShardKey)) {
if (MONGO_unlikely(hangBeforeThrowWouldChangeOwningShard.shouldFail())) {
LOGV2(20605, "Hit hangBeforeThrowWouldChangeOwningShard failpoint");