From c0c28f5663d232c67949ba3c6a5536c50018d0d0 Mon Sep 17 00:00:00 2001 From: William Schultz Date: Tue, 29 Aug 2017 13:16:11 -0400 Subject: SERVER-30413 Add function to set options.temp when resyncing collection metadata for rollback --- src/mongo/db/catalog/collection_catalog_entry.h | 6 ++ src/mongo/db/repl/rs_rollback.cpp | 8 +- src/mongo/db/repl/rs_rollback_test.cpp | 107 +++++++++++---------- .../db/storage/kv/kv_collection_catalog_entry.cpp | 6 ++ .../db/storage/kv/kv_collection_catalog_entry.h | 2 + .../catalog/namespace_details_collection_entry.cpp | 5 + .../catalog/namespace_details_collection_entry.h | 2 + 7 files changed, 84 insertions(+), 52 deletions(-) diff --git a/src/mongo/db/catalog/collection_catalog_entry.h b/src/mongo/db/catalog/collection_catalog_entry.h index cbf206b0c13..9a2b6ba8f93 100644 --- a/src/mongo/db/catalog/collection_catalog_entry.h +++ b/src/mongo/db/catalog/collection_catalog_entry.h @@ -133,6 +133,12 @@ public: const BSONObj& validator, StringData validationLevel, StringData validationAction) = 0; + + /** + * Updates the 'temp' setting for this collection. + */ + 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. diff --git a/src/mongo/db/repl/rs_rollback.cpp b/src/mongo/db/repl/rs_rollback.cpp index 7b16265bcb8..7f7eb82bb06 100644 --- a/src/mongo/db/repl/rs_rollback.cpp +++ b/src/mongo/db/repl/rs_rollback.cpp @@ -1148,9 +1148,11 @@ void rollback_internal::syncFixUp(OperationContext* opCtx, WriteUnitOfWork wuow(opCtx); - // TODO: Reset options.temp. If the collection is temporary, we set the - // temp field to true. Otherwise, we do not add the the temp field. - // See SERVER-30413. + // If the collection is temporary, we set the temp field to true. Otherwise, we do not + // add the the temp field. + if (options.temp) { + cce->setIsTemp(opCtx, options.temp); + } // Resets collection user flags such as noPadding and usePowerOf2Sizes. if (options.flagsSet || cce->getCollectionOptions(opCtx).flagsSet) { diff --git a/src/mongo/db/repl/rs_rollback_test.cpp b/src/mongo/db/repl/rs_rollback_test.cpp index ee1caf718be..62fbecbf7f5 100644 --- a/src/mongo/db/repl/rs_rollback_test.cpp +++ b/src/mongo/db/repl/rs_rollback_test.cpp @@ -34,6 +34,7 @@ #include #include "mongo/db/catalog/collection.h" +#include "mongo/db/catalog/collection_catalog_entry.h" #include "mongo/db/catalog/drop_indexes.h" #include "mongo/db/catalog/index_catalog.h" #include "mongo/db/catalog/index_create.h" @@ -976,55 +977,63 @@ TEST_F(RSRollbackTest, RollbackRenameCollectionInSameDatabaseCommand) { } } -// TODO: Un-comment this test once options.temp is properly resynced during rollback. -// See SERVER-30413 - -// TEST_F(RSRollbackTest, -// RollingBackRenameCollectionFromTempToPermanentCollectionSetsCollectionOptionToTemp) { -// createOplog(_opCtx.get()); -// -// CollectionOptions options; -// options.uuid = UUID::gen(); -// auto collection = _createCollection(_opCtx.get(), "test.y", options); -// auto collectionUUID = collection->uuid().get(); -// -// class RollbackSourceLocal : public RollbackSourceMock { -// public: -// RollbackSourceLocal(std::unique_ptr oplog) -// : RollbackSourceMock(std::move(oplog)) {} -// StatusWith getCollectionInfoByUUID(const std::string& db, const UUID& uuid) const -// { -// getCollectionInfoCalled = true; -// return BSON("info" << BSON("uuid" << uuid) << "options" << BSON("temp" << true))); -// } -// mutable bool getCollectionInfoCalled = false; -// }; -// -// auto commonOperation = -// std::make_pair(BSON("ts" << Timestamp(Seconds(1), 0) << "h" << 1LL), RecordId(1)); -// -// auto renameCollectionOperation = makeRenameCollectionOplogEntry(NamespaceString("test.x"), -// NamespaceString("test.y"), -// collectionUUID, -// boost::none, -// boost::none, -// false, -// OpTime(Timestamp(2, 0), 5)); -// -// RollbackSourceLocal rollbackSource(std::unique_ptr(new OplogInterfaceMock({ -// commonOperation, -// }))); -// -// ASSERT_OK(syncRollback(_opCtx.get(), -// OplogInterfaceMock({renameCollectionOperation, commonOperation}), -// rollbackSource, -// {}, -// _coordinator, -// _replicationProcess.get())); -// -// ASSERT_TRUE(rollbackSource.getCollectionInfoCalled); -// ASSERT_EQUALS(options.temp, true); -//} +TEST_F(RSRollbackTest, + RollingBackRenameCollectionFromTempToPermanentCollectionSetsCollectionOptionToTemp) { + createOplog(_opCtx.get()); + + auto renameFromNss = NamespaceString("test.renameFrom"); + auto renameToNss = NamespaceString("test.renameTo"); + + CollectionOptions options; + options.uuid = UUID::gen(); + ASSERT_FALSE(options.temp); + + // Create the collection and save its UUID. + auto collection = _createCollection(_opCtx.get(), renameToNss, options); + auto collectionUUID = collection->uuid().get(); + + class RollbackSourceLocal : public RollbackSourceMock { + public: + RollbackSourceLocal(std::unique_ptr oplog) + : RollbackSourceMock(std::move(oplog)) {} + StatusWith getCollectionInfoByUUID(const std::string& db, const UUID& uuid) const { + getCollectionInfoCalled = true; + return BSON("info" << BSON("uuid" << uuid) << "options" << BSON("temp" << true)); + } + mutable bool getCollectionInfoCalled = false; + }; + + auto commonOperation = + std::make_pair(BSON("ts" << Timestamp(Seconds(1), 0) << "h" << 1LL), RecordId(1)); + + bool stayTemp = false; + auto renameCollectionOperation = makeRenameCollectionOplogEntry(NamespaceString(renameFromNss), + NamespaceString(renameToNss), + collectionUUID, + boost::none, + boost::none, + stayTemp, + OpTime(Timestamp(2, 0), 5)); + + RollbackSourceLocal rollbackSource(std::unique_ptr(new OplogInterfaceMock({ + commonOperation, + }))); + + ASSERT_OK(syncRollback(_opCtx.get(), + OplogInterfaceMock({renameCollectionOperation, commonOperation}), + rollbackSource, + {}, + _coordinator, + _replicationProcess.get())); + + ASSERT_TRUE(rollbackSource.getCollectionInfoCalled); + + AutoGetCollectionForReadCommand autoColl(_opCtx.get(), NamespaceString(renameFromNss)); + auto collAfterRollback = autoColl.getCollection(); + auto collAfterRollbackOptions = + collAfterRollback->getCatalogEntry()->getCollectionOptions(_opCtx.get()); + ASSERT_TRUE(collAfterRollbackOptions.temp); +} TEST_F(RSRollbackTest, RollbackRenameCollectionInDatabaseWithDropTargetTrueCommand) { createOplog(_opCtx.get()); 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 d354af2bb05..05f9f927c79 100644 --- a/src/mongo/db/storage/kv/kv_collection_catalog_entry.cpp +++ b/src/mongo/db/storage/kv/kv_collection_catalog_entry.cpp @@ -289,6 +289,12 @@ void KVCollectionCatalogEntry::updateValidator(OperationContext* opCtx, _catalog->putMetaData(opCtx, ns().toString(), md); } +void KVCollectionCatalogEntry::setIsTemp(OperationContext* opCtx, bool isTemp) { + MetaData md = _getMetaData(opCtx); + md.options.temp = isTemp; + _catalog->putMetaData(opCtx, ns().toString(), md); +} + void KVCollectionCatalogEntry::updateCappedSize(OperationContext* opCtx, long long size) { MetaData md = _getMetaData(opCtx); md.options.cappedSize = size; 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 6ff063eb388..9cae8936e95 100644 --- a/src/mongo/db/storage/kv/kv_collection_catalog_entry.h +++ b/src/mongo/db/storage/kv/kv_collection_catalog_entry.h @@ -79,6 +79,8 @@ public: StringData validationLevel, StringData validationAction) final; + void setIsTemp(OperationContext* opCtx, bool isTemp); + void updateCappedSize(OperationContext*, long long int) final; void addUUID(OperationContext* opCtx, CollectionUUID uuid, Collection* coll) final; 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 c0ecd805ad1..ee8a60a9fa3 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 @@ -494,6 +494,11 @@ void NamespaceDetailsCollectionCatalogEntry::updateValidator(OperationContext* o << validationAction))); } +void NamespaceDetailsCollectionCatalogEntry::setIsTemp(OperationContext* opCtx, bool isTemp) { + _updateSystemNamespaces(opCtx, BSON("$set" << BSON("options.temp" << isTemp))); +} + + void NamespaceDetailsCollectionCatalogEntry::setNamespacesRecordId(OperationContext* opCtx, RecordId newId) { if (newId.isNull()) { 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 c5226f3934e..9cfbce48933 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 @@ -109,6 +109,8 @@ public: StringData validationLevel, StringData validationAction) final; + void setIsTemp(OperationContext* opCtx, bool isTemp) final; + void updateCappedSize(OperationContext* opCtx, long long size) final; // not part of interface, but available to my storage engine -- cgit v1.2.1