diff options
| -rw-r--r-- | buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml | 2 | ||||
| -rw-r--r-- | jstests/sharding/movePrimary1.js | 6 | ||||
| -rw-r--r-- | jstests/views/views_all_commands.js | 92 | ||||
| -rw-r--r-- | jstests/views/views_sharded.js | 14 | ||||
| -rw-r--r-- | src/mongo/s/commands/cluster_shard_collection_cmd.cpp | 96 |
5 files changed, 151 insertions, 59 deletions
diff --git a/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml b/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml index 882821c1490..34091bebed6 100644 --- a/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml +++ b/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards.yml @@ -14,6 +14,8 @@ selector: - jstests/sharding/add_shard_to_zone.js - jstests/sharding/remove_shard_from_zone.js - jstests/sharding/update_zone_key_range.js + # TODO Assumes mongod and mongos handle read on view. Enable when 3.4 becomes 'last-stable'. + - jstests/sharding/movePrimary1.js # v3.4 replace noAutoSplit flag with document in config.settings - jstests/sharding/disable_autosplit.js # v3.4 changes $in shard key extraction. diff --git a/jstests/sharding/movePrimary1.js b/jstests/sharding/movePrimary1.js index 25217879e6d..4db1a09c41a 100644 --- a/jstests/sharding/movePrimary1.js +++ b/jstests/sharding/movePrimary1.js @@ -10,6 +10,9 @@ c.save({a: 3}); assert.eq(3, c.count()); + assert.commandWorked( + db.runCommand({create: "view", viewOn: "foo", pipeline: [{$match: {a: 3}}]})); + return s.getPrimaryShard(name); }; @@ -18,6 +21,7 @@ assert.eq(3, from.getDB("test1").foo.count(), "from doesn't have data before move"); assert.eq(0, to.getDB("test1").foo.count(), "to has data before move"); + assert.eq(1, s.s.getDB("test1").view.count(), "count on view incorrect before move"); assert.eq(s.normalize(s.config.databases.findOne({_id: "test1"}).primary), s.normalize(from.name), @@ -32,6 +36,7 @@ assert.eq(0, from.getDB("test1").foo.count(), "from still has data after move"); assert.eq(3, to.getDB("test1").foo.count(), "to doesn't have data after move"); + assert.eq(1, s.s.getDB("test1").view.count(), "count on view incorrect after move"); // move back, now using shard name instead of server address s.admin.runCommand({moveprimary: "test1", to: oldShardName}); @@ -42,6 +47,7 @@ assert.eq(3, from.getDB("test1").foo.count(), "from doesn't have data after move back"); assert.eq(0, to.getDB("test1").foo.count(), "to has data after move back"); + assert.eq(1, s.s.getDB("test1").view.count(), "count on view incorrect after move back"); // attempting to move primary DB to non-existent shard should error out with appropriate code var res = s.admin.runCommand({movePrimary: 'test1', to: 'dontexist'}); diff --git a/jstests/views/views_all_commands.js b/jstests/views/views_all_commands.js index e912b894c4a..6076ca77992 100644 --- a/jstests/views/views_all_commands.js +++ b/jstests/views/views_all_commands.js @@ -28,8 +28,12 @@ * selection of commonly-used reasons below.) * * expectFailure - * If true, assert that the command fails with a views-specific error code. Otherwise, all - * commands are expected to succeed. + * If true, assert that the command fails. Otherwise, all commands are expected to succeed. + * + * expectedErrorCode + * When 'expectFailure' is true, specifies the error code expected. Defaults to + * 'CommandNotSupportedOnView' when not specified. Set to 'null' when expecting an error + * without an error code field. * * setup * A function that will be run before the command is executed. It takes a handle to the 'test' @@ -97,7 +101,9 @@ expectFailure: true, }, checkShardingIndex: {skip: isUnrelated}, - cleanupOrphaned: {command: {cleanupOrphaned: 1}, skip: "TODO(SERVER-24764)"}, + cleanupOrphaned: { + skip: "Tested in views/views_sharded.js", + }, clone: {skip: "TODO(SERVER-24506)"}, cloneCollection: {skip: "TODO(SERVER-24506)"}, cloneCollectionAsCapped: { @@ -228,7 +234,7 @@ command: {getShardVersion: "test.view"}, isAdminCommand: true, expectFailure: true, - skip: "TODO(SERVER-24764)" + skipSharded: true, // mongos is tested in views/views_sharded.js }, getnonce: {skip: isUnrelated}, godinsert: {skip: isAnInternalCommand}, @@ -296,15 +302,25 @@ "mapreduce.shardedfinish": {skip: isAnInternalCommand}, mergeChunks: { command: {mergeChunks: "test.view", bounds: [{x: 0}, {x: 10}]}, + setup: function(conn) { + assert.commandWorked(conn.adminCommand({enableSharding: "test"})); + }, + skipStandalone: true, isAdminCommand: true, - skip: "TODO(SERVER-24764) Confirm/add correct mongoS error handling.", + expectFailure: true, + expectedErrorCode: ErrorCodes.NamespaceNotSharded, }, moveChunk: { command: {moveChunk: "test.view"}, + setup: function(conn) { + assert.commandWorked(conn.adminCommand({enableSharding: "test"})); + }, + skipStandalone: true, isAdminCommand: true, - skip: "TODO(SERVER-24764) Confirm/add correct mongoS error handling.", + expectFailure: true, + expectedErrorCode: ErrorCodes.NamespaceNotSharded, }, - movePrimary: {skip: "TODO(SERVER-24764) Add test for views"}, + movePrimary: {skip: "Tested in sharding/movePrimary1.js"}, netstat: {skip: isAnInternalCommand}, parallelCollectionScan: {command: {parallelCollectionScan: "view"}, expectFailure: true}, ping: {command: {ping: 1}}, @@ -328,7 +344,8 @@ { isAdminCommand: true, command: {renameCollection: "test.collection", to: "test.view"}, - expectFailure: ErrorCodes.NamespaceExists, + expectFailure: true, + expectedErrorCode: ErrorCodes.NamespaceExists, } ], repairCursor: {command: {repairCursor: "view"}, expectFailure: true}, @@ -382,15 +399,44 @@ skipStandalone: true, expectFailure: true, isAdminCommand: true, - skip: "TODO(SERVER-24764) Add view check to mongoS" }, shardConnPoolStats: {skip: isUnrelated}, shardingState: {skip: isUnrelated}, shutdown: {skip: isUnrelated}, sleep: {skip: isUnrelated}, - split: {skip: "TODO(SERVER-24764) Test that split on view fails"}, - splitChunk: {skip: "TODO(SERVER-24764) Test that split on view fails"}, - splitVector: {skip: "TODO(SERVER-24764) Test that split on view fails"}, + split: { + command: {split: "test.view", find: {_id: 1}}, + setup: function(conn) { + assert.commandWorked(conn.adminCommand({enableSharding: "test"})); + }, + skipStandalone: true, + expectFailure: true, + expectedErrorCode: ErrorCodes.NamespaceNotSharded, + isAdminCommand: true, + }, + splitChunk: { + command: { + splitChunk: "test.view", + from: "shard0000", + min: {x: MinKey}, + max: {x: 0}, + keyPattern: {x: 1}, + splitKeys: [{x: -2}, {x: -1}], + shardVersion: [1, 2] + }, + skipSharded: true, + expectFailure: true, + expectedErrorCode: null, + isAdminCommand: true, + }, + splitVector: { + command: { + splitVector: "test.view", + keyPattern: {x: 1}, + maxChunkSize: 1, + }, + expectFailure: true, + }, stageDebug: {skip: isAnInternalCommand}, top: {command: {top: "view"}, isAdminCommand: true, skip: "TODO(SERVER-24568)"}, touch: { @@ -421,18 +467,16 @@ /** * Helper function for failing commands or writes that checks the result 'res' of either. - * If 'code' is undefined or true, the expected error defaults to CommandNotSupportedOnView. - * Otherwise it is the numerical value of code. On no error, or wrong error code, the resulting - * assert includes the message 'msg'. + * If 'code' is null we only check for failure, otherwise we confirm error code matches as + * well. On assert 'msg' is printed. */ let assertCommandOrWriteFailed = function(res, code, msg) { - if (code == undefined || code === true) - code = ErrorCodes.CommandNotSupportedOnView; - if (res.writeErrors !== undefined) assert.neq(0, res.writeErrors.length, msg); - else + else if (res.code !== null) assert.commandFailedWithCode(res, code, msg); + else + assert.commandFailed(res, msg); }; function runTests(db) { @@ -493,11 +537,15 @@ if (subtest.isAdminCommand) commandHandle = db.getSiblingDB("admin"); - if (subtest.expectFailure) + if (subtest.expectFailure) { + let expectedErrorCode = subtest.expectedErrorCode; + if (expectedErrorCode === undefined) + expectedErrorCode = ErrorCodes.CommandNotSupportedOnView; + assertCommandOrWriteFailed(commandHandle.runCommand(subtest.command), - subtest.expectFailure, + expectedErrorCode, tojson(subtest.command)); - else if (subtest.command instanceof Function) + } else if (subtest.command instanceof Function) subtest.command(commandHandle); else assert.commandWorked(commandHandle.runCommand(subtest.command), diff --git a/jstests/views/views_sharded.js b/jstests/views/views_sharded.js index 07113d34dad..99408fcc9a3 100644 --- a/jstests/views/views_sharded.js +++ b/jstests/views/views_sharded.js @@ -75,4 +75,18 @@ assert.commandWorked(result); assert(result.hasOwnProperty("shards"), tojson(result)); + // + // Confirm cleanupOrphaned command fails. + // + result = st.getPrimaryShard(db.getName()).getDB("admin").runCommand({ + cleanupOrphaned: view.getFullName() + }); + assert.commandFailedWithCode(result, ErrorCodes.CommandNotSupportedOnView); + + // + // Confirm getShardVersion command fails. + // + assert.commandFailedWithCode(db.adminCommand({getShardVersion: view.getFullName()}), + ErrorCodes.NamespaceNotSharded); + })(); diff --git a/src/mongo/s/commands/cluster_shard_collection_cmd.cpp b/src/mongo/s/commands/cluster_shard_collection_cmd.cpp index 2d4d55bff4c..5471371e0e8 100644 --- a/src/mongo/s/commands/cluster_shard_collection_cmd.cpp +++ b/src/mongo/s/commands/cluster_shard_collection_cmd.cpp @@ -223,52 +223,74 @@ public: } } - BSONObj collectionOptions; - if (res["options"].type() == BSONType::Object) { - collectionOptions = res["options"].Obj(); - } + BSONObj defaultCollation; - // Check that collection is not capped. - if (collectionOptions["capped"].trueValue()) { - errmsg = "can't shard capped collection"; - conn.done(); - return false; - } + if (!res.isEmpty()) { + // Check that namespace is not a view. + { + std::string namespaceType; + auto status = bsonExtractStringField(res, "type", &namespaceType); + if (!status.isOK()) { + conn.done(); + return appendCommandStatus(result, status); + } - // Get collection default collation. - BSONObj defaultCollation; - { - BSONElement collationElement; - auto status = bsonExtractTypedField( - collectionOptions, "collation", BSONType::Object, &collationElement); - if (status.isOK()) { - defaultCollation = collationElement.Obj().getOwned(); - if (defaultCollation.isEmpty()) { + if (namespaceType == "view") { conn.done(); return appendCommandStatus( result, - {ErrorCodes::BadValue, - "Default collation in collection metadata cannot be empty."}); + {ErrorCodes::CommandNotSupportedOnView, "Views cannot be sharded."}); } - } else if (status != ErrorCodes::NoSuchKey) { + } + + BSONObj collectionOptions; + if (res["options"].type() == BSONType::Object) { + collectionOptions = res["options"].Obj(); + } + + // Check that collection is not capped. + if (collectionOptions["capped"].trueValue()) { + errmsg = "can't shard capped collection"; conn.done(); - return appendCommandStatus( - result, - {status.code(), - str::stream() << "Could not parse default collation in collection metadata " - << causedBy(status)}); + return false; } - } - // If the collection has a non-simple default collation but the user did not specify the - // simple collation explicitly, return an error. - if (!defaultCollation.isEmpty() && !simpleCollationSpecified) { - return appendCommandStatus(result, - {ErrorCodes::BadValue, - str::stream() - << "Collection has default collation: " - << collectionOptions["collation"] - << ". Must specify collation {locale: 'simple'}"}); + // Get collection default collation. + { + BSONElement collationElement; + auto status = bsonExtractTypedField( + collectionOptions, "collation", BSONType::Object, &collationElement); + if (status.isOK()) { + defaultCollation = collationElement.Obj().getOwned(); + if (defaultCollation.isEmpty()) { + conn.done(); + return appendCommandStatus( + result, + {ErrorCodes::BadValue, + "Default collation in collection metadata cannot be empty."}); + } + } else if (status != ErrorCodes::NoSuchKey) { + conn.done(); + return appendCommandStatus( + result, + {status.code(), + str::stream() + << "Could not parse default collation in collection metadata " + << causedBy(status)}); + } + } + + // If the collection has a non-simple default collation but the user did not specify the + // simple collation explicitly, return an error. + if (!defaultCollation.isEmpty() && !simpleCollationSpecified) { + conn.done(); + return appendCommandStatus(result, + {ErrorCodes::BadValue, + str::stream() + << "Collection has default collation: " + << collectionOptions["collation"] + << ". Must specify collation {locale: 'simple'}"}); + } } // The proposed shard key must be validated against the set of existing indexes. |