summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorWilliam Schultz <william.schultz@mongodb.com>2017-10-24 18:02:30 -0400
committerWilliam Schultz <william.schultz@mongodb.com>2017-10-24 18:09:42 -0400
commita4a94724dc82af8a314f98c2245d4e61233f56bf (patch)
treebdb631abdfd525963b46c7d4cff0788aa756eb9b
parent18007ba100d8716fea8925d373d61cb762a1e989 (diff)
downloadmongo-a4a94724dc82af8a314f98c2245d4e61233f56bf.tar.gz
SERVER-31047 Rollback properly removes redundant index operations
-rw-r--r--src/mongo/db/repl/rs_rollback.cpp50
-rw-r--r--src/mongo/db/repl/rs_rollback.h7
-rw-r--r--src/mongo/db/repl/rs_rollback_test.cpp300
3 files changed, 274 insertions, 83 deletions
diff --git a/src/mongo/db/repl/rs_rollback.cpp b/src/mongo/db/repl/rs_rollback.cpp
index b54eea9a7d8..f8ca535773d 100644
--- a/src/mongo/db/repl/rs_rollback.cpp
+++ b/src/mongo/db/repl/rs_rollback.cpp
@@ -122,19 +122,47 @@ void FixUpInfo::removeRedundantOperations() {
}
bool FixUpInfo::removeRedundantIndexCommands(UUID uuid, std::string indexName) {
+ LOG(2) << "Attempting to remove redundant index operations from the set of indexes to create "
+ "for collection "
+ << uuid << ", for index '" << indexName << "'";
+
+ // See if there are any indexes to create for this collection.
auto indexes = indexesToCreate.find(uuid);
- if (indexes != indexesToCreate.end()) {
- invariant(indexesToCreate.count(uuid) == 1);
- invariant((*indexes).second.count(indexName) == 1);
- (*indexes).second.erase(indexName);
- if ((*indexes).second.empty()) {
- indexesToCreate.erase(uuid);
- }
- log() << "Index " << indexName
- << " was previously dropped. The createIndexes command is canceled out.";
- return true;
+
+ // There are no indexes to create for this collection UUID, so there are no index creation
+ // operations to remove.
+ if (indexes == indexesToCreate.end()) {
+ LOG(2)
+ << "Collection " << uuid
+ << " has no indexes to create. Not removing any index creation operations for index '"
+ << indexName << "'.";
+ return false;
+ }
+
+ // This is the set of all indexes to create for the given collection UUID. Keep a reference so
+ // we can modify the original object.
+ std::map<std::string, BSONObj>* indexesToCreateForColl = &(indexes->second);
+
+ // If this index was not previously added to the set of indexes that need to be created for this
+ // collection, then we do nothing.
+ if (indexesToCreateForColl->find(indexName) == indexesToCreateForColl->end()) {
+ LOG(2) << "Index '" << indexName << "' was not previously set to be created for collection "
+ << uuid << ". Not removing any index creation operations.";
+ return false;
}
- return false;
+
+ // This index was previously added to the set of indexes to create for this collection, so we
+ // remove it from that set.
+ LOG(2) << "Index '" << indexName << "' was previously set to be created for collection " << uuid
+ << ". Removing this redundant index creation operation.";
+ indexesToCreateForColl->erase(indexName);
+ // If there are now no remaining indexes to create for this collection, remove it from
+ // the set of collections that we need to create indexes for.
+ if (indexesToCreateForColl->empty()) {
+ indexesToCreate.erase(uuid);
+ }
+
+ return true;
}
void FixUpInfo::recordRollingBackDrop(const NamespaceString& nss, OpTime opTime, UUID uuid) {
diff --git a/src/mongo/db/repl/rs_rollback.h b/src/mongo/db/repl/rs_rollback.h
index 265e3104a62..fade3074331 100644
--- a/src/mongo/db/repl/rs_rollback.h
+++ b/src/mongo/db/repl/rs_rollback.h
@@ -312,10 +312,9 @@ struct FixUpInfo {
void removeRedundantOperations();
/**
- * Removes any redundant index commands. An example is if we create
- * an index with name "a_1" and then later proceed to drop that index.
- * We return true if a redundant index command was found and false
- * if it was not.
+ * Removes any redundant index commands. For example, if we create an index with name "a_1" and
+ * then later proceed to drop that index, we can ignore the first index creation. We return true
+ * if a redundant index command was removed and false if it was not.
*/
bool removeRedundantIndexCommands(UUID uuid, std::string indexName);
diff --git a/src/mongo/db/repl/rs_rollback_test.cpp b/src/mongo/db/repl/rs_rollback_test.cpp
index ad413f2db21..dd849c9d1a8 100644
--- a/src/mongo/db/repl/rs_rollback_test.cpp
+++ b/src/mongo/db/repl/rs_rollback_test.cpp
@@ -182,16 +182,13 @@ OplogInterfaceMock::Operation makeCreateIndexOplogEntry(Collection* collection,
BSONObj key,
std::string indexName,
int time) {
- auto indexSpec = BSON("createIndexes"
- << "t"
- << "ns"
- << collection->ns().ns()
- << "v"
- << static_cast<int>(kIndexVersion)
- << "key"
- << key
- << "name"
- << indexName);
+ auto indexSpec =
+ BSON("createIndexes" << collection->ns().coll() << "ns" << collection->ns().ns() << "v"
+ << static_cast<int>(kIndexVersion)
+ << "key"
+ << key
+ << "name"
+ << indexName);
return std::make_pair(BSON("ts" << Timestamp(Seconds(time), 0) << "h" << 1LL << "op"
<< "c"
@@ -234,6 +231,23 @@ OplogInterfaceMock::Operation makeRenameCollectionOplogEntry(const NamespaceStri
RecordId(opTime.getTimestamp().getSecs()));
}
+// Create an index on the given collection. Returns the number of indexes that exist on the
+// collection after the given index is created.
+int createIndexForColl(OperationContext* opCtx,
+ Collection* coll,
+ NamespaceString nss,
+ BSONObj indexSpec) {
+ Lock::DBLock dbLock(opCtx, nss.db(), MODE_X);
+ MultiIndexBlock indexer(opCtx, coll);
+ ASSERT_OK(indexer.init(indexSpec).getStatus());
+ WriteUnitOfWork wunit(opCtx);
+ indexer.commit();
+ wunit.commit();
+ auto indexCatalog = coll->getIndexCatalog();
+ ASSERT(indexCatalog);
+ return indexCatalog->numIndexesReady(opCtx);
+}
+
TEST_F(RSRollbackTest, InconsistentMinValid) {
_replicationProcess->getConsistencyMarkers()->setAppliedThrough(
_opCtx.get(), OpTime(Timestamp(Seconds(0), 0), 0));
@@ -523,26 +537,15 @@ TEST_F(RSRollbackTest, RollbackCreateIndexCommand) {
createOplog(_opCtx.get());
CollectionOptions options;
options.uuid = UUID::gen();
- auto collection = _createCollection(_opCtx.get(), "test.t", options);
- auto indexSpec = BSON("ns"
- << "test.t"
- << "v"
- << static_cast<int>(kIndexVersion)
- << "key"
- << BSON("a" << 1)
- << "name"
- << "a_1");
- {
- Lock::DBLock dbLock(_opCtx.get(), "test", MODE_X);
- MultiIndexBlock indexer(_opCtx.get(), collection);
- ASSERT_OK(indexer.init(indexSpec).getStatus());
- WriteUnitOfWork wunit(_opCtx.get());
- indexer.commit();
- wunit.commit();
- auto indexCatalog = collection->getIndexCatalog();
- ASSERT(indexCatalog);
- ASSERT_EQUALS(2, indexCatalog->numIndexesReady(_opCtx.get()));
- }
+ NamespaceString nss("test", "coll");
+ auto collection = _createCollection(_opCtx.get(), nss.toString(), options);
+ auto indexSpec = BSON("ns" << nss.toString() << "v" << static_cast<int>(kIndexVersion) << "key"
+ << BSON("a" << 1)
+ << "name"
+ << "a_1");
+
+ int numIndexes = createIndexForColl(_opCtx.get(), collection, nss, indexSpec);
+ ASSERT_EQUALS(2, numIndexes);
auto commonOperation =
std::make_pair(BSON("ts" << Timestamp(Seconds(1), 0) << "h" << 1LL), RecordId(1));
@@ -564,12 +567,14 @@ TEST_F(RSRollbackTest, RollbackCreateIndexCommand) {
_replicationProcess.get()));
stopCapturingLogMessages();
ASSERT_EQUALS(1,
- countLogLinesContaining(
- str::stream() << "Dropped index in rollback for collection: test.t, UUID: "
- << options.uuid->toString()
- << ", index: a_1"));
+ countLogLinesContaining(str::stream()
+ << "Dropped index in rollback for collection: "
+ << nss.toString()
+ << ", UUID: "
+ << options.uuid->toString()
+ << ", index: a_1"));
{
- Lock::DBLock dbLock(_opCtx.get(), "test", MODE_S);
+ Lock::DBLock dbLock(_opCtx.get(), nss.db(), MODE_S);
auto indexCatalog = collection->getIndexCatalog();
ASSERT(indexCatalog);
ASSERT_EQUALS(1, indexCatalog->numIndexesReady(_opCtx.get()));
@@ -733,27 +738,16 @@ TEST_F(RSRollbackTest, RollingBackDropAndCreateOfSameIndexNameWithDifferentSpecs
createOplog(_opCtx.get());
CollectionOptions options;
options.uuid = UUID::gen();
- auto collection = _createCollection(_opCtx.get(), "test.t", options);
+ NamespaceString nss("test", "coll");
+ auto collection = _createCollection(_opCtx.get(), nss.toString(), options);
- auto indexSpec = BSON("ns"
- << "test.t"
- << "v"
- << static_cast<int>(kIndexVersion)
- << "key"
- << BSON("b" << 1)
- << "name"
- << "a_1");
- {
- Lock::DBLock dbLock(_opCtx.get(), "test", MODE_X);
- MultiIndexBlock indexer(_opCtx.get(), collection);
- ASSERT_OK(indexer.init(indexSpec).getStatus());
- WriteUnitOfWork wunit(_opCtx.get());
- indexer.commit();
- wunit.commit();
- auto indexCatalog = collection->getIndexCatalog();
- ASSERT(indexCatalog);
- ASSERT_EQUALS(2, indexCatalog->numIndexesReady(_opCtx.get()));
- }
+ auto indexSpec = BSON("ns" << nss.toString() << "v" << static_cast<int>(kIndexVersion) << "key"
+ << BSON("b" << 1)
+ << "name"
+ << "a_1");
+
+ int numIndexes = createIndexForColl(_opCtx.get(), collection, nss, indexSpec);
+ ASSERT_EQUALS(2, numIndexes);
auto commonOperation =
std::make_pair(BSON("ts" << Timestamp(Seconds(1), 0) << "h" << 1LL), RecordId(1));
@@ -776,22 +770,24 @@ TEST_F(RSRollbackTest, RollingBackDropAndCreateOfSameIndexNameWithDifferentSpecs
_replicationProcess.get()));
stopCapturingLogMessages();
{
- Lock::DBLock dbLock(_opCtx.get(), "test", MODE_S);
+ Lock::DBLock dbLock(_opCtx.get(), nss.db(), MODE_S);
auto indexCatalog = collection->getIndexCatalog();
ASSERT(indexCatalog);
ASSERT_EQUALS(2, indexCatalog->numIndexesReady(_opCtx.get()));
- ASSERT_EQUALS(
- 1,
- countLogLinesContaining(str::stream()
- << "Dropped index in rollback for collection: test.t, UUID: "
- << options.uuid->toString()
- << ", index: a_1"));
- ASSERT_EQUALS(
- 1,
- countLogLinesContaining(str::stream()
- << "Created index in rollback for collection: test.t, UUID: "
- << options.uuid->toString()
- << ", index: a_1"));
+ ASSERT_EQUALS(1,
+ countLogLinesContaining(str::stream()
+ << "Dropped index in rollback for collection: "
+ << nss.toString()
+ << ", UUID: "
+ << options.uuid->toString()
+ << ", index: a_1"));
+ ASSERT_EQUALS(1,
+ countLogLinesContaining(str::stream()
+ << "Created index in rollback for collection: "
+ << nss.toString()
+ << ", UUID: "
+ << options.uuid->toString()
+ << ", index: a_1"));
std::vector<IndexDescriptor*> indexes;
indexCatalog->findIndexesByKeyPattern(_opCtx.get(), BSON("a" << 1), false, &indexes);
ASSERT(indexes.size() == 1);
@@ -847,6 +843,174 @@ TEST_F(RSRollbackTest, RollbackCreateIndexCommandMissingIndexName) {
"Missing index name in createIndexes operation on rollback, document: "));
}
+// Generators of standard index keys and names given an index 'id'.
+std::string idxKey(std::string id) {
+ return "key_" + id;
+};
+std::string idxName(std::string id) {
+ return "index_" + id;
+};
+
+// Create an index spec object given the namespace and the index 'id'.
+BSONObj idxSpec(NamespaceString nss, std::string id) {
+ return BSON("ns" << nss.toString() << "v" << static_cast<int>(kIndexVersion) << "key"
+ << BSON(idxKey(id) << 1)
+ << "name"
+ << idxName(id));
+}
+
+// Returns the number of indexes that exist on the given collection.
+int numIndexesOnColl(OperationContext* opCtx, NamespaceString nss, Collection* coll) {
+ Lock::DBLock dbLock(opCtx, nss.db(), MODE_X);
+ auto indexCatalog = coll->getIndexCatalog();
+ ASSERT(indexCatalog);
+ return indexCatalog->numIndexesReady(opCtx);
+}
+
+TEST_F(RSRollbackTest, RollbackDropIndexOnCollectionWithTwoExistingIndexes) {
+ createOplog(_opCtx.get());
+ CollectionOptions options;
+ options.uuid = UUID::gen();
+ NamespaceString nss("test", "coll");
+ auto coll = _createCollection(_opCtx.get(), nss.toString(), options);
+
+ // Create the necessary indexes. Index 0 is created and dropped in the sequence of ops that will
+ // be rolled back, so we only create index 1.
+ int numIndexes = createIndexForColl(_opCtx.get(), coll, nss, idxSpec(nss, "1"));
+ ASSERT_EQUALS(2, numIndexes);
+
+ auto commonOp = std::make_pair(BSON("ts" << Timestamp(1, 0) << "h" << 1LL), RecordId(1));
+
+ // The ops that will be rolled back.
+ auto createIndex0Op = makeCreateIndexOplogEntry(coll, BSON(idxKey("0") << 1), idxName("0"), 2);
+ auto createIndex1Op = makeCreateIndexOplogEntry(coll, BSON(idxKey("1") << 1), idxName("1"), 3);
+ auto dropIndex0Op = makeDropIndexOplogEntry(coll, BSON(idxKey("0") << 1), idxName("0"), 4);
+
+ auto remoteOplog = {commonOp};
+ auto localOplog = {dropIndex0Op, createIndex1Op, createIndex0Op, commonOp};
+
+ // Set up the mock rollback source and then run rollback.
+ RollbackSourceMock rollbackSource(stdx::make_unique<OplogInterfaceMock>(remoteOplog));
+ ASSERT_OK(syncRollback(_opCtx.get(),
+ OplogInterfaceMock(localOplog),
+ rollbackSource,
+ {},
+ _coordinator,
+ _replicationProcess.get()));
+
+ // Make sure the collection indexes are in the proper state post-rollback.
+ ASSERT_EQUALS(1, numIndexesOnColl(_opCtx.get(), nss, coll));
+}
+
+TEST_F(RSRollbackTest, RollbackTwoIndexDropsPrecededByTwoIndexCreationsOnSameCollection) {
+ createOplog(_opCtx.get());
+ CollectionOptions options;
+ options.uuid = UUID::gen();
+ NamespaceString nss("test", "coll");
+ auto coll = _createCollection(_opCtx.get(), nss.toString(), options);
+
+ auto commonOp = std::make_pair(BSON("ts" << Timestamp(1, 0) << "h" << 1LL), RecordId(1));
+
+ // The ops that will be rolled back.
+ auto createIndex0Op = makeCreateIndexOplogEntry(coll, BSON(idxKey("0") << 1), idxName("0"), 2);
+ auto createIndex1Op = makeCreateIndexOplogEntry(coll, BSON(idxKey("1") << 1), idxName("1"), 3);
+ auto dropIndex0Op = makeDropIndexOplogEntry(coll, BSON(idxKey("0") << 1), idxName("0"), 4);
+ auto dropIndex1Op = makeDropIndexOplogEntry(coll, BSON(idxKey("1") << 1), idxName("1"), 5);
+
+ auto remoteOplog = {commonOp};
+ auto localOplog = {dropIndex1Op, dropIndex0Op, createIndex1Op, createIndex0Op, commonOp};
+
+ // Set up the mock rollback source and then run rollback.
+ RollbackSourceMock rollbackSource(stdx::make_unique<OplogInterfaceMock>(remoteOplog));
+ ASSERT_OK(syncRollback(_opCtx.get(),
+ OplogInterfaceMock(localOplog),
+ rollbackSource,
+ {},
+ _coordinator,
+ _replicationProcess.get()));
+
+ // Make sure the collection indexes are in the proper state post-rollback.
+ ASSERT_EQUALS(1, numIndexesOnColl(_opCtx.get(), nss, coll));
+}
+
+TEST_F(RSRollbackTest, RollbackMultipleCreateIndexesOnSameCollection) {
+ createOplog(_opCtx.get());
+ CollectionOptions options;
+ options.uuid = UUID::gen();
+ NamespaceString nss("test", "coll");
+ auto coll = _createCollection(_opCtx.get(), nss.toString(), options);
+
+ auto commonOp = std::make_pair(BSON("ts" << Timestamp(1, 0) << "h" << 1LL), RecordId(1));
+
+ // Create all of the necessary indexes.
+ createIndexForColl(_opCtx.get(), coll, nss, idxSpec(nss, "0"));
+ createIndexForColl(_opCtx.get(), coll, nss, idxSpec(nss, "1"));
+ createIndexForColl(_opCtx.get(), coll, nss, idxSpec(nss, "2"));
+ ASSERT_EQUALS(4, numIndexesOnColl(_opCtx.get(), nss, coll));
+
+ // The ops that will be rolled back.
+ auto createIndex0Op = makeCreateIndexOplogEntry(coll, BSON(idxKey("0") << 1), idxName("0"), 2);
+ auto createIndex1Op = makeCreateIndexOplogEntry(coll, BSON(idxKey("1") << 1), idxName("1"), 3);
+ auto createIndex2Op = makeCreateIndexOplogEntry(coll, BSON(idxKey("2") << 1), idxName("2"), 4);
+
+ auto remoteOplog = {commonOp};
+ auto localOplog = {createIndex2Op, createIndex1Op, createIndex0Op, commonOp};
+
+ // Set up the mock rollback source and then run rollback.
+ RollbackSourceMock rollbackSource(stdx::make_unique<OplogInterfaceMock>(remoteOplog));
+ ASSERT_OK(syncRollback(_opCtx.get(),
+ OplogInterfaceMock(localOplog),
+ rollbackSource,
+ {},
+ _coordinator,
+ _replicationProcess.get()));
+
+ // Make sure the collection indexes are in the proper state post-rollback.
+ ASSERT_EQUALS(1, numIndexesOnColl(_opCtx.get(), nss, coll));
+}
+
+TEST_F(RSRollbackTest, RollbackCreateDropRecreateIndexOnCollection) {
+ createOplog(_opCtx.get());
+ CollectionOptions options;
+ options.uuid = UUID::gen();
+ NamespaceString nss("test", "coll");
+ auto coll = _createCollection(_opCtx.get(), nss.toString(), options);
+
+ // Create the necessary indexes. Index 0 is created, dropped, and created again in the
+ // sequence of ops, so we create that index.
+ auto indexSpec = BSON("ns" << nss.toString() << "v" << static_cast<int>(kIndexVersion) << "key"
+ << BSON(idxKey("0") << 1)
+ << "name"
+ << idxName("0"));
+
+ int numIndexes = createIndexForColl(_opCtx.get(), coll, nss, indexSpec);
+ ASSERT_EQUALS(2, numIndexes);
+
+ auto commonOp = std::make_pair(BSON("ts" << Timestamp(1, 0) << "h" << 1LL), RecordId(1));
+
+ // The ops that will be rolled back.
+ auto createIndex0Op = makeCreateIndexOplogEntry(coll, BSON(idxKey("0") << 1), idxName("0"), 2);
+ auto dropIndex0Op = makeDropIndexOplogEntry(coll, BSON(idxKey("0") << 1), idxName("0"), 3);
+ auto createIndex0AgainOp =
+ makeCreateIndexOplogEntry(coll, BSON(idxKey("0") << 1), idxName("0"), 4);
+
+ auto remoteOplog = {commonOp};
+ auto localOplog = {createIndex0AgainOp, dropIndex0Op, createIndex0Op, commonOp};
+
+ // Set up the mock rollback source and then run rollback.
+ RollbackSourceMock rollbackSource(stdx::make_unique<OplogInterfaceMock>(remoteOplog));
+ ASSERT_OK(syncRollback(_opCtx.get(),
+ OplogInterfaceMock(localOplog),
+ rollbackSource,
+ {},
+ _coordinator,
+ _replicationProcess.get()));
+
+ // Make sure the collection indexes are in the proper state post-rollback.
+ ASSERT_EQUALS(1, numIndexesOnColl(_opCtx.get(), nss, coll));
+}
+
+
TEST_F(RSRollbackTest, RollbackUnknownCommand) {
createOplog(_opCtx.get());
auto commonOperation =