diff options
6 files changed, 127 insertions, 28 deletions
diff --git a/buildscripts/resmokeconfig/suites/sharded_collections_jscore_passthrough.yml b/buildscripts/resmokeconfig/suites/sharded_collections_jscore_passthrough.yml index 4c68e692df8..af856f86243 100644 --- a/buildscripts/resmokeconfig/suites/sharded_collections_jscore_passthrough.yml +++ b/buildscripts/resmokeconfig/suites/sharded_collections_jscore_passthrough.yml @@ -87,12 +87,6 @@ selector: - jstests/core/index_multikey.js - jstests/core/optimized_match_explain.js - jstests/core/sort_array.js - # TODO: SERVER-16605 - - jstests/core/mr_index.js - - jstests/core/mr1.js - - jstests/core/mr3.js - - jstests/core/mr4.js - - jstests/core/mr5.js exclude_with_any_tags: - assumes_against_mongod_not_mongos # Tests tagged with the following will fail because they assume collections are not sharded. 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 9921398ded5..8e645823b62 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 @@ -67,6 +67,8 @@ selector: - jstests/sharding/wildcard_index_banned_for_shard_key.js # Enable if SERVER-20865 is backported or 4.2 becomes last-stable - jstests/sharding/sharding_statistics_server_status.js + # Enable if SERVER-36966 is backported or 4.2 becomes last-stable + - jstests/sharding/mr_output_sharded_validation.js executor: config: shell_options: diff --git a/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards_misc.yml b/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards_misc.yml index cbb084ae70b..c8e3c1abbe2 100644 --- a/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards_misc.yml +++ b/buildscripts/resmokeconfig/suites/sharding_last_stable_mongos_and_mixed_shards_misc.yml @@ -381,6 +381,8 @@ selector: - jstests/sharding/wildcard_index_banned_for_shard_key.js # Enable if SERVER-20865 is backported or 4.2 becomes last-stable - jstests/sharding/sharding_statistics_server_status.js + # Enable if SERVER-36966 is backported or 4.2 becomes last-stable + - jstests/sharding/mr_output_sharded_validation.js executor: config: diff --git a/jstests/libs/override_methods/implicitly_shard_accessed_collections.js b/jstests/libs/override_methods/implicitly_shard_accessed_collections.js index f06cfc6fb8b..93f3cbd3e15 100644 --- a/jstests/libs/override_methods/implicitly_shard_accessed_collections.js +++ b/jstests/libs/override_methods/implicitly_shard_accessed_collections.js @@ -12,11 +12,15 @@ (function() { 'use strict'; - // Save a reference to the original getCollection method in the IIFE's scope. - // This scoping allows the original method to be called by the getCollection override below. + load("jstests/libs/override_methods/override_helpers.js"); // For 'OverrideHelpers'. + + // Save a reference to the original methods in the IIFE's scope. + // This scoping allows the original methods to be called by the overrides below. var originalGetCollection = DB.prototype.getCollection; var originalDBCollectionDrop = DBCollection.prototype.drop; var originalStartParallelShell = startParallelShell; + var originalRunCommand = Mongo.prototype.runCommand; + var testMayRunDropInParallel = false; // Blacklisted namespaces that should not be sharded. @@ -104,6 +108,54 @@ return dropResult; }; + // The mapReduce command has a special requirement where the command must indicate the output + // collection is sharded, so we must be sure to add this information in this passthrough. + Mongo.prototype.runCommand = function(dbName, cmdObj, options) { + // Skip any commands that are not mapReduce or do not have an 'out' option. + if (typeof cmdObj !== 'object' || cmdObj === null || + (!cmdObj.hasOwnProperty('mapreduce') && !cmdObj.hasOwnProperty('mapReduce')) || + !cmdObj.hasOwnProperty('out')) { + return originalRunCommand.apply(this, arguments); + } + + const originalCmdObj = Object.merge({}, cmdObj); + + // SERVER-5448 'jsMode' is not supported through mongos. The 'jsMode' should not impact the + // results at all, so can be safely deleted in the sharded environment. + delete cmdObj.jsMode; + + // Modify the output options to specify that the collection is sharded. + let outputSpec = cmdObj.out; + if (typeof(outputSpec) === "string") { + this.getDB(dbName)[outputSpec].drop(); // This will implicitly shard it. + outputSpec = {replace: outputSpec, sharded: true}; + } else if (typeof(outputSpec) !== "object") { + // This is a malformed command, just send it along. + return originalRunCommand.apply(this, arguments); + } else if (!outputSpec.hasOwnProperty("sharded")) { + let outputColl = null; + if (outputSpec.hasOwnProperty("replace")) { + outputColl = outputSpec.replace; + } else if (outputSpec.hasOwnProperty("merge")) { + outputColl = outputSpec.merge; + } else if (outputSpec.hasOwnProperty("reduce")) { + outputColl = outputSpec.reduce; + } + + if (outputColl === null) { + // This is a malformed command, just send it along. + return originalRunCommand.apply(this, arguments); + } + this.getDB(dbName)[outputColl].drop(); // This will implicitly shard it. + outputSpec.sharded = true; + } + + cmdObj.out = outputSpec; + jsTestLog('Overriding mapReduce command. Original command: ' + tojson(originalCmdObj) + + ' New command: ' + tojson(cmdObj)); + return originalRunCommand.apply(this, arguments); + }; + // Tests may use a parallel shell to run the "drop" command concurrently with other // operations. This can cause the "shardCollection" command to return a // ConflictingOperationInProgress error response. @@ -111,4 +163,8 @@ testMayRunDropInParallel = true; return originalStartParallelShell.apply(this, arguments); }; + + OverrideHelpers.prependOverrideInParallelShell( + "jstests/libs/override_methods/implicitly_shard_accessed_collections.js"); + }()); diff --git a/jstests/sharding/mr_output_sharded_validation.js b/jstests/sharding/mr_output_sharded_validation.js new file mode 100644 index 00000000000..1d33af3f83b --- /dev/null +++ b/jstests/sharding/mr_output_sharded_validation.js @@ -0,0 +1,44 @@ +// Tests that the cluster mapReduce command will prevent outputting to a sharded collection without +// the sharded: true option specified. Also tests that the command will correctly clean up the +// output namespace of the first phase of a mapReduce with sharded input before the final result +// collection is created. This test was designed to reproduce SERVER-36966. +(function() { + "use strict"; + + const st = new ShardingTest({shards: 2, config: 1, verbose: ''}); + + const mongosDB = st.s.getDB("test"); + st.shardColl(mongosDB.foo, {_id: 1}, {_id: 0}, {_id: -1}); + + assert.commandWorked(mongosDB.foo.insert([{_id: 1}, {_id: 2}])); + + assert.commandWorked(mongosDB.adminCommand( + {shardCollection: mongosDB.output.getFullName(), key: {_id: "hashed"}})); + + assert.commandWorked(mongosDB.foo.mapReduce( + function() { + emit(this._id, 1); + }, + function(key, values) { + return Array.sum(values); + }, + {out: {replace: "output", sharded: true}})); + + // Test that using just a collection name without specifying a merge mode or the 'sharded: true' + // information will fail if the named collection is sharded. + const error = assert.throws(() => mongosDB.foo.mapReduce( + function() { + emit(this._id, 1); + }, + function(key, values) { + return Array.sum(values); + }, + {out: "output"})); + assert.eq(error.code, 15920); + + for (let name of mongosDB.getCollectionNames()) { + assert.eq(-1, name.indexOf("tmp.mrs"), name); + } + + st.stop(); +}()); diff --git a/src/mongo/s/commands/cluster_map_reduce_cmd.cpp b/src/mongo/s/commands/cluster_map_reduce_cmd.cpp index af3f0acc46d..c2343f1bdae 100644 --- a/src/mongo/s/commands/cluster_map_reduce_cmd.cpp +++ b/src/mongo/s/commands/cluster_map_reduce_cmd.cpp @@ -227,11 +227,13 @@ public: } outputCollNss = NamespaceString(outDB, finalColShort); - uassert(ErrorCodes::InvalidNamespace, - "Invalid output namespace", - outputCollNss.isValid()); } + } else if (outElmt.type() == String) { + outputCollNss = NamespaceString(outDB, outElmt.String()); } + uassert(ErrorCodes::InvalidNamespace, + "Invalid output namespace", + inlineOutput || outputCollNss.isValid()); auto const catalogCache = Grid::get(opCtx)->catalogCache(); @@ -332,22 +334,25 @@ public: auto splitPts = SimpleBSONObjComparator::kInstance.makeBSONObjSet(); + // TODO: take distributed lock to prevent split / migration? + try { + Strategy::commandOp( + opCtx, dbname, shardedCommand, nss.ns(), q, collation, &mrCommandResults); + } catch (DBException& e) { + e.addContext(str::stream() << "could not run map command on all shards for ns " + << nss.ns() + << " and query " + << q); + throw; + } + + // Now that the output collections of the first phase ("tmp.mrs.<>") have been created, make + // a best effort to drop them if any part of the second phase fails. + ON_BLOCK_EXIT([&]() { cleanUp(servers, dbname, shardResultCollection); }); + { bool ok = true; - // TODO: take distributed lock to prevent split / migration? - - try { - Strategy::commandOp( - opCtx, dbname, shardedCommand, nss.ns(), q, collation, &mrCommandResults); - } catch (DBException& e) { - e.addContext(str::stream() << "could not run map command on all shards for ns " - << nss.ns() - << " and query " - << q); - throw; - } - for (const auto& mrResult : mrCommandResults) { // Need to gather list of all servers even if an error happened const auto server = [&]() { @@ -394,8 +399,6 @@ public: } if (!ok) { - cleanUp(servers, dbname, shardResultCollection); - // Add "code" to the top-level response, if the failure of the sharded command // can be accounted to a single error. int code = getUniqueCodeFromCommandResults(mrCommandResults); @@ -613,8 +616,6 @@ public: outputRoutingInfo.cm()); } - cleanUp(servers, dbname, shardResultCollection); - if (!ok) { errmsg = str::stream() << "MR post processing failed: " << singleResult.toString(); return false; |