From 83162ebfc508346a283463053d858dd7dfe6d97d Mon Sep 17 00:00:00 2001 From: Henrik Edin Date: Fri, 30 Oct 2020 10:26:10 -0400 Subject: SERVER-51895 Fix so drop index is registering a valid ident with the reaper when possible Also fix so the drop pending ident reaper is used in standalone mode --- src/mongo/db/catalog/index_catalog_entry.h | 2 ++ src/mongo/db/catalog/index_catalog_entry_impl.cpp | 4 ++++ src/mongo/db/catalog/index_catalog_entry_impl.h | 2 ++ src/mongo/db/catalog/index_catalog_impl.cpp | 22 ++++++++-------------- src/mongo/db/catalog/index_catalog_impl.h | 8 ++++++++ src/mongo/db/index/index_access_method.cpp | 4 ---- src/mongo/db/index/index_access_method.h | 9 +-------- src/mongo/db/storage/storage_engine_impl.cpp | 2 +- 8 files changed, 26 insertions(+), 27 deletions(-) diff --git a/src/mongo/db/catalog/index_catalog_entry.h b/src/mongo/db/catalog/index_catalog_entry.h index 7e8ea4013e8..20094c5246b 100644 --- a/src/mongo/db/catalog/index_catalog_entry.h +++ b/src/mongo/db/catalog/index_catalog_entry.h @@ -49,6 +49,7 @@ class CollatorInterface; class Collection; class CollectionPtr; class CollectionCatalogEntry; +class Ident; class IndexAccessMethod; class IndexBuildInterceptor; class IndexDescriptor; @@ -66,6 +67,7 @@ public: virtual void init(std::unique_ptr accessMethod) = 0; virtual const std::string& getIdent() const = 0; + virtual std::shared_ptr getSharedIdent() const = 0; virtual IndexDescriptor* descriptor() = 0; diff --git a/src/mongo/db/catalog/index_catalog_entry_impl.cpp b/src/mongo/db/catalog/index_catalog_entry_impl.cpp index 7e46b1326a8..a15d671099e 100644 --- a/src/mongo/db/catalog/index_catalog_entry_impl.cpp +++ b/src/mongo/db/catalog/index_catalog_entry_impl.cpp @@ -319,6 +319,10 @@ Status IndexCatalogEntryImpl::_setMultikeyInMultiDocumentTransaction( return Status::OK(); } +std::shared_ptr IndexCatalogEntryImpl::getSharedIdent() const { + return {shared_from_this(), _accessMethod->getSortedDataInterface()}; // aliasing constructor +} + // ---- NamespaceString IndexCatalogEntryImpl::getNSSFromCatalog(OperationContext* opCtx) const { diff --git a/src/mongo/db/catalog/index_catalog_entry_impl.h b/src/mongo/db/catalog/index_catalog_entry_impl.h index fc83722f42b..96177a9f4b8 100644 --- a/src/mongo/db/catalog/index_catalog_entry_impl.h +++ b/src/mongo/db/catalog/index_catalog_entry_impl.h @@ -69,6 +69,8 @@ public: return _ident; } + std::shared_ptr getSharedIdent() const final; + IndexDescriptor* descriptor() final { return _descriptor.get(); } diff --git a/src/mongo/db/catalog/index_catalog_impl.cpp b/src/mongo/db/catalog/index_catalog_impl.cpp index 6ba972ea18e..ddf67684ecc 100644 --- a/src/mongo/db/catalog/index_catalog_impl.cpp +++ b/src/mongo/db/catalog/index_catalog_impl.cpp @@ -1071,32 +1071,26 @@ Status IndexCatalogImpl::dropIndexEntry(OperationContext* opCtx, IndexCatalogEnt } CollectionQueryInfo::get(_collection).droppedIndex(opCtx, _collection, indexName); - entry = nullptr; - deleteIndexFromDisk(opCtx, indexName); + _deleteIndexFromDisk(opCtx, indexName, entry->getSharedIdent()); return Status::OK(); } void IndexCatalogImpl::deleteIndexFromDisk(OperationContext* opCtx, const string& indexName) { invariant(opCtx->lockState()->isCollectionLockedForMode(_collection->ns(), MODE_X)); + _deleteIndexFromDisk(opCtx, indexName, nullptr); +} - // The index catalog entry may not exist in the catalog depending on how far an index build may - // have gotten before needing to abort. If the catalog entry cannot be found, then there are no - // concurrent operations still using the index. - auto ident = [&]() -> std::shared_ptr { - auto indexDesc = findIndexByName(opCtx, indexName, true /* includeUnfinishedIndexes */); - if (!indexDesc) { - return nullptr; - } - return getEntry(indexDesc)->accessMethod()->getSharedIdent(); - }(); - +void IndexCatalogImpl::_deleteIndexFromDisk(OperationContext* opCtx, + const string& indexName, + std::shared_ptr ident) { + invariant(!findIndexByName(opCtx, indexName, true /* includeUnfinishedIndexes*/)); catalog::removeIndex(opCtx, indexName, _collection->getCatalogId(), _collection->uuid(), _collection->ns(), - ident); + std::move(ident)); } void IndexCatalogImpl::setMultikeyPaths(OperationContext* const opCtx, diff --git a/src/mongo/db/catalog/index_catalog_impl.h b/src/mongo/db/catalog/index_catalog_impl.h index ab12e104fcc..83711ee9d92 100644 --- a/src/mongo/db/catalog/index_catalog_impl.h +++ b/src/mongo/db/catalog/index_catalog_impl.h @@ -359,6 +359,14 @@ private: bool logIfError, int64_t* keysDeletedOut); + /** + * Helper to remove the index from disk. + * The index should be removed from the in-memory catalog beforehand. + */ + void _deleteIndexFromDisk(OperationContext* opCtx, + const std::string& indexName, + std::shared_ptr ident); + /** * Applies a set of transformations to the user-provided index object 'spec' to make it * conform to the standard for insertion. Removes the '_id' field if it exists, applies diff --git a/src/mongo/db/index/index_access_method.cpp b/src/mongo/db/index/index_access_method.cpp index ed7090c12a9..3710cee3476 100644 --- a/src/mongo/db/index/index_access_method.cpp +++ b/src/mongo/db/index/index_access_method.cpp @@ -859,10 +859,6 @@ SortedDataInterface* AbstractIndexAccessMethod::getSortedDataInterface() const { return _newInterface.get(); } -std::shared_ptr AbstractIndexAccessMethod::getSharedIdent() const { - return _newInterface; -} - /** * Generates a new file name on each call using a static, atomic and monotonically increasing * number. Each name is suffixed with a random number generated at startup, to prevent name diff --git a/src/mongo/db/index/index_access_method.h b/src/mongo/db/index/index_access_method.h index 1daf02f3015..5aee4fad6e4 100644 --- a/src/mongo/db/index/index_access_method.h +++ b/src/mongo/db/index/index_access_method.h @@ -376,11 +376,6 @@ public: * documents into an index, except for testing purposes. */ virtual SortedDataInterface* getSortedDataInterface() const = 0; - - /** - * Fetches the Ident for this index. - */ - virtual std::shared_ptr getSharedIdent() const = 0; }; /** @@ -571,8 +566,6 @@ public: SortedDataInterface* getSortedDataInterface() const override final; - std::shared_ptr getSharedIdent() const override final; - protected: /** * Fills 'keys' with the keys that should be generated for 'obj' on this index. @@ -620,7 +613,7 @@ private: const KeyString::Value& dataKey, const RecordIdHandlerFn& onDuplicateRecord); - const std::shared_ptr _newInterface; + const std::unique_ptr _newInterface; }; } // namespace mongo diff --git a/src/mongo/db/storage/storage_engine_impl.cpp b/src/mongo/db/storage/storage_engine_impl.cpp index 897eea72c33..5c9f90bc56f 100644 --- a/src/mongo/db/storage/storage_engine_impl.cpp +++ b/src/mongo/db/storage/storage_engine_impl.cpp @@ -764,7 +764,7 @@ Status StorageEngineImpl::_dropCollectionsNoTimestamp(OperationContext* opCtx, coll->getCatalogId(), coll->uuid(), coll->ns(), - ice->accessMethod()->getSharedIdent()); + ice->getSharedIdent()); } Status result = catalog::dropCollection( -- cgit v1.2.1