From c7108af024e67563eb32a312eac7fb5d5bd9009f Mon Sep 17 00:00:00 2001 From: Gregory Wlodarek Date: Wed, 17 Jun 2020 13:46:20 -0400 Subject: SERVER-48772 Correctly timestamp writes when dropping multiple indexes (cherry picked from commit 9397a9e59b22909a6bf00050bb9653117544aea1) --- src/mongo/db/catalog/drop_indexes.cpp | 9 ++- src/mongo/dbtests/storage_timestamp_tests.cpp | 85 ++++++++++++++++++++++++++- 2 files changed, 89 insertions(+), 5 deletions(-) diff --git a/src/mongo/db/catalog/drop_indexes.cpp b/src/mongo/db/catalog/drop_indexes.cpp index 3b947e81851..8159ebea355 100644 --- a/src/mongo/db/catalog/drop_indexes.cpp +++ b/src/mongo/db/catalog/drop_indexes.cpp @@ -196,14 +196,17 @@ Status dropIndexByDescriptor(OperationContext* opCtx, << "can't drop unfinished index with name: " << desc->indexName()); } + // Log the operation first, which reserves an optime in the oplog and sets the timestamp for + // future writes. This guarantees the durable catalog's metadata change to share the same + // timestamp when dropping the index below. + opCtx->getServiceContext()->getOpObserver()->onDropIndex( + opCtx, collection->ns(), collection->uuid(), desc->indexName(), desc->infoObj()); + auto s = indexCatalog->dropIndex(opCtx, desc); if (!s.isOK()) { return s; } - opCtx->getServiceContext()->getOpObserver()->onDropIndex( - opCtx, collection->ns(), collection->uuid(), desc->indexName(), desc->infoObj()); - return Status::OK(); } diff --git a/src/mongo/dbtests/storage_timestamp_tests.cpp b/src/mongo/dbtests/storage_timestamp_tests.cpp index ec4c490b483..0204a577597 100644 --- a/src/mongo/dbtests/storage_timestamp_tests.cpp +++ b/src/mongo/dbtests/storage_timestamp_tests.cpp @@ -33,6 +33,7 @@ #include +#include "mongo/bson/bsonmisc.h" #include "mongo/bson/simple_bsonobj_comparator.h" #include "mongo/bson/timestamp.h" #include "mongo/db/catalog/collection.h" @@ -2540,7 +2541,7 @@ public: } }; -class TimestampIndexDrops : public StorageTimestampTest { +class TimestampIndexDropsWildcard : public StorageTimestampTest { public: void run() { auto storageEngine = _opCtx->getServiceContext()->getStorageEngine(); @@ -2618,6 +2619,85 @@ public: } }; +class TimestampIndexDropsListed : public StorageTimestampTest { +public: + void run() { + auto storageEngine = _opCtx->getServiceContext()->getStorageEngine(); + auto durableCatalog = storageEngine->getCatalog(); + + NamespaceString nss("unittests.timestampIndexDrops"); + reset(nss); + + AutoGetCollection autoColl(_opCtx, nss, LockMode::MODE_X); + + const LogicalTime insertTimestamp = _clock->reserveTicks(1); + { + WriteUnitOfWork wuow(_opCtx); + insertDocument(autoColl.getCollection(), + InsertStatement(BSON("_id" << 0 << "a" << 1 << "b" << 2 << "c" << 3), + insertTimestamp.asTimestamp(), + presentTerm)); + wuow.commit(); + ASSERT_EQ(1, itCount(autoColl.getCollection())); + } + + + const Timestamp beforeIndexBuild = _clock->reserveTicks(1).asTimestamp(); + + // Save the pre-state idents so we can capture the specific ident related to index + // creation. + std::vector origIdents = durableCatalog->getAllIdents(_opCtx); + + std::vector afterCreateTimestamps; + std::vector indexIdents; + // Create an index and get the ident for each index. + for (auto key : {"a", "b", "c"}) { + createIndex(autoColl.getCollection(), str::stream() << key << "_1", BSON(key << 1)); + + // Timestamps at the completion of each index build. + afterCreateTimestamps.push_back(_clock->reserveTicks(1).asTimestamp()); + + // Add the new ident to the vector and reset the current idents. + indexIdents.push_back( + getNewIndexIdentAtTime(durableCatalog, origIdents, Timestamp::min())); + origIdents = durableCatalog->getAllIdents(_opCtx); + } + + // Ensure each index is visible at the correct timestamp, and not before. + for (size_t i = 0; i < indexIdents.size(); i++) { + auto beforeTs = (i == 0) ? beforeIndexBuild : afterCreateTimestamps[i - 1]; + assertIdentsMissingAtTimestamp(durableCatalog, "", indexIdents[i], beforeTs); + assertIdentsExistAtTimestamp( + durableCatalog, "", indexIdents[i], afterCreateTimestamps[i]); + } + + const LogicalTime beforeDropTs = _clock->getClusterTime(); + + // Drop all of the indexes. + BSONObjBuilder result; + ASSERT_OK(dropIndexes(_opCtx, + nss, + BSON("index" << BSON_ARRAY("a_1" + << "b_1" + << "c_1")), + &result)); + + // Assert that each index is dropped individually and with its own timestamp. The order of + // dropping and creating are not guaranteed to be the same, but assert all of the created + // indexes were also dropped. + size_t nIdents = indexIdents.size(); + for (size_t i = 0; i < nIdents; i++) { + OneOffRead oor(_opCtx, beforeDropTs.addTicks(i + 1).asTimestamp()); + + auto ident = getDroppedIndexIdent(durableCatalog, origIdents); + indexIdents.erase(std::remove(indexIdents.begin(), indexIdents.end(), ident)); + + origIdents = durableCatalog->getAllIdents(_opCtx); + } + ASSERT_EQ(indexIdents.size(), 0ul) << "Dropped idents should match created idents"; + } +}; + /** * Test specific OplogApplierImpl subclass that allows for custom applyOplogBatchPerWorker to be run * during multiApply. @@ -3970,7 +4050,8 @@ public: addIf(); addIf(); addIf(); - addIf(); + addIf(); + addIf(); addIf(); addIf(); addIf(); -- cgit v1.2.1