From 6bc9ed599c3fa164703346a22bad17e33fa913e4 Mon Sep 17 00:00:00 2001 From: Benety Goh Date: Mon, 10 Sep 2018 10:48:52 -0400 Subject: SERVER-37002 collection rename drops indexes with long names in the target namespace This is already done implicitly during a two phase collection drop. Renaming a collection with dropTarget set to true should handle long index names in a similar manner. (cherry picked from commit 8f37bba5a81b975101ae0ad4a0f894cce6dba648) --- ...rop_collections_two_phase_rename_drop_target.js | 52 +++++++++++++++++++++- src/mongo/db/catalog/rename_collection.cpp | 47 +++++++++++++++++++ 2 files changed, 97 insertions(+), 2 deletions(-) diff --git a/jstests/replsets/drop_collections_two_phase_rename_drop_target.js b/jstests/replsets/drop_collections_two_phase_rename_drop_target.js index e0c2aabadb6..5a646036fc8 100644 --- a/jstests/replsets/drop_collections_two_phase_rename_drop_target.js +++ b/jstests/replsets/drop_collections_two_phase_rename_drop_target.js @@ -9,6 +9,17 @@ load('jstests/replsets/libs/two_phase_drops.js'); // For TwoPhaseDropCollectionTest. load('jstests/libs/check_log.js'); // For checkLog.contains(). + // Return a list of all indexes for a given collection. Use 'args' as the + // 'listIndexes' command arguments. + // Assumes all indexes in the collection fit in the first batch of results. + function listIndexes(database, coll, args) { + args = args || {}; + let failMsg = "'listIndexes' command failed"; + let listIndexesCmd = {listIndexes: coll}; + let res = assert.commandWorked(database.runCommand(listIndexesCmd, args), failMsg); + return res.cursor.firstBatch; + } + // Set up a two phase drop test. let testName = 'drop_collection_two_phase_rename_drop_target'; let dbName = testName; @@ -23,11 +34,23 @@ twoPhaseDropTest.createCollection(fromCollName); twoPhaseDropTest.createCollection(toCollName); - // Insert documents into both collections so that we can tell them apart. + // Collection renames with dropTarget set to true should handle long index names in the target + // collection gracefully. MMAPv1 imposes a hard limit on index namespaces so we have to drop + // indexes that are too long to store on disk after renaming the collection. const primary = replTest.getPrimary(); const testDb = primary.getDB(dbName); const fromColl = testDb.getCollection(fromCollName); const toColl = testDb.getCollection(toCollName); + let maxNsLength = 127; + let longIndexName = ''.pad(maxNsLength - (toColl.getFullName() + '.$').length, true, 'a'); + let shortIndexName = "short_name"; + + // In the target collection, which will be dropped, create one index with a "too long" name, and + // one with a name of acceptable size. + assert.commandWorked(toColl.ensureIndex({a: 1}, {name: longIndexName})); + assert.commandWorked(toColl.ensureIndex({b: 1}, {name: shortIndexName})); + + // Insert documents into both collections so that we can tell them apart. assert.writeOK(fromColl.insert({_id: 'from'})); assert.writeOK(toColl.insert({_id: 'to'})); replTest.awaitReplication(); @@ -61,7 +84,32 @@ assert.eq({_id: 'from'}, toColl.findOne()); // Confirm that original target collection is now a drop-pending collection. - assert(twoPhaseDropTest.collectionIsPendingDrop(toCollName)); + const isPendingDropResult = twoPhaseDropTest.collectionIsPendingDrop(toCollName); + assert(isPendingDropResult); + const droppedCollName = isPendingDropResult.name; + jsTestLog('Original target collection is now in a drop-pending state: ' + droppedCollName); + + // Check that indexes that would violate the namespace length constraints after rename were + // dropped. + const indexes = listIndexes(testDb, droppedCollName); + jsTestLog('Indexes in ' + droppedCollName + ': ' + tojson(indexes)); + assert(indexes.find(idx => idx.name === shortIndexName)); + assert.eq(undefined, indexes.find(idx => idx.name === longIndexName)); + + // Check that index drop appears before collection rename in the oplog. + const oplogColl = primary.getCollection('local.oplog.rs'); + const cmdNs = testDb.getCollection('$cmd').getFullName(); + const renameOplogEntry = + oplogColl.findOne({ns: cmdNs, 'o.renameCollection': fromColl.getFullName()}); + const dropIndexOplogEntry = + oplogColl.findOne({ns: cmdNs, o: {dropIndexes: toCollName, index: longIndexName}}); + const renameTimestamp = renameOplogEntry.ts; + const dropIndexTimestamp = dropIndexOplogEntry.ts; + assert.lt(dropIndexTimestamp, + renameTimestamp, + 'index was not dropped before collection. index drop: ' + + tojson(dropIndexOplogEntry) + ' . collection rename: ' + + tojson(renameOplogEntry)); // COMMIT collection drop. twoPhaseDropTest.resumeOplogApplication(secondary); diff --git a/src/mongo/db/catalog/rename_collection.cpp b/src/mongo/db/catalog/rename_collection.cpp index 9ef84923199..af4f6970285 100644 --- a/src/mongo/db/catalog/rename_collection.cpp +++ b/src/mongo/db/catalog/rename_collection.cpp @@ -254,6 +254,53 @@ Status renameCollectionCommon(OperationContext* opCtx, // Target collection exists - drop it. invariant(options.dropTarget); auto dropTargetUUID = targetColl->uuid(); + + // If this rename collection is replicated, check for long index names in the target + // collection that may exceed the MMAPv1 namespace limit when the target collection + // is renamed with a drop-pending namespace. + auto replCoord = repl::ReplicationCoordinator::get(opCtx); + auto isOplogDisabledForNamespace = replCoord->isOplogDisabledFor(opCtx, target); + auto isMasterSlave = + repl::ReplicationCoordinator::modeMasterSlave == replCoord->getReplicationMode(); + if (!isOplogDisabledForNamespace && !isMasterSlave) { + invariant(opCtx->writesAreReplicated()); + invariant(renameOpTimeFromApplyOps.isNull()); + + // Compile a list of any indexes that would become too long following the + // drop-pending rename. In the case that this collection drop gets rolled back, this + // will incur a performance hit, since those indexes will have to be rebuilt from + // scratch, but data integrity is maintained. + std::vector indexesToDrop; + auto indexIter = targetColl->getIndexCatalog()->getIndexIterator(opCtx, true); + + // Determine which index names are too long. Since we don't have the collection + // rename optime at this time, use the maximum optime to check the index names. + auto longDpns = target.makeDropPendingNamespace(repl::OpTime::max()); + while (indexIter.more()) { + auto index = indexIter.next(); + auto status = longDpns.checkLengthForRename(index->indexName().size()); + if (!status.isOK()) { + indexesToDrop.push_back(index); + } + } + + // Drop the offending indexes. + auto sourceUuidString = sourceUUID ? sourceUUID.get().toString() : "no UUID"; + auto dropTargetUuidString = + dropTargetUUID ? dropTargetUUID.get().toString() : "no UUID"; + for (auto&& index : indexesToDrop) { + log() << "renameCollection: renaming collection " << sourceUuidString + << " from " << source << " to " << target << " (" << dropTargetUuidString + << ") - target collection contains an index namespace '" + << index->indexNamespace() + << "' that would be too long after drop-pending rename. Dropping index " + "immediately."; + fassertStatusOK(50941, targetColl->getIndexCatalog()->dropIndex(opCtx, index)); + opObserver->onDropIndex( + opCtx, target, targetColl->uuid(), index->indexName(), index->infoObj()); + } + } + auto renameOpTime = opObserver->onRenameCollection( opCtx, source, target, sourceUUID, true, dropTargetUUID, options.stayTemp); -- cgit v1.2.1