diff options
26 files changed, 71 insertions, 455 deletions
diff --git a/src/mongo/db/catalog/coll_mod.cpp b/src/mongo/db/catalog/coll_mod.cpp index b004bf6d837..9aa6e0391b3 100644 --- a/src/mongo/db/catalog/coll_mod.cpp +++ b/src/mongo/db/catalog/coll_mod.cpp @@ -292,11 +292,14 @@ void setCollectionOptionFlag(OperationContext* opCtx, invariant(newOptions.flagsSet); } +/** + * If uuid is specified, add it to the collection specified by nss. This will error if the + * collection already has a UUID. + */ Status _collModInternal(OperationContext* opCtx, const NamespaceString& nss, const BSONObj& cmdObj, BSONObjBuilder* result, - bool upgradeUUID, OptionalCollectionUUID uuid) { StringData dbName = nss.db(); AutoGetDb autoDb(opCtx, dbName, MODE_X); @@ -420,21 +423,14 @@ Status _collModInternal(OperationContext* opCtx, if (!cmr.noPadding.eoo()) setCollectionOptionFlag(opCtx, coll, cmr.noPadding, result); - // Modify collection UUID if we are upgrading or downgrading. This is a no-op if we have - // already upgraded or downgraded. As we don't assign UUIDs to system.indexes (SERVER-29926), - // don't implicitly upgrade them on collMod either. - if (upgradeUUID && !nss.isSystemDotIndexes()) { + // Add collection UUID if it is missing. This returns an error if a collection already has a + // different UUID. As we don't assign UUIDs to system.indexes (SERVER-29926), don't implicitly + // upgrade them on collMod either. + if (!nss.isSystemDotIndexes()) { if (uuid && !coll->uuid()) { log() << "Assigning UUID " << uuid.get().toString() << " to collection " << coll->ns(); CollectionCatalogEntry* cce = coll->getCatalogEntry(); cce->addUUID(opCtx, uuid.get(), coll); - } else if (!uuid && coll->uuid() && - serverGlobalParams.featureCompatibility.getVersion() < - ServerGlobalParams::FeatureCompatibility::Version::kFullyDowngradedTo36) { - log() << "Removing UUID " << coll->uuid().get().toString() << " from collection " - << coll->ns(); - CollectionCatalogEntry* cce = coll->getCatalogEntry(); - cce->removeUUID(opCtx); } else if (uuid && coll->uuid() && uuid.get() != coll->uuid().get()) { return Status(ErrorCodes::Error(40676), str::stream() << "collMod " << redact(cmdObj) << " provides a UUID (" @@ -458,10 +454,9 @@ Status _collModInternal(OperationContext* opCtx, return Status::OK(); } -void _updateDatabaseUUIDSchemaVersion(OperationContext* opCtx, - const std::string& dbname, - std::map<std::string, UUID>& collToUUID, - bool needUUIDAdded) { +void _addCollectionUUIDsPerDatabase(OperationContext* opCtx, + const std::string& dbname, + std::map<std::string, UUID>& collToUUID) { // Iterate through all collections of database dbname and make necessary UUID changes. std::vector<NamespaceString> collNamespaceStrings; { @@ -496,74 +491,19 @@ void _updateDatabaseUUIDSchemaVersion(OperationContext* opCtx, BSONObjBuilder collModObjBuilder; collModObjBuilder.append("collMod", coll->ns().coll()); BSONObj collModObj = collModObjBuilder.done(); - OptionalCollectionUUID uuid = boost::none; - if (needUUIDAdded) { - if (collToUUID.find(collNSS.coll().toString()) != collToUUID.end()) { - // This is a sharded collection. Use the UUID generated by the config server. - uuid = collToUUID.at(collNSS.coll().toString()); - } else { - // This is an unsharded collection. Generate a UUID. - uuid = UUID::gen(); - } - } - if ((needUUIDAdded && !coll->uuid()) || (!needUUIDAdded && coll->uuid())) { - uassertStatusOK(collModForUUIDUpgrade(opCtx, coll->ns(), collModObj, uuid)); - } - } -} - -Status _updateDatabaseUUIDSchemaVersionNonReplicated(OperationContext* opCtx, - const std::string& dbname, - bool needUUIDAdded) { - // Iterate through all collections if we're in the "local" database. - std::vector<NamespaceString> collNamespaceStrings; - if (dbname == "local") { - AutoGetDb autoDb(opCtx, dbname, MODE_X); - Database* const db = autoDb.getDb(); - if (!db) { - return Status(ErrorCodes::NamespaceNotFound, - str::stream() << "database " << dbname << " does not exist"); - } - for (auto collectionIt = db->begin(); collectionIt != db->end(); ++collectionIt) { - Collection* coll = *collectionIt; - collNamespaceStrings.push_back(coll->ns()); - } - } else { - // If we're not in the "local" database, the only non-replicated collection - // is system.profile, if present. - collNamespaceStrings.push_back(NamespaceString(dbname, "system.profile")); - } - for (auto& collNSS : collNamespaceStrings) { - // Skip system.namespaces until SERVER-30095 is addressed. - if (collNSS.coll() == "system.namespaces") { - continue; - } - AutoGetDb autoDb(opCtx, dbname, MODE_X); - Database* const db = autoDb.getDb(); - Collection* coll = db ? db->getCollection(opCtx, collNSS) : nullptr; - if (!coll) { - // If the collection or database was dropped, or if we incorrectly assumed there was - // a system.profile collection present, continue. - continue; - } - BSONObjBuilder collModObjBuilder; - collModObjBuilder.append("collMod", coll->ns().coll()); - BSONObj collModObj = collModObjBuilder.done(); - OptionalCollectionUUID uuid = boost::none; - if (needUUIDAdded) { + if (collToUUID.find(collNSS.coll().toString()) != collToUUID.end()) { + // This is a sharded collection. Use the UUID generated by the config server. + uuid = collToUUID.at(collNSS.coll().toString()); + } else { + // This is an unsharded collection. Generate a UUID. uuid = UUID::gen(); } - if ((needUUIDAdded && !coll->uuid()) || (!needUUIDAdded && coll->uuid())) { - BSONObjBuilder resultWeDontCareAbout; - auto collModStatus = _collModInternal( - opCtx, coll->ns(), collModObj, &resultWeDontCareAbout, /*upgradeUUID*/ true, uuid); - if (!collModStatus.isOK()) { - return collModStatus; - } + + if (!coll->uuid()) { + uassertStatusOK(collModForUUIDUpgrade(opCtx, coll->ns(), collModObj, uuid.get())); } } - return Status::OK(); } } // namespace @@ -571,33 +511,21 @@ Status collMod(OperationContext* opCtx, const NamespaceString& nss, const BSONObj& cmdObj, BSONObjBuilder* result) { - return _collModInternal( - opCtx, nss, cmdObj, result, /*upgradeUUID*/ false, /*UUID*/ boost::none); + return _collModInternal(opCtx, nss, cmdObj, result, /*UUID*/ boost::none); } Status collModForUUIDUpgrade(OperationContext* opCtx, const NamespaceString& nss, const BSONObj& cmdObj, - OptionalCollectionUUID uuid) { + CollectionUUID uuid) { BSONObjBuilder resultWeDontCareAbout; - // Update all non-replicated collection UUIDs. - if (nss.ns() == "admin.system.version") { - auto schemaStatus = updateUUIDSchemaVersionNonReplicated(opCtx, !!uuid); - if (!schemaStatus.isOK()) { - return schemaStatus; - } - } - return _collModInternal(opCtx, nss, cmdObj, &resultWeDontCareAbout, /*upgradeUUID*/ true, uuid); + return _collModInternal(opCtx, nss, cmdObj, &resultWeDontCareAbout, uuid); } -void updateUUIDSchemaVersion(OperationContext* opCtx, bool upgrade) { - if (!enableCollectionUUIDs) { - return; - } - +void addCollectionUUIDs(OperationContext* opCtx) { // A map of the form { db1: { collB: UUID, collA: UUID, ... }, db2: { ... } } std::map<std::string, std::map<std::string, UUID>> dbToCollToUUID; - if (upgrade && ShardingState::get(opCtx)->enabled()) { + if (ShardingState::get(opCtx)->enabled()) { log() << "obtaining UUIDs for pre-existing sharded collections from config server"; // Get UUIDs for all existing sharded collections from the config server. Since the sharded @@ -628,7 +556,7 @@ void updateUUIDSchemaVersion(OperationContext* opCtx, bool upgrade) { } } - // Update UUIDs on all collections of all databases. + // Add UUIDs to all collections of all databases if they do not already have UUIDs. std::vector<std::string> dbNames; StorageEngine* storageEngine = opCtx->getServiceContext()->getGlobalStorageEngine(); { @@ -651,15 +579,14 @@ void updateUUIDSchemaVersion(OperationContext* opCtx, bool upgrade) { } } - _updateDatabaseUUIDSchemaVersion(opCtx, dbName, dbToCollToUUID[dbName], upgrade); + _addCollectionUUIDsPerDatabase(opCtx, dbName, dbToCollToUUID[dbName]); } const auto& clientInfo = repl::ReplClientInfo::forClient(opCtx->getClient()); auto awaitOpTime = clientInfo.getLastOp(); - std::string upgradeStr = upgrade ? "upgrade" : "downgrade"; - log() << "Finished updating UUID schema version for " << upgradeStr - << ", waiting for all UUIDs to be committed at optime " << awaitOpTime << "."; + log() << "Finished adding UUIDs to collections, waiting for all UUIDs to be committed at optime" + << awaitOpTime << "."; const WriteConcernOptions writeConcern(WriteConcernOptions::kMajority, WriteConcernOptions::SyncMode::UNSET, @@ -667,24 +594,4 @@ void updateUUIDSchemaVersion(OperationContext* opCtx, bool upgrade) { repl::ReplicationCoordinator::get(opCtx)->awaitReplication(opCtx, awaitOpTime, writeConcern); } -Status updateUUIDSchemaVersionNonReplicated(OperationContext* opCtx, bool upgrade) { - if (!enableCollectionUUIDs) { - return Status::OK(); - } - // Update UUIDs on all collections of all non-replicated databases. - std::vector<std::string> dbNames; - StorageEngine* storageEngine = opCtx->getServiceContext()->getGlobalStorageEngine(); - { - Lock::GlobalLock lk(opCtx, MODE_IS, Date_t::max()); - storageEngine->listDatabases(&dbNames); - } - for (auto it = dbNames.begin(); it != dbNames.end(); ++it) { - auto dbName = *it; - auto schemaStatus = _updateDatabaseUUIDSchemaVersionNonReplicated(opCtx, dbName, upgrade); - if (!schemaStatus.isOK()) { - return schemaStatus; - } - } - return Status::OK(); -} } // namespace mongo diff --git a/src/mongo/db/catalog/coll_mod.h b/src/mongo/db/catalog/coll_mod.h index 6eb8b58b883..3dceb206981 100644 --- a/src/mongo/db/catalog/coll_mod.h +++ b/src/mongo/db/catalog/coll_mod.h @@ -38,17 +38,10 @@ class NamespaceString; class OperationContext; /** - * If upgrade is true, adds UUIDs to all collections of all databases. If upgrade is false, removes - * UUIDs from all collections of all databases. It updates non-replicated collections by indirectly - * calling updateUUIDSchemaVersionNonReplicated(). + * Adds UUIDs to all replicated collections of all databases if they do not already have UUIDs. If + * this function is not necessary for SERVER-33247, it can be removed. */ -void updateUUIDSchemaVersion(OperationContext* opCtx, bool upgrade); - -/** - * If upgrade is true, adds UUIDs to all non-replicated collections of all databases. If upgrade is - * false, removes UUIDs from all non-replicated collections of all databases. - */ -Status updateUUIDSchemaVersionNonReplicated(OperationContext* opCtx, bool upgrade); +void addCollectionUUIDs(OperationContext* opCtx); /** * Performs the collection modification described in "cmdObj" on the collection "ns". @@ -59,12 +52,11 @@ Status collMod(OperationContext* opCtx, BSONObjBuilder* result); /* - * Adds uuid to the collection "ns" if uuid exists and removes any existing UUID from - * the collection "ns" if uuid is boost::none. This is called in circumstances where - * we may be upgrading or downgrading and we need to update the UUID. + * Adds uuid to the collection "ns" if the collection does not already have a UUID. + * This is called if a collection failed to be assigned a UUID during upgrade to 3.6. */ Status collModForUUIDUpgrade(OperationContext* opCtx, const NamespaceString& ns, const BSONObj& cmdObj, - OptionalCollectionUUID uuid); + CollectionUUID uuid); } // namespace mongo diff --git a/src/mongo/db/catalog/collection_catalog_entry.h b/src/mongo/db/catalog/collection_catalog_entry.h index 2bd8bf2d125..ab8774eeb79 100644 --- a/src/mongo/db/catalog/collection_catalog_entry.h +++ b/src/mongo/db/catalog/collection_catalog_entry.h @@ -146,21 +146,16 @@ public: virtual void setIsTemp(OperationContext* opCtx, bool isTemp) = 0; /** - * Assigns a new UUID to this collection. This is to be called when the schemaVersion is set - * to 3.6 and there are collections that still do not have UUIDs. + * Assigns a new UUID to this collection. All collections must have UUIDs, so this is called if + * a collection erroneously does not have a UUID. */ virtual void addUUID(OperationContext* opCtx, CollectionUUID uuid, Collection* coll) = 0; - /** - * Removes the UUID from this collection. This is to be called when the schemaVersion is set - * to 3.4 and there are collections that still have UUIDs. - */ - virtual void removeUUID(OperationContext* opCtx) = 0; /** * Compare the UUID argument to the UUID obtained from the metadata. Return true if they * are equal, false otherwise. */ - virtual bool isEqualToMetadataUUID(OperationContext* opCtx, OptionalCollectionUUID uuid) = 0; + virtual bool isEqualToMetadataUUID(OperationContext* opCtx, CollectionUUID uuid) = 0; /** * Updates size of a capped Collection. diff --git a/src/mongo/db/catalog/collection_options.cpp b/src/mongo/db/catalog/collection_options.cpp index 7a5eecb1ee5..5cedeab2302 100644 --- a/src/mongo/db/catalog/collection_options.cpp +++ b/src/mongo/db/catalog/collection_options.cpp @@ -41,8 +41,6 @@ namespace mongo { -bool enableCollectionUUIDs = true; - // static bool CollectionOptions::validMaxCappedDocs(long long* max) { if (*max <= 0 || *max == std::numeric_limits<long long>::max()) { diff --git a/src/mongo/db/catalog/collection_options.h b/src/mongo/db/catalog/collection_options.h index 28941dfc71f..27a42d5f27e 100644 --- a/src/mongo/db/catalog/collection_options.h +++ b/src/mongo/db/catalog/collection_options.h @@ -38,8 +38,6 @@ namespace mongo { -extern bool enableCollectionUUIDs; // TODO(SERVER-27993) Replace based on upgrade/downgrade state. - class CollatorFactoryInterface; /** @@ -97,7 +95,9 @@ struct CollectionOptions { // ---- - // Collection UUID. Will exist if featureCompatibilityVersion >= 3.6. + // Collection UUID. Present for all CollectionOptions parsed for storage, except for those + // corresponding to the system.namespaces and system.indexes collections on MMAP (see + // SERVER-29926, SERVER-30095). OptionalCollectionUUID uuid; bool capped = false; diff --git a/src/mongo/db/catalog/database_impl.cpp b/src/mongo/db/catalog/database_impl.cpp index 1c7d4b0157b..62ef7317090 100644 --- a/src/mongo/db/catalog/database_impl.cpp +++ b/src/mongo/db/catalog/database_impl.cpp @@ -650,11 +650,9 @@ Collection* DatabaseImpl::getCollection(OperationContext* opCtx, const Namespace if (it != _collections.end() && it->second) { Collection* found = it->second; - if (enableCollectionUUIDs) { - NamespaceUUIDCache& cache = NamespaceUUIDCache::get(opCtx); - if (auto uuid = found->uuid()) - cache.ensureNamespaceInCache(nss, uuid.get()); - } + NamespaceUUIDCache& cache = NamespaceUUIDCache::get(opCtx); + if (auto uuid = found->uuid()) + cache.ensureNamespaceInCache(nss, uuid.get()); return found; } diff --git a/src/mongo/db/catalog/rename_collection.cpp b/src/mongo/db/catalog/rename_collection.cpp index 1d93179c373..eb8659b80dc 100644 --- a/src/mongo/db/catalog/rename_collection.cpp +++ b/src/mongo/db/catalog/rename_collection.cpp @@ -315,7 +315,7 @@ Status renameCollectionCommon(OperationContext* opCtx, // two collections with the same uuid (temporarily). if (targetUUID) newUUID = targetUUID; - else if (collectionOptions.uuid && enableCollectionUUIDs) + else if (collectionOptions.uuid) newUUID = UUID::gen(); collectionOptions.uuid = newUUID; diff --git a/src/mongo/db/commands/mr.cpp b/src/mongo/db/commands/mr.cpp index 5b60dd056e7..c4a2ce98128 100644 --- a/src/mongo/db/commands/mr.cpp +++ b/src/mongo/db/commands/mr.cpp @@ -426,9 +426,8 @@ void State::prepTempCollection() { CollectionOptions options; options.setNoIdIndex(); options.temp = true; - if (enableCollectionUUIDs) { - options.uuid.emplace(UUID::gen()); - } + options.uuid.emplace(UUID::gen()); + incColl = incCtx.db()->createCollection(_opCtx, _config.incLong.ns(), options); invariant(incColl); @@ -472,8 +471,7 @@ void State::prepTempCollection() { << _config.outputOptions.finalNamespace.ns() << " does not match UUID for the existing collection with that " "name on this shard", - finalColl->getCatalogEntry()->isEqualToMetadataUUID( - _opCtx, _config.finalOutputCollUUID)); + finalColl->uuid() == _config.finalOutputCollUUID); } IndexCatalog::IndexIterator ii = @@ -512,14 +510,12 @@ void State::prepTempCollection() { CollectionOptions options = finalOptions; options.temp = true; - if (enableCollectionUUIDs) { - // If a UUID for the final output collection was sent by mongos (i.e., the final output - // collection is sharded), use the UUID mongos sent when creating the temp collection. - // When the temp collection is renamed to the final output collection, the UUID will be - // preserved. - options.uuid.emplace(_config.finalOutputCollUUID ? *_config.finalOutputCollUUID - : UUID::gen()); - } + // If a UUID for the final output collection was sent by mongos (i.e., the final output + // collection is sharded), use the UUID mongos sent when creating the temp collection. + // When the temp collection is renamed to the final output collection, the UUID will be + // preserved. + options.uuid.emplace(_config.finalOutputCollUUID ? *_config.finalOutputCollUUID + : UUID::gen()); tempColl = tempCtx.db()->createCollection(_opCtx, _config.tempNamespace.ns(), options); for (vector<BSONObj>::iterator it = indexesToInsert.begin(); it != indexesToInsert.end(); diff --git a/src/mongo/db/op_observer_impl.cpp b/src/mongo/db/op_observer_impl.cpp index 05d6ac0b91c..f8db896ae6a 100644 --- a/src/mongo/db/op_observer_impl.cpp +++ b/src/mongo/db/op_observer_impl.cpp @@ -650,7 +650,7 @@ void OpObserverImpl::onCollMod(OperationContext* opCtx, ->logOp(opCtx, "c", cmdNss, cmdObj, nullptr); // Make sure the UUID values in the Collection metadata, the Collection object, and the UUID - // catalog are all present and equal if uuid exists and do not exist if uuid does not exist. + // catalog are all present and equal. invariant(opCtx->lockState()->isDbLockedForMode(nss.db(), MODE_X)); Database* db = dbHolder().get(opCtx, nss.db()); // Some unit tests call the op observer on an unregistered Database. @@ -658,9 +658,9 @@ void OpObserverImpl::onCollMod(OperationContext* opCtx, return; } Collection* coll = db->getCollection(opCtx, nss.ns()); - invariant(coll->uuid() == uuid); + invariant(coll->uuid() == uuid && coll->uuid()); CollectionCatalogEntry* entry = coll->getCatalogEntry(); - invariant(entry->isEqualToMetadataUUID(opCtx, uuid)); + invariant(entry->isEqualToMetadataUUID(opCtx, uuid.get())); } void OpObserverImpl::onDropDatabase(OperationContext* opCtx, const std::string& dbName) { @@ -773,7 +773,7 @@ repl::OpTime OpObserverImpl::onRenameCollection(OperationContext* const opCtx, builder.append("renameCollection", fromCollection.ns()); builder.append("to", toCollection.ns()); builder.append("stayTemp", stayTemp); - if (dropTargetUUID && enableCollectionUUIDs && !isMasterSlave(opCtx)) { + if (dropTargetUUID && !isMasterSlave(opCtx)) { dropTargetUUID->appendToBuilder(&builder, "dropTarget"); } else { builder.append("dropTarget", dropTarget); diff --git a/src/mongo/db/repl/initial_syncer.cpp b/src/mongo/db/repl/initial_syncer.cpp index 5e5cb8635c7..c66fbedeee0 100644 --- a/src/mongo/db/repl/initial_syncer.cpp +++ b/src/mongo/db/repl/initial_syncer.cpp @@ -1087,26 +1087,6 @@ void InitialSyncer::_rollbackCheckerCheckForRollbackCallback( return; } - // Set UUIDs for all non-replicated collections on secondaries. See comment in - // ReplicationCoordinatorExternalStateImpl::initializeReplSetStorage() for the explanation of - // why we do this. - const NamespaceString nss("admin", "system.version"); - auto opCtx = makeOpCtx(); - auto statusWithUUID = _storage->getCollectionUUID(opCtx.get(), nss); - if (!statusWithUUID.isOK()) { - // If the admin database does not exist, we intentionally fail initial sync. As part of - // SERVER-29448, we disallow dropping the admin database, so failing here is fine. - onCompletionGuard->setResultAndCancelRemainingWork_inlock(lock, statusWithUUID.getStatus()); - return; - } - if (statusWithUUID.getValue()) { - auto schemaStatus = _storage->upgradeUUIDSchemaVersionNonReplicated(opCtx.get()); - if (!schemaStatus.isOK()) { - onCompletionGuard->setResultAndCancelRemainingWork_inlock(lock, schemaStatus); - return; - } - } - // Success! onCompletionGuard->setResultAndCancelRemainingWork_inlock(lock, _lastApplied); } diff --git a/src/mongo/db/repl/initial_syncer_test.cpp b/src/mongo/db/repl/initial_syncer_test.cpp index 91d79e87e72..aeb11b63527 100644 --- a/src/mongo/db/repl/initial_syncer_test.cpp +++ b/src/mongo/db/repl/initial_syncer_test.cpp @@ -239,10 +239,6 @@ protected: bool droppedUserDBs = false; std::vector<std::string> droppedCollections; int documentsInsertedCount = 0; - bool schemaUpgraded = false; - OptionalCollectionUUID uuid; - bool getCollectionUUIDShouldFail = false; - bool upgradeUUIDSchemaVersionNonReplicatedShouldFail = false; }; stdx::mutex _storageInterfaceWorkDoneMutex; // protects _storageInterfaceWorkDone. @@ -255,10 +251,6 @@ protected: const NamespaceString& nss) { LockGuard lock(_storageInterfaceWorkDoneMutex); _storageInterfaceWorkDone.createOplogCalled = true; - _storageInterfaceWorkDone.schemaUpgraded = false; - _storageInterfaceWorkDone.uuid = boost::none; - _storageInterfaceWorkDone.getCollectionUUIDShouldFail = false; - _storageInterfaceWorkDone.upgradeUUIDSchemaVersionNonReplicatedShouldFail = false; return Status::OK(); }; _storageInterface->truncateCollFn = [this](OperationContext* opCtx, @@ -311,35 +303,6 @@ protected: return StatusWith<std::unique_ptr<CollectionBulkLoader>>( std::unique_ptr<CollectionBulkLoader>(collInfo->loader)); }; - _storageInterface->getCollectionUUIDFn = [this](OperationContext* opCtx, - const NamespaceString& nss) { - LockGuard lock(_storageInterfaceWorkDoneMutex); - if (_storageInterfaceWorkDone.getCollectionUUIDShouldFail) { - // getCollectionUUID returns NamespaceNotFound if either the db or the collection is - // missing. - return StatusWith<OptionalCollectionUUID>(Status( - ErrorCodes::NamespaceNotFound, - str::stream() << "getCollectionUUID failed because namespace " << nss.ns() - << " not found.")); - } else { - return StatusWith<OptionalCollectionUUID>(_storageInterfaceWorkDone.uuid); - } - }; - - _storageInterface->upgradeUUIDSchemaVersionNonReplicatedFn = - [this](OperationContext* opCtx) { - LockGuard lock(_storageInterfaceWorkDoneMutex); - if (_storageInterfaceWorkDone.upgradeUUIDSchemaVersionNonReplicatedShouldFail) { - // One of the status codes a failed upgradeUUIDSchemaVersionNonReplicated call - // can return is NamespaceNotFound. - return Status(ErrorCodes::NamespaceNotFound, - "upgradeUUIDSchemaVersionNonReplicated failed because the " - "desired ns was not found."); - } else { - _storageInterfaceWorkDone.schemaUpgraded = true; - return Status::OK(); - } - }; _dbWorkThreadPool = stdx::make_unique<OldThreadPool>(1); @@ -3493,13 +3456,7 @@ void InitialSyncerTest::doSuccessfulInitialSyncWithOneBatch() { TEST_F(InitialSyncerTest, InitialSyncerReturnsLastAppliedOnReachingStopTimestampAfterApplyingOneBatch) { - // In this test, getCollectionUUID should not return a UUID. Hence, - // upgradeUUIDSchemaVersionNonReplicated should not be called. doSuccessfulInitialSyncWithOneBatch(); - - // Ensure upgradeUUIDSchemaVersionNonReplicated was not called. - LockGuard lock(_storageInterfaceWorkDoneMutex); - ASSERT_FALSE(_storageInterfaceWorkDone.schemaUpgraded); } TEST_F(InitialSyncerTest, @@ -4040,53 +3997,4 @@ TEST_F(InitialSyncerTest, GetInitialSyncProgressReturnsCorrectProgress) { << attempt1; } -TEST_F(InitialSyncerTest, InitialSyncerUpdatesCollectionUUIDsIfgetCollectionUUIDReturnsUUID) { - // Ensure getCollectionUUID returns a UUID. This should trigger a call to - // upgradeUUIDSchemaVersionNonReplicated. - { - LockGuard lock(_storageInterfaceWorkDoneMutex); - _storageInterfaceWorkDone.uuid = UUID::gen(); - } - doSuccessfulInitialSyncWithOneBatch(); - - // Ensure upgradeUUIDSchemaVersionNonReplicated was called. - LockGuard lock(_storageInterfaceWorkDoneMutex); - ASSERT_TRUE(_storageInterfaceWorkDone.schemaUpgraded); -} - -TEST_F(InitialSyncerTest, InitialSyncerCapturesGetCollectionUUIDError) { - // Ensure getCollectionUUID returns a bad status. This should be passed to the initial syncer. - { - LockGuard lock(_storageInterfaceWorkDoneMutex); - _storageInterfaceWorkDone.getCollectionUUIDShouldFail = true; - } - doInitialSyncWithOneBatch(); - - // Ensure the getCollectionUUID status was captured. - ASSERT_EQUALS(ErrorCodes::NamespaceNotFound, _lastApplied); - - // Ensure upgradeUUIDSchemaVersionNonReplicated was not called. - LockGuard lock(_storageInterfaceWorkDoneMutex); - ASSERT_FALSE(_storageInterfaceWorkDone.schemaUpgraded); -} - -TEST_F(InitialSyncerTest, InitialSyncerCapturesUpgradeUUIDSchemaVersionError) { - // Ensure getCollectionUUID returns a UUID. This should trigger a call to - // upgradeUUIDSchemaVersionNonReplicated. - { - LockGuard lock(_storageInterfaceWorkDoneMutex); - _storageInterfaceWorkDone.uuid = UUID::gen(); - } - - // Ensure upgradeUUIDSchemaVersionNonReplicated returns a bad status. This should be passed to - // the initial syncer. - { - LockGuard lock(_storageInterfaceWorkDoneMutex); - _storageInterfaceWorkDone.upgradeUUIDSchemaVersionNonReplicatedShouldFail = true; - } - doInitialSyncWithOneBatch(); - - // Ensure the upgradeUUIDSchemaVersionNonReplicated status was captured. - ASSERT_EQUALS(ErrorCodes::NamespaceNotFound, _lastApplied); -} } // namespace diff --git a/src/mongo/db/repl/oplog.cpp b/src/mongo/db/repl/oplog.cpp index 4b9158877dd..62e2512564f 100644 --- a/src/mongo/db/repl/oplog.cpp +++ b/src/mongo/db/repl/oplog.cpp @@ -788,10 +788,10 @@ std::map<std::string, ApplyOpMetadata> opsMap = { BSONObj& cmd, const OpTime& opTime, OplogApplication::Mode mode) -> Status { - OptionalCollectionUUID uuid; NamespaceString nss; - std::tie(uuid, nss) = parseCollModUUIDAndNss(opCtx, ui, ns, cmd); - return collModForUUIDUpgrade(opCtx, nss, cmd, uuid); + BSONObjBuilder resultWeDontCareAbout; + std::tie(std::ignore, nss) = parseCollModUUIDAndNss(opCtx, ui, ns, cmd); + return collMod(opCtx, nss, cmd, &resultWeDontCareAbout); }, {ErrorCodes::IndexNotFound, ErrorCodes::NamespaceNotFound}}}, {"dbCheck", {dbCheckOplogCommand, {}}}, @@ -1503,8 +1503,7 @@ Status applyCommand_inlock(OperationContext* opCtx, } } - // During upgrade from 3.4 to 3.6, the feature compatibility version cannot change during - // initial sync because we cannot do some operations with UUIDs and others without. + // The feature compatibility version cannot change during initial sync. // We do not attempt to parse the whitelisted ops because they do not have a collection // namespace. If we drop the 'admin' database we will also log a 'drop' oplog entry for each // collection dropped. 'applyOps' will try to apply each individual operation, and those diff --git a/src/mongo/db/repl/replication_coordinator_external_state_impl.cpp b/src/mongo/db/repl/replication_coordinator_external_state_impl.cpp index c4af85c09fd..6f86ecc17b8 100644 --- a/src/mongo/db/repl/replication_coordinator_external_state_impl.cpp +++ b/src/mongo/db/repl/replication_coordinator_external_state_impl.cpp @@ -409,28 +409,6 @@ Status ReplicationCoordinatorExternalStateImpl::initializeReplSetStorage(Operati waitForAllEarlierOplogWritesToBeVisible(opCtx); }); - // Set UUIDs for all non-replicated collections. This is necessary for independent replica - // sets and config server replica sets started with no data files because collections in - // local are created prior to the featureCompatibilityVersion being set to 3.6, so the - // collections are not created with UUIDs. We exclude ShardServers when adding UUIDs to - // non-replicated collections on the primary because ShardServers are started up by default - // with featureCompatibilityVersion 3.4, so we don't want to assign UUIDs to them until the - // cluster's featureCompatibilityVersion is explicitly set to 3.6 by the config server. The - // below UUID addition for non-replicated collections only occurs on the primary; UUIDs are - // added to non-replicated collections on secondaries during InitialSync. When the config - // server sets the featureCompatibilityVersion to 3.6, the shard primary will add UUIDs to - // all the collections that need them. One special case here is if a shard is already in - // featureCompatibilityVersion 3.6 and a new node is started up with --shardsvr and added to - // that shard, the new node will still start up with featureCompatibilityVersion 3.4 and - // need to have UUIDs added to each collection. These UUIDs are added during InitialSync, - // because the new node is a secondary. - if (serverGlobalParams.clusterRole != ClusterRole::ShardServer && - FeatureCompatibilityVersion::isCleanStartUp()) { - auto schemaStatus = updateUUIDSchemaVersionNonReplicated(opCtx, true); - if (!schemaStatus.isOK()) { - return schemaStatus; - } - } FeatureCompatibilityVersion::setIfCleanStartup(opCtx, _storageInterface); } catch (const DBException& ex) { return ex.toStatus(); diff --git a/src/mongo/db/repl/rs_rollback_test.cpp b/src/mongo/db/repl/rs_rollback_test.cpp index 38fbde8f10d..258b88d8305 100644 --- a/src/mongo/db/repl/rs_rollback_test.cpp +++ b/src/mongo/db/repl/rs_rollback_test.cpp @@ -77,7 +77,6 @@ private: void RSRollbackTest::setUp() { RollbackTest::setUp(); - enableCollectionUUIDs = true; auto observerRegistry = stdx::make_unique<OpObserverRegistry>(); observerRegistry->addObserver(stdx::make_unique<UUIDCatalogObserver>()); _serviceContextMongoDTest.getServiceContext()->setOpObserver( diff --git a/src/mongo/db/repl/storage_interface.h b/src/mongo/db/repl/storage_interface.h index bade022996a..895c6d93a6e 100644 --- a/src/mongo/db/repl/storage_interface.h +++ b/src/mongo/db/repl/storage_interface.h @@ -333,12 +333,6 @@ public: const NamespaceString& nss) = 0; /** - * Adds UUIDs for non-replicated collections. To be called only at the end of initial - * sync and only if the admin.system.version collection has a UUID. - */ - virtual Status upgradeUUIDSchemaVersionNonReplicated(OperationContext* opCtx) = 0; - - /** * Sets the highest timestamp at which the storage engine is allowed to take a checkpoint. * This timestamp can never decrease, and thus should be a timestamp that can never roll back. */ diff --git a/src/mongo/db/repl/storage_interface_impl.cpp b/src/mongo/db/repl/storage_interface_impl.cpp index eb1503f611e..10b4684ba64 100644 --- a/src/mongo/db/repl/storage_interface_impl.cpp +++ b/src/mongo/db/repl/storage_interface_impl.cpp @@ -1006,10 +1006,6 @@ StatusWith<OptionalCollectionUUID> StorageInterfaceImpl::getCollectionUUID( return collection->uuid(); } -Status StorageInterfaceImpl::upgradeUUIDSchemaVersionNonReplicated(OperationContext* opCtx) { - return updateUUIDSchemaVersionNonReplicated(opCtx, true); -} - void StorageInterfaceImpl::setStableTimestamp(ServiceContext* serviceCtx, Timestamp snapshotName) { serviceCtx->getGlobalStorageEngine()->setStableTimestamp(snapshotName); } diff --git a/src/mongo/db/repl/storage_interface_impl.h b/src/mongo/db/repl/storage_interface_impl.h index d5545e5270f..5200b11839b 100644 --- a/src/mongo/db/repl/storage_interface_impl.h +++ b/src/mongo/db/repl/storage_interface_impl.h @@ -152,8 +152,6 @@ public: StatusWith<OptionalCollectionUUID> getCollectionUUID(OperationContext* opCtx, const NamespaceString& nss) override; - Status upgradeUUIDSchemaVersionNonReplicated(OperationContext* opCtx) override; - void setStableTimestamp(ServiceContext* serviceCtx, Timestamp snapshotName) override; void setInitialDataTimestamp(ServiceContext* serviceCtx, Timestamp snapshotName) override; diff --git a/src/mongo/db/repl/storage_interface_mock.h b/src/mongo/db/repl/storage_interface_mock.h index 3eff258e2c7..76acf703819 100644 --- a/src/mongo/db/repl/storage_interface_mock.h +++ b/src/mongo/db/repl/storage_interface_mock.h @@ -125,7 +125,6 @@ public: using IsAdminDbValidFn = stdx::function<Status(OperationContext* opCtx)>; using GetCollectionUUIDFn = stdx::function<StatusWith<OptionalCollectionUUID>( OperationContext* opCtx, const NamespaceString& nss)>; - using UpgradeUUIDSchemaVersionNonReplicatedFn = stdx::function<Status(OperationContext* opCtx)>; StorageInterfaceMock() = default; @@ -278,10 +277,6 @@ public: return getCollectionUUIDFn(opCtx, nss); } - Status upgradeUUIDSchemaVersionNonReplicated(OperationContext* opCtx) override { - return upgradeUUIDSchemaVersionNonReplicatedFn(opCtx); - } - void setStableTimestamp(ServiceContext* serviceCtx, Timestamp snapshotName) override; void setInitialDataTimestamp(ServiceContext* serviceCtx, Timestamp snapshotName) override; @@ -363,11 +358,6 @@ public: OperationContext* opCtx, const NamespaceString& nss) -> StatusWith<OptionalCollectionUUID> { return Status{ErrorCodes::IllegalOperation, "GetCollectionUUIDFn not implemented."}; }; - UpgradeUUIDSchemaVersionNonReplicatedFn upgradeUUIDSchemaVersionNonReplicatedFn = - [](OperationContext* opCtx) -> Status { - return Status{ErrorCodes::IllegalOperation, - "UpgradeUUIDSchemaVersionNonReplicatedFn not implemented."}; - }; private: mutable stdx::mutex _mutex; diff --git a/src/mongo/db/repl/sync_tail_test.cpp b/src/mongo/db/repl/sync_tail_test.cpp index 83b474cafa9..bf818d354e4 100644 --- a/src/mongo/db/repl/sync_tail_test.cpp +++ b/src/mongo/db/repl/sync_tail_test.cpp @@ -1744,61 +1744,6 @@ TEST_F(IdempotencyTest, CollModIndexNotFound) { testOpsAreIdempotent(ops); } -TEST_F(SyncTailTest, FailOnAssigningUUIDToCollectionWithExistingUUID) { - NamespaceString nss("local." + _agent.getSuiteName() + "_" + _agent.getTestName()); - auto oldUUID = UUID::gen(); - CollectionOptions options; - options.uuid = oldUUID; - createCollection(_opCtx.get(), nss, options); - - auto collModCmd = BSON("collMod" << nss.coll()); - auto newUUID = UUID::gen(); - auto collModOp = repl::OplogEntry(nextOpTime(), - 1LL, - OpTypeEnum::kCommand, - nss, - newUUID, - boost::none, - repl::OplogEntry::kOplogVersion, - collModCmd, - boost::none, - {}, - boost::none, - boost::none, - boost::none, - boost::none, - boost::none); - - ASSERT_EQUALS(runOpInitialSync(collModOp), ErrorCodes::duplicateCodeForTest(40676)); -} - -TEST_F(SyncTailTest, SuccessOnAssigningUUIDToCollectionWithExistingUUID) { - NamespaceString nss("local." + _agent.getSuiteName() + "_" + _agent.getTestName()); - auto oldUUID = UUID::gen(); - CollectionOptions options; - options.uuid = oldUUID; - createCollection(_opCtx.get(), nss, options); - - auto collModCmd = BSON("collMod" << nss.coll()); - auto collModOp = repl::OplogEntry(nextOpTime(), - 1LL, - OpTypeEnum::kCommand, - nss, - oldUUID, - boost::none, - repl::OplogEntry::kOplogVersion, - collModCmd, - boost::none, - {}, - boost::none, - boost::none, - boost::none, - boost::none, - boost::none); - - ASSERT_OK(runOpInitialSync(collModOp)); -} - TEST_F(SyncTailTest, FailOnDropFCVCollection) { ASSERT_OK( ReplicationCoordinator::get(_opCtx.get())->setFollowerMode(MemberState::RS_RECOVERING)); diff --git a/src/mongo/db/s/config/sharding_catalog_manager.h b/src/mongo/db/s/config/sharding_catalog_manager.h index 7acf7ff81a5..786b862f7ee 100644 --- a/src/mongo/db/s/config/sharding_catalog_manager.h +++ b/src/mongo/db/s/config/sharding_catalog_manager.h @@ -263,7 +263,7 @@ public: * Iterates through each entry in config.collections that does not have a UUID, generates a UUID * for the collection, and updates the entry with the generated UUID. * - * Remove after 3.4 -> 3.6 upgrade. + * If this function is not necessary for SERVER-33247, it can be removed. */ void generateUUIDsForExistingShardedCollections(OperationContext* opCtx); diff --git a/src/mongo/db/s/migration_destination_manager.cpp b/src/mongo/db/s/migration_destination_manager.cpp index 6da2d5c121a..8aa3bd3651e 100644 --- a/src/mongo/db/s/migration_destination_manager.cpp +++ b/src/mongo/db/s/migration_destination_manager.cpp @@ -609,7 +609,7 @@ void MigrationDestinationManager::_migrateDriver(OperationContext* opCtx, donorUUID.emplace(UUID::parse(donorOptions)); } - if (!collection->getCatalogEntry()->isEqualToMetadataUUID(opCtx, donorUUID)) { + if (collection->uuid() != donorUUID) { setStateFailWarn( str::stream() << "Cannot receive chunk " diff --git a/src/mongo/db/storage/kv/kv_collection_catalog_entry.cpp b/src/mongo/db/storage/kv/kv_collection_catalog_entry.cpp index 2e1a787bd78..a7f586d396d 100644 --- a/src/mongo/db/storage/kv/kv_collection_catalog_entry.cpp +++ b/src/mongo/db/storage/kv/kv_collection_catalog_entry.cpp @@ -248,29 +248,9 @@ void KVCollectionCatalogEntry::addUUID(OperationContext* opCtx, } } -void KVCollectionCatalogEntry::removeUUID(OperationContext* opCtx) { - // Remove the UUID from CollectionOptions if a UUID exists. +bool KVCollectionCatalogEntry::isEqualToMetadataUUID(OperationContext* opCtx, CollectionUUID uuid) { MetaData md = _getMetaData(opCtx); - if (md.options.uuid) { - CollectionUUID uuid = md.options.uuid.get(); - md.options.uuid = boost::none; - _catalog->putMetaData(opCtx, ns().toString(), md); - UUIDCatalog& catalog = UUIDCatalog::get(opCtx->getServiceContext()); - Collection* coll = catalog.lookupCollectionByUUID(uuid); - if (coll) { - catalog.onDropCollection(opCtx, uuid); - } - } -} - -bool KVCollectionCatalogEntry::isEqualToMetadataUUID(OperationContext* opCtx, - OptionalCollectionUUID uuid) { - MetaData md = _getMetaData(opCtx); - if (uuid) { - return md.options.uuid && md.options.uuid.get() == uuid.get(); - } else { - return !md.options.uuid; - } + return md.options.uuid && md.options.uuid.get() == uuid; } void KVCollectionCatalogEntry::updateFlags(OperationContext* opCtx, int newValue) { diff --git a/src/mongo/db/storage/kv/kv_collection_catalog_entry.h b/src/mongo/db/storage/kv/kv_collection_catalog_entry.h index 609793f0ebb..5fd500be864 100644 --- a/src/mongo/db/storage/kv/kv_collection_catalog_entry.h +++ b/src/mongo/db/storage/kv/kv_collection_catalog_entry.h @@ -87,9 +87,7 @@ public: void addUUID(OperationContext* opCtx, CollectionUUID uuid, Collection* coll) final; - void removeUUID(OperationContext* opCtx) final; - - bool isEqualToMetadataUUID(OperationContext* opCtx, OptionalCollectionUUID uuid) final; + bool isEqualToMetadataUUID(OperationContext* opCtx, CollectionUUID uuid) final; RecordStore* getRecordStore() { return _recordStore.get(); diff --git a/src/mongo/db/storage/mmap_v1/catalog/namespace_details_collection_entry.cpp b/src/mongo/db/storage/mmap_v1/catalog/namespace_details_collection_entry.cpp index 25d45ee751b..d010c2d2ed0 100644 --- a/src/mongo/db/storage/mmap_v1/catalog/namespace_details_collection_entry.cpp +++ b/src/mongo/db/storage/mmap_v1/catalog/namespace_details_collection_entry.cpp @@ -449,47 +449,15 @@ void NamespaceDetailsCollectionCatalogEntry::addUUID(OperationContext* opCtx, } } -void NamespaceDetailsCollectionCatalogEntry::removeUUID(OperationContext* opCtx) { - // Remove the UUID from CollectionOptions if a UUID exists. - if (ns().coll() == "system.namespaces") { - return; - } - RecordData namespaceData; - invariant(_namespacesRecordStore->findRecord(opCtx, _namespacesRecordId, &namespaceData)); - auto namespacesBson = namespaceData.releaseToBson(); - if (!namespacesBson["options"].isABSONObj()) { - return; - } - auto optionsObj = namespacesBson["options"].Obj(); - - if (!optionsObj["uuid"].eoo()) { - CollectionUUID uuid = UUID::parse(optionsObj["uuid"]).getValue(); - _updateSystemNamespaces(opCtx, - BSON("$unset" << BSON("options.uuid" - << ""))); - UUIDCatalog& catalog = UUIDCatalog::get(opCtx->getServiceContext()); - Collection* coll = catalog.lookupCollectionByUUID(uuid); - if (coll) { - catalog.onDropCollection(opCtx, uuid); - } - } -} - bool NamespaceDetailsCollectionCatalogEntry::isEqualToMetadataUUID(OperationContext* opCtx, - OptionalCollectionUUID uuid) { + CollectionUUID uuid) { if (ns().coll() != "system.namespaces") { RecordData namespaceData; invariant(_namespacesRecordStore->findRecord(opCtx, _namespacesRecordId, &namespaceData)); auto namespacesBson = namespaceData.releaseToBson(); - if (uuid && namespacesBson["options"].isABSONObj()) { - auto optionsObj = namespacesBson["options"].Obj(); - return !optionsObj["uuid"].eoo() && - UUID::parse(optionsObj["uuid"]).getValue() == uuid.get(); - } else { - return !uuid && (!namespacesBson["options"].isABSONObj() || - namespacesBson["options"].Obj()["uuid"].eoo()); - } + auto optionsObj = namespacesBson["options"].Obj(); + return !optionsObj["uuid"].eoo() && UUID::parse(optionsObj["uuid"]).getValue() == uuid; } else { return true; } diff --git a/src/mongo/db/storage/mmap_v1/catalog/namespace_details_collection_entry.h b/src/mongo/db/storage/mmap_v1/catalog/namespace_details_collection_entry.h index 138cd2ee121..3a8f04ee615 100644 --- a/src/mongo/db/storage/mmap_v1/catalog/namespace_details_collection_entry.h +++ b/src/mongo/db/storage/mmap_v1/catalog/namespace_details_collection_entry.h @@ -104,9 +104,7 @@ public: void addUUID(OperationContext* opCtx, CollectionUUID uuid, Collection* coll) final; - void removeUUID(OperationContext* opCtx) final; - - bool isEqualToMetadataUUID(OperationContext* opCtx, OptionalCollectionUUID uuid) final; + bool isEqualToMetadataUUID(OperationContext* opCtx, CollectionUUID uuid); void updateValidator(OperationContext* opCtx, const BSONObj& validator, diff --git a/src/mongo/s/commands/cluster_map_reduce_cmd.cpp b/src/mongo/s/commands/cluster_map_reduce_cmd.cpp index 694a076563f..0ce5092a761 100644 --- a/src/mongo/s/commands/cluster_map_reduce_cmd.cpp +++ b/src/mongo/s/commands/cluster_map_reduce_cmd.cpp @@ -687,8 +687,7 @@ private: Shard::RetryPolicy::kIdempotent)); uassertStatusOK(cmdResponse.commandStatus); - // Parse the UUID for the sharded collection from the shardCollection response, if one is - // present. It will only be present if the cluster is in fcv=3.6. + // Parse the UUID for the sharded collection from the shardCollection response. auto shardCollResponse = ConfigsvrShardCollectionResponse::parse( IDLParserErrorContext("ConfigsvrShardCollectionResponse"), cmdResponse.response); *outUUID = std::move(shardCollResponse.getCollectionUUID()); |