SERVER-34777 Multi-index drops should timestamp each drop individually along with its oplog entry.

6 files changed, 155 insertions, 23 deletions

diff --git a/src/mongo/db/catalog/database_impl.cpp b/src/mongo/db/catalog/database_impl.cpp
index df4b21ae892..06a65e1c4d5 100644
--- a/src/mongo/db/catalog/database_impl.cpp
+++ b/src/mongo/db/catalog/database_impl.cpp
@@ -553,9 +553,11 @@ Status DatabaseImpl::dropCollectionEvenIfSystem(OperationContext* opCtx,
             log() << "dropCollection: " << fullns << " (" << uuidString << ") - index namespace '"
                   << index->indexNamespace()
                   << "' would be too long after drop-pending rename. Dropping index immediately.";
-            fassert(40463, collection->getIndexCatalog()->dropIndex(opCtx, index));
+            // Log the operation before the drop so that each drop is timestamped at the same time
+            // as the oplog entry.
             opObserver->onDropIndex(
                 opCtx, fullns, collection->uuid(), index->indexName(), index->infoObj());
+            fassert(40463, collection->getIndexCatalog()->dropIndex(opCtx, index));
         }
 
         // Log oplog entry for collection drop and proceed to complete rest of two phase drop
diff --git a/src/mongo/db/catalog/drop_indexes.cpp b/src/mongo/db/catalog/drop_indexes.cpp
index 10f78594b51..e6d263deb6f 100644
--- a/src/mongo/db/catalog/drop_indexes.cpp
+++ b/src/mongo/db/catalog/drop_indexes.cpp
@@ -60,14 +60,15 @@ Status wrappedRun(OperationContext* opCtx,
         std::string indexToDelete = f.valuestr();
 
         if (indexToDelete == "*") {
-            std::map<std::string, BSONObj> droppedIndexes;
-            indexCatalog->dropAllIndexes(opCtx, false, &droppedIndexes);
-
-            // We log one op for every dropped index so that we can roll them back if necessary.
-            for (auto const& idx : droppedIndexes) {
-                opCtx->getServiceContext()->getOpObserver()->onDropIndex(
-                    opCtx, collection->ns(), collection->uuid(), idx.first, idx.second);
-            }
+            indexCatalog->dropAllIndexes(
+                opCtx, false, [opCtx, collection](const IndexDescriptor* desc) {
+                    opCtx->getServiceContext()->getOpObserver()->onDropIndex(opCtx,
+                                                                             collection->ns(),
+                                                                             collection->uuid(),
+                                                                             desc->indexName(),
+                                                                             desc->infoObj());
+
+                });
 
             anObjBuilder->append("msg", "non-_id indexes dropped for collection");
             return Status::OK();
diff --git a/src/mongo/db/catalog/index_catalog.h b/src/mongo/db/catalog/index_catalog.h
index bb9902f91cd..171d7ac035c 100644
--- a/src/mongo/db/catalog/index_catalog.h
+++ b/src/mongo/db/catalog/index_catalog.h
@@ -207,9 +207,10 @@ public:
         virtual StatusWith<BSONObj> prepareSpecForCreate(OperationContext* opCtx,
                                                          const BSONObj& original) const = 0;
 
-        virtual void dropAllIndexes(OperationContext* opCtx,
-                                    bool includingIdIndex,
-                                    std::map<std::string, BSONObj>* droppedIndexes) = 0;
+        virtual void dropAllIndexes(
+            OperationContext* opCtx,
+            bool includingIdIndex,
+            stdx::function<void(const IndexDescriptor*)> onDropFn = nullptr) = 0;
 
         virtual Status dropIndex(OperationContext* opCtx, IndexDescriptor* desc) = 0;
 
@@ -448,13 +449,13 @@ public:
 
     /**
      * Drops all indexes in the index catalog, optionally dropping the id index depending on the
-     * 'includingIdIndex' parameter value. If the 'droppedIndexes' parameter is not null,
-     * it is filled with the names and index info of the dropped indexes.
+     * 'includingIdIndex' parameter value. If 'onDropFn' is provided, it will be called before each
+     * index is dropped to allow timestamping each individual drop.
      */
-    inline void dropAllIndexes(OperationContext* const opCtx,
-                               const bool includingIdIndex,
-                               std::map<std::string, BSONObj>* const droppedIndexes = nullptr) {
-        this->_impl().dropAllIndexes(opCtx, includingIdIndex, droppedIndexes);
+    inline void dropAllIndexes(OperationContext* opCtx,
+                               bool includingIdIndex,
+                               stdx::function<void(const IndexDescriptor*)> onDropFn = nullptr) {
+        this->_impl().dropAllIndexes(opCtx, includingIdIndex, onDropFn);
     }
 
     inline Status dropIndex(OperationContext* const opCtx, IndexDescriptor* const desc) {
diff --git a/src/mongo/db/catalog/index_catalog_impl.cpp b/src/mongo/db/catalog/index_catalog_impl.cpp
index fa87c413439..accd0b99539 100644
--- a/src/mongo/db/catalog/index_catalog_impl.cpp
+++ b/src/mongo/db/catalog/index_catalog_impl.cpp
@@ -894,7 +894,7 @@ BSONObj IndexCatalogImpl::getDefaultIdIndexSpec() const {
 
 void IndexCatalogImpl::dropAllIndexes(OperationContext* opCtx,
                                       bool includingIdIndex,
-                                      std::map<std::string, BSONObj>* droppedIndexes) {
+                                      stdx::function<void(const IndexDescriptor*)> onDropFn) {
     invariant(opCtx->lockState()->isCollectionLockedForMode(_collection->ns().toString(), MODE_X));
 
     BackgroundOperation::assertNoBgOpInProgForNs(_collection->ns().ns());
@@ -934,11 +934,13 @@ void IndexCatalogImpl::dropAllIndexes(OperationContext* opCtx,
         LOG(1) << "\t dropAllIndexes dropping: " << desc->toString();
         IndexCatalogEntry* entry = _entries.find(desc);
         invariant(entry);
-        _dropIndex(opCtx, entry).transitional_ignore();
 
-        if (droppedIndexes != nullptr) {
-            droppedIndexes->emplace(desc->indexName(), desc->infoObj());
+        // If the onDrop function creates an oplog entry, it should run first so that the drop is
+        // timestamped at the same optime.
+        if (onDropFn) {
+            onDropFn(desc);
         }
+        _dropIndex(opCtx, entry).transitional_ignore();
     }
 
     // verify state is sane post cleaning
diff --git a/src/mongo/db/catalog/index_catalog_impl.h b/src/mongo/db/catalog/index_catalog_impl.h
index cca465c732e..27aa657318d 100644
--- a/src/mongo/db/catalog/index_catalog_impl.h
+++ b/src/mongo/db/catalog/index_catalog_impl.h
@@ -229,7 +229,7 @@ public:
      */
     void dropAllIndexes(OperationContext* opCtx,
                         bool includingIdIndex,
-                        std::map<std::string, BSONObj>* droppedIndexes = nullptr) override;
+                        stdx::function<void(const IndexDescriptor*)> onDropFn = nullptr) override;
 
     Status dropIndex(OperationContext* opCtx, IndexDescriptor* desc) override;
 
diff --git a/src/mongo/dbtests/storage_timestamp_tests.cpp b/src/mongo/dbtests/storage_timestamp_tests.cpp
index 2926da0f990..ed0a4e3b65e 100644
--- a/src/mongo/dbtests/storage_timestamp_tests.cpp
+++ b/src/mongo/dbtests/storage_timestamp_tests.cpp
@@ -34,6 +34,7 @@
 #include "mongo/bson/timestamp.h"
 #include "mongo/db/catalog/collection.h"
 #include "mongo/db/catalog/drop_database.h"
+#include "mongo/db/catalog/drop_indexes.h"
 #include "mongo/db/catalog/index_catalog.h"
 #include "mongo/db/catalog/index_create.h"
 #include "mongo/db/catalog/uuid_catalog.h"
@@ -218,6 +219,32 @@ public:
         ASSERT_OK(coll->insertDocument(_opCtx, stmt, nullOpDebug, enforceQuota, fromMigrate));
     }
 
+    void createIndex(Collection* coll, std::string indexName, const BSONObj& indexKey) {
+
+        // Build an index.
+        MultiIndexBlock indexer(_opCtx, coll);
+        BSONObj indexInfoObj;
+        {
+            auto swIndexInfoObj = indexer.init({BSON(
+                "v" << 2 << "name" << indexName << "ns" << coll->ns().ns() << "key" << indexKey)});
+            ASSERT_OK(swIndexInfoObj.getStatus());
+            indexInfoObj = std::move(swIndexInfoObj.getValue()[0]);
+        }
+
+        ASSERT_OK(indexer.insertAllDocumentsInCollection());
+
+        {
+            WriteUnitOfWork wuow(_opCtx);
+            // Timestamping index completion. Primaries write an oplog entry.
+            indexer.commit();
+            // The op observer is not called from the index builder, but rather the
+            // `createIndexes` command.
+            _opCtx->getServiceContext()->getOpObserver()->onCreateIndex(
+                _opCtx, coll->ns(), coll->uuid(), indexInfoObj, false);
+            wuow.commit();
+        }
+    }
+
     std::int32_t itCount(Collection* coll) {
         std::uint64_t ret = 0;
         auto cursor = coll->getRecordStore()->getCursor(_opCtx);
@@ -370,6 +397,23 @@ public:
         return collAndIdxIdents[0];
     }
 
+    std::string getDroppedIndexIdent(KVCatalog* kvCatalog, std::vector<std::string>& origIdents) {
+        // Find the collection and index ident by performing a set difference on the original
+        // idents and the current idents.
+        std::vector<std::string> identsWithColl = kvCatalog->getAllIdents(_opCtx);
+        std::sort(origIdents.begin(), origIdents.end());
+        std::sort(identsWithColl.begin(), identsWithColl.end());
+        std::vector<std::string> collAndIdxIdents;
+        std::set_difference(origIdents.begin(),
+                            origIdents.end(),
+                            identsWithColl.begin(),
+                            identsWithColl.end(),
+                            std::back_inserter(collAndIdxIdents));
+
+        ASSERT(collAndIdxIdents.size() == 1) << "Num idents: " << collAndIdxIdents.size();
+        return collAndIdxIdents[0];
+    }
+
     std::tuple<std::string, std::string> getNewCollectionIndexIdent(
         KVCatalog* kvCatalog, std::vector<std::string>& origIdents) {
         // Find the collection and index ident by performing a set difference on the original
@@ -1842,6 +1886,87 @@ public:
     }
 };
 
+class TimestampIndexDrops : public StorageTimestampTest {
+public:
+    void run() {
+        // Only run on 'wiredTiger'. No other storage engines to-date support timestamp writes.
+        if (mongo::storageGlobalParams.engine != "wiredTiger") {
+            return;
+        }
+        auto kvStorageEngine =
+            dynamic_cast<KVStorageEngine*>(_opCtx->getServiceContext()->getStorageEngine());
+        KVCatalog* kvCatalog = kvStorageEngine->getCatalog();
+
+        NamespaceString nss("unittests.timestampIndexDrops");
+        reset(nss);
+
+        AutoGetCollection autoColl(_opCtx, nss, LockMode::MODE_X, 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(),
+                                           0LL));
+            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<std::string> origIdents = kvCatalog->getAllIdents(_opCtx);
+
+        std::vector<Timestamp> afterCreateTimestamps;
+        std::vector<std::string> 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(getNewIndexIdent(kvCatalog, origIdents));
+            origIdents = kvCatalog->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(kvCatalog, "", indexIdents[i], beforeTs);
+            assertIdentsExistAtTimestamp(kvCatalog, "", indexIdents[i], afterCreateTimestamps[i]);
+        }
+
+        const LogicalTime beforeDropTs = _clock->getClusterTime();
+
+        // Drop all of the indexes.
+        BSONObjBuilder result;
+        ASSERT_OK(dropIndexes(_opCtx,
+                              nss,
+                              BSON("index"
+                                   << "*"),
+                              &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(kvCatalog, origIdents);
+            indexIdents.erase(std::remove(indexIdents.begin(), indexIdents.end(), ident));
+
+            origIdents = kvCatalog->getAllIdents(_opCtx);
+        }
+        ASSERT_EQ(indexIdents.size(), 0ul) << "Dropped idents should match created idents";
+    }
+};
+
 class SecondaryReadsDuringBatchApplicationAreAllowed : public StorageTimestampTest {
 public:
     void run() {
@@ -2077,6 +2202,7 @@ public:
         // TimestampIndexBuilds<SimulatePrimary>
         add<TimestampIndexBuilds<false>>();
         add<TimestampIndexBuilds<true>>();
+        add<TimestampIndexDrops>();
         // TimestampIndexBuilderOnPrimary<Background>
         add<TimestampIndexBuilderOnPrimary<false>>();
         add<TimestampIndexBuilderOnPrimary<true>>();