diff options
author | Nicholas Zolnierz <nicholas.zolnierz@mongodb.com> | 2020-01-13 21:44:46 +0000 |
---|---|---|
committer | evergreen <evergreen@mongodb.com> | 2020-01-13 21:44:46 +0000 |
commit | fa836e4fe1a7b5fc7f94abffa951a2d864069a2f (patch) | |
tree | 6e8630a130bab73466e18d385e27eb2bf1124195 | |
parent | dc711f478d75eeb00a33f8653334fd32616f4b2f (diff) | |
download | mongo-fa836e4fe1a7b5fc7f94abffa951a2d864069a2f.tar.gz |
SERVER-44527 Avoid renaming to target collection in mapReduce if the shard does not own any documents
(cherry picked from commit b4db881a18cbe15127a5a60c971cd393e0621466)
-rw-r--r-- | jstests/sharding/mapReduce_outSharded_checkUUID.js | 21 | ||||
-rw-r--r-- | src/mongo/db/commands/mr.cpp | 42 |
2 files changed, 41 insertions, 22 deletions
diff --git a/jstests/sharding/mapReduce_outSharded_checkUUID.js b/jstests/sharding/mapReduce_outSharded_checkUUID.js index 2e2df300c75..1845dbf45dc 100644 --- a/jstests/sharding/mapReduce_outSharded_checkUUID.js +++ b/jstests/sharding/mapReduce_outSharded_checkUUID.js @@ -82,6 +82,25 @@ assert.eq(newUUID, getUUIDFromListCollections(st.shard0.getDB("mrShard"), "outSharded")); assert.eq(newUUID, getUUIDFromListCollections(st.shard1.getDB("mrShard"), "outSharded")); + // Check that merge to an existing sharding collection that has data only on the primary shard + // works and that the collection uses the same UUID after M/R. + assert.writeOK(st.s.getCollection("mrShard.outSharded").remove({_id: 2001})); + out = db.srcSharded.mapReduce(map, reduce, {out: {merge: "outSharded", sharded: true}}); + verifyOutput(out, 513); + newUUID = getUUIDFromConfigCollections(st.s, "mrShard.outSharded"); + assert.eq(origUUID, newUUID); + assert.eq(newUUID, getUUIDFromListCollections(st.shard0.getDB(db.getName()), "outSharded")); + assert.eq(newUUID, getUUIDFromListCollections(st.shard1.getDB(db.getName()), "outSharded")); + + // Similarly, check that reduce to an existing sharding collection that has data only on the + // primary shard works and that the collection uses the same UUID after M/R. + out = db.srcSharded.mapReduce(map, reduce, {out: {reduce: "outSharded", sharded: true}}); + verifyOutput(out, 513); + newUUID = getUUIDFromConfigCollections(st.s, "mrShard.outSharded"); + assert.eq(origUUID, newUUID); + assert.eq(newUUID, getUUIDFromListCollections(st.shard0.getDB(db.getName()), "outSharded")); + assert.eq(newUUID, getUUIDFromListCollections(st.shard1.getDB(db.getName()), "outSharded")); + // Check that replace to an existing sharded collection has data on all shards works and that // the collection creates a new UUID after M/R. origUUID = getUUIDFromConfigCollections(st.s, "mrShard.outSharded"); @@ -148,4 +167,4 @@ st.stop(); -})();
\ No newline at end of file +})(); diff --git a/src/mongo/db/commands/mr.cpp b/src/mongo/db/commands/mr.cpp index 7e4cb30ae97..ced28109df0 100644 --- a/src/mongo/db/commands/mr.cpp +++ b/src/mongo/db/commands/mr.cpp @@ -659,11 +659,16 @@ long long State::postProcessCollectionNonAtomic(OperationContext* opCtx, CurOp* curOp, ProgressMeterHolder& pm, bool callerHoldsGlobalLock) { - if (_config.outputOptions.finalNamespace == _config.tempNamespace) - return _collectionCount(opCtx, _config.outputOptions.finalNamespace, callerHoldsGlobalLock); - - if (_config.outputOptions.outType == Config::REPLACE || - _collectionCount(opCtx, _config.outputOptions.finalNamespace, callerHoldsGlobalLock) == 0) { + auto outputCount = + _collectionCount(opCtx, _config.outputOptions.finalNamespace, callerHoldsGlobalLock); + + // Determine whether the temp collection should be renamed to the final output collection and + // thus preserve the UUID. This is possible in the following cases: + // * Output mode "replace" + // * If this mapReduce is creating a new sharded output collection, which can be determined by + // whether mongos sent the UUID that the final output collection should have (that is, whether + // _config.finalOutputCollUUID is set). + if (_config.outputOptions.outType == Config::REPLACE || _config.finalOutputCollUUID) { // This must be global because we may write across different databases. Lock::GlobalWrite lock(opCtx); // replace: just rename from temp to final collection name, dropping previous collection @@ -682,12 +687,11 @@ long long State::postProcessCollectionNonAtomic(OperationContext* opCtx, _db.dropCollection(_config.tempNamespace.ns()); } else if (_config.outputOptions.outType == Config::MERGE) { // merge: upsert new docs into old collection + { - const auto count = - _collectionCount(opCtx, _config.tempNamespace, callerHoldsGlobalLock); - stdx::lock_guard<Client> lk(*opCtx->getClient()); + stdx::unique_lock<Client> lk(*opCtx->getClient()); curOp->setMessage_inlock( - "m/r: merge post processing", "M/R Merge Post Processing Progress", count); + "m/r: merge post processing", "M/R Merge Post Processing Progress", outputCount); } unique_ptr<DBClientCursor> cursor = _db.query(_config.tempNamespace.ns(), BSONObj()); while (cursor->more()) { @@ -703,12 +707,11 @@ long long State::postProcessCollectionNonAtomic(OperationContext* opCtx, BSONList values; { - const auto count = - _collectionCount(opCtx, _config.tempNamespace, callerHoldsGlobalLock); - stdx::lock_guard<Client> lk(*opCtx->getClient()); + stdx::unique_lock<Client> lk(*opCtx->getClient()); curOp->setMessage_inlock( - "m/r: reduce post processing", "M/R Reduce Post Processing Progress", count); + "m/r: reduce post processing", "M/R Reduce Post Processing Progress", outputCount); } + unique_ptr<DBClientCursor> cursor = _db.query(_config.tempNamespace.ns(), BSONObj()); while (cursor->more()) { // This must be global because we may write across different databases. @@ -716,14 +719,11 @@ long long State::postProcessCollectionNonAtomic(OperationContext* opCtx, BSONObj temp = cursor->nextSafe(); BSONObj old; - bool found; - { - OldClientContext tx(opCtx, _config.outputOptions.finalNamespace.ns()); - Collection* coll = - getCollectionOrUassert(opCtx, tx.db(), _config.outputOptions.finalNamespace); - found = Helpers::findOne(opCtx, coll, temp["_id"].wrap(), old, true); - } - + const bool found = [&] { + AutoGetCollection autoColl(opCtx, _config.outputOptions.finalNamespace, MODE_IS); + return Helpers::findOne( + opCtx, autoColl.getCollection(), temp["_id"].wrap(), old, true); + }(); if (found) { // need to reduce values.clear(); |