diff options
author | Jack Mulrow <jack.mulrow@mongodb.com> | 2017-08-16 18:07:02 -0400 |
---|---|---|
committer | Jack Mulrow <jack.mulrow@mongodb.com> | 2017-08-23 12:41:47 -0400 |
commit | c1e7921e9d69bd9a37761deb58d119a324341a54 (patch) | |
tree | 86e7fb3084d9aea2f08263f261474c1163de15cc | |
parent | ee6a79935e98b4a12bc74cb385e7d5f62633347e (diff) | |
download | mongo-c1e7921e9d69bd9a37761deb58d119a324341a54.tar.gz |
SERVER-30508 Fail rollback via refetch if transactions collection UUID is different on sync source
-rw-r--r-- | jstests/replsets/rollback_transaction_table.js | 7 | ||||
-rw-r--r-- | src/mongo/client/dbclient.cpp | 9 | ||||
-rw-r--r-- | src/mongo/client/dbclientinterface.h | 8 | ||||
-rw-r--r-- | src/mongo/db/repl/rollback_source.h | 9 | ||||
-rw-r--r-- | src/mongo/db/repl/rollback_source_impl.cpp | 6 | ||||
-rw-r--r-- | src/mongo/db/repl/rollback_source_impl.h | 4 | ||||
-rw-r--r-- | src/mongo/db/repl/rs_rollback.cpp | 186 | ||||
-rw-r--r-- | src/mongo/db/repl/rs_rollback.h | 13 | ||||
-rw-r--r-- | src/mongo/db/repl/rs_rollback_no_uuid_test.cpp | 12 | ||||
-rw-r--r-- | src/mongo/db/repl/rs_rollback_test.cpp | 116 |
10 files changed, 256 insertions, 114 deletions
diff --git a/jstests/replsets/rollback_transaction_table.js b/jstests/replsets/rollback_transaction_table.js index e18a209eedb..9e0ca77a6d4 100644 --- a/jstests/replsets/rollback_transaction_table.js +++ b/jstests/replsets/rollback_transaction_table.js @@ -65,6 +65,13 @@ jsTestLog("Making sure 'downstream node' is the primary node."); assert.eq(downstream, replTest.getPrimary()); + // Renaming or dropping the transactions collection shouldn't crash if command is not rolled + // back. + assert.commandWorked(downstream.getDB("config").transactions.renameCollection("foo")); + assert.commandWorked(downstream.getDB("config").foo.renameCollection("transactions")); + assert(downstream.getDB("config").transactions.drop()); + assert.commandWorked(downstream.getDB("config").createCollection("transactions")); + jsTestLog("Running a transaction on the 'downstream node' and waiting for it to replicate."); let firstLsid = {id: UUID()}; let firstCmd = { diff --git a/src/mongo/client/dbclient.cpp b/src/mongo/client/dbclient.cpp index 3c42efc050b..71087890376 100644 --- a/src/mongo/client/dbclient.cpp +++ b/src/mongo/client/dbclient.cpp @@ -684,7 +684,9 @@ BSONObj DBClientBase::findOne(const string& ns, return v.empty() ? BSONObj() : v[0]; } -BSONObj DBClientBase::findOneByUUID(const std::string& db, UUID uuid, const BSONObj& filter) { +std::pair<BSONObj, NamespaceString> DBClientBase::findOneByUUID(const std::string& db, + UUID uuid, + const BSONObj& filter) { list<BSONObj> results; BSONObj res; @@ -705,10 +707,11 @@ BSONObj DBClientBase::findOneByUUID(const std::string& db, UUID uuid, const BSON results.push_back(e.Obj().getOwned()); } invariant(results.size() <= 1); + NamespaceString resNss(cursorObj["ns"].valueStringData()); if (results.empty()) { - return BSONObj(); + return {BSONObj(), resNss}; } - return results.front(); + return {results.front(), resNss}; } uasserted( 40586, diff --git a/src/mongo/client/dbclientinterface.h b/src/mongo/client/dbclientinterface.h index 06494f61e8c..85b1006f72d 100644 --- a/src/mongo/client/dbclientinterface.h +++ b/src/mongo/client/dbclientinterface.h @@ -199,12 +199,16 @@ public: int queryOptions = 0); /** - * @return a single object that matches the filter within the collection specified by the UUID. + * @return a pair with a single object that matches the filter within the collection specified + * by the UUID and the namespace of that collection on the queried node. + * * If the command fails, an assertion error is thrown. Otherwise, if no document matches * the query, an empty BSONObj is returned. * @throws AssertionException */ - virtual BSONObj findOneByUUID(const std::string& db, UUID uuid, const BSONObj& filter); + virtual std::pair<BSONObj, NamespaceString> findOneByUUID(const std::string& db, + UUID uuid, + const BSONObj& filter); virtual std::string getServerAddress() const = 0; diff --git a/src/mongo/db/repl/rollback_source.h b/src/mongo/db/repl/rollback_source.h index 0c55bcb9ae2..e3379135aaf 100644 --- a/src/mongo/db/repl/rollback_source.h +++ b/src/mongo/db/repl/rollback_source.h @@ -81,11 +81,12 @@ public: virtual BSONObj findOne(const NamespaceString& nss, const BSONObj& filter) const = 0; /** - * Fetch a single document from the sync source using the UUID. + * Fetch a single document from the sync source using the UUID. Returns the namespace matching + * the UUID on the sync source as well. */ - virtual BSONObj findOneByUUID(const std::string& db, - UUID uuid, - const BSONObj& filter) const = 0; + virtual std::pair<BSONObj, NamespaceString> findOneByUUID(const std::string& db, + UUID uuid, + const BSONObj& filter) const = 0; /** * Clones a single collection from the sync source. diff --git a/src/mongo/db/repl/rollback_source_impl.cpp b/src/mongo/db/repl/rollback_source_impl.cpp index 3e05e8a41be..f0f5667c25d 100644 --- a/src/mongo/db/repl/rollback_source_impl.cpp +++ b/src/mongo/db/repl/rollback_source_impl.cpp @@ -73,9 +73,9 @@ BSONObj RollbackSourceImpl::findOne(const NamespaceString& nss, const BSONObj& f return _getConnection()->findOne(nss.toString(), filter, NULL, QueryOption_SlaveOk).getOwned(); } -BSONObj RollbackSourceImpl::findOneByUUID(const std::string& db, - UUID uuid, - const BSONObj& filter) const { +std::pair<BSONObj, NamespaceString> RollbackSourceImpl::findOneByUUID(const std::string& db, + UUID uuid, + const BSONObj& filter) const { return _getConnection()->findOneByUUID(db, uuid, filter); } diff --git a/src/mongo/db/repl/rollback_source_impl.h b/src/mongo/db/repl/rollback_source_impl.h index 1654194ea5d..dcb6252ddbb 100644 --- a/src/mongo/db/repl/rollback_source_impl.h +++ b/src/mongo/db/repl/rollback_source_impl.h @@ -66,7 +66,9 @@ public: BSONObj findOne(const NamespaceString& nss, const BSONObj& filter) const override; - BSONObj findOneByUUID(const std::string& db, UUID uuid, const BSONObj& filter) const override; + std::pair<BSONObj, NamespaceString> findOneByUUID(const std::string& db, + UUID uuid, + const BSONObj& filter) const override; void copyCollectionFromRemote(OperationContext* opCtx, const NamespaceString& nss) const override; diff --git a/src/mongo/db/repl/rs_rollback.cpp b/src/mongo/db/repl/rs_rollback.cpp index 8daad87bd8b..7b16265bcb8 100644 --- a/src/mongo/db/repl/rs_rollback.cpp +++ b/src/mongo/db/repl/rs_rollback.cpp @@ -869,11 +869,88 @@ void rollbackRenameCollection(OperationContext* opCtx, UUID uuid, RenameCollecti << " with uuid: " << uuid; } -void syncFixUp(OperationContext* opCtx, - const FixUpInfo& fixUpInfo, - const RollbackSource& rollbackSource, - ReplicationCoordinator* replCoord, - ReplicationProcess* replicationProcess) { +Status _syncRollback(OperationContext* opCtx, + const OplogInterface& localOplog, + const RollbackSource& rollbackSource, + int requiredRBID, + ReplicationCoordinator* replCoord, + ReplicationProcess* replicationProcess) { + invariant(!opCtx->lockState()->isLocked()); + + FixUpInfo how; + log() << "Starting rollback. Sync source: " << rollbackSource.getSource() << rsLog; + how.rbid = rollbackSource.getRollbackId(); + uassert( + 40506, "Upstream node rolled back. Need to retry our rollback.", how.rbid == requiredRBID); + + // Find the UUID of the transactions collection. An OperationContext is required because the + // UUID is not known at compile time, so the SessionCatalog needs to load the collection. + how.transactionTableUUID = SessionCatalog::getTransactionTableUUID(opCtx); + + log() << "Finding the Common Point"; + try { + + auto processOperationForFixUp = [&how](const BSONObj& operation) { + return updateFixUpInfoFromLocalOplogEntry(how, operation); + }; + + // Calls syncRollBackLocalOperations to run updateFixUpInfoFromLocalOplogEntry + // on each oplog entry up until the common point. + auto res = syncRollBackLocalOperations( + localOplog, rollbackSource.getOplog(), processOperationForFixUp); + if (!res.isOK()) { + const auto status = res.getStatus(); + switch (status.code()) { + case ErrorCodes::OplogStartMissing: + case ErrorCodes::UnrecoverableRollbackError: + return status; + default: + throw RSFatalException(status.toString()); + } + } + + how.commonPoint = res.getValue().first; // OpTime + how.commonPointOurDiskloc = res.getValue().second; // RecordID + how.removeRedundantOperations(); + } catch (const RSFatalException& e) { + return Status(ErrorCodes::UnrecoverableRollbackError, + str::stream() + << "need to rollback, but unable to determine common point between" + " local and remote oplog: " + << e.what()); + } + + log() << "Rollback common point is " << how.commonPoint; + try { + ON_BLOCK_EXIT([&] { + auto status = replicationProcess->incrementRollbackID(opCtx); + fassertStatusOK(40497, status); + }); + syncFixUp(opCtx, how, rollbackSource, replCoord, replicationProcess); + } catch (const RSFatalException& e) { + return Status(ErrorCodes::UnrecoverableRollbackError, e.what()); + } + + if (MONGO_FAIL_POINT(rollbackHangBeforeFinish)) { + // This log output is used in js tests so please leave it. + log() << "rollback - rollbackHangBeforeFinish fail point " + "enabled. Blocking until fail point is disabled."; + while (MONGO_FAIL_POINT(rollbackHangBeforeFinish)) { + invariant(!globalInShutdownDeprecated()); // It is an error to shutdown while enabled. + mongo::sleepsecs(1); + } + } + + return Status::OK(); +} + +} // namespace + +void rollback_internal::syncFixUp(OperationContext* opCtx, + const FixUpInfo& fixUpInfo, + const RollbackSource& rollbackSource, + ReplicationCoordinator* replCoord, + ReplicationProcess* replicationProcess) { unsigned long long totalSize = 0; // UUID -> doc id -> doc @@ -896,7 +973,27 @@ void syncFixUp(OperationContext* opCtx, << ", _id: " << redact(doc._id); // TODO : Slow. Lots of round trips. numFetched++; - BSONObj good = rollbackSource.findOneByUUID(nss.db().toString(), uuid, doc._id.wrap()); + + BSONObj good; + NamespaceString resNss; + std::tie(good, resNss) = + rollbackSource.findOneByUUID(nss.db().toString(), uuid, doc._id.wrap()); + + // To prevent inconsistencies in the transactions collection, rollback fails if the UUID + // of the collection is different on the sync source than on the node rolling back, + // forcing an initial sync. This is detected if the returned namespace for a refetch of + // a transaction table document is not "config.transactions," which implies a rename or + // drop of the collection occured on either node. + if (uuid == fixUpInfo.transactionTableUUID && + resNss != NamespaceString::kSessionTransactionsTableNamespace) { + throw RSFatalException( + str::stream() + << "A fetch on the transactions collection returned an unexpected namespace: " + << resNss.ns() + << ". The transactions collection cannot be correctly rolled back, a full " + "resync is required."); + } + totalSize += good.objsize(); // Checks that the total amount of data that needs to be refetched is at most @@ -1298,83 +1395,6 @@ void syncFixUp(OperationContext* opCtx, replCoord->resetLastOpTimesFromOplog(opCtx); } -Status _syncRollback(OperationContext* opCtx, - const OplogInterface& localOplog, - const RollbackSource& rollbackSource, - int requiredRBID, - ReplicationCoordinator* replCoord, - ReplicationProcess* replicationProcess) { - invariant(!opCtx->lockState()->isLocked()); - - FixUpInfo how; - log() << "Starting rollback. Sync source: " << rollbackSource.getSource() << rsLog; - how.rbid = rollbackSource.getRollbackId(); - uassert( - 40506, "Upstream node rolled back. Need to retry our rollback.", how.rbid == requiredRBID); - - // Find the UUID of the transactions collection. An OperationContext is required because the - // UUID is not known at compile time, so the SessionCatalog needs to load the collection. - how.transactionTableUUID = SessionCatalog::getTransactionTableUUID(opCtx); - - log() << "Finding the Common Point"; - try { - - auto processOperationForFixUp = [&how](const BSONObj& operation) { - return updateFixUpInfoFromLocalOplogEntry(how, operation); - }; - - // Calls syncRollBackLocalOperations to run updateFixUpInfoFromLocalOplogEntry - // on each oplog entry up until the common point. - auto res = syncRollBackLocalOperations( - localOplog, rollbackSource.getOplog(), processOperationForFixUp); - if (!res.isOK()) { - const auto status = res.getStatus(); - switch (status.code()) { - case ErrorCodes::OplogStartMissing: - case ErrorCodes::UnrecoverableRollbackError: - return status; - default: - throw RSFatalException(status.toString()); - } - } - - how.commonPoint = res.getValue().first; // OpTime - how.commonPointOurDiskloc = res.getValue().second; // RecordID - how.removeRedundantOperations(); - } catch (const RSFatalException& e) { - return Status(ErrorCodes::UnrecoverableRollbackError, - str::stream() - << "need to rollback, but unable to determine common point between" - " local and remote oplog: " - << e.what()); - } - - log() << "Rollback common point is " << how.commonPoint; - try { - ON_BLOCK_EXIT([&] { - auto status = replicationProcess->incrementRollbackID(opCtx); - fassertStatusOK(40497, status); - }); - syncFixUp(opCtx, how, rollbackSource, replCoord, replicationProcess); - } catch (const RSFatalException& e) { - return Status(ErrorCodes::UnrecoverableRollbackError, e.what()); - } - - if (MONGO_FAIL_POINT(rollbackHangBeforeFinish)) { - // This log output is used in js tests so please leave it. - log() << "rollback - rollbackHangBeforeFinish fail point " - "enabled. Blocking until fail point is disabled."; - while (MONGO_FAIL_POINT(rollbackHangBeforeFinish)) { - invariant(!globalInShutdownDeprecated()); // It is an error to shutdown while enabled. - mongo::sleepsecs(1); - } - } - - return Status::OK(); -} - -} // namespace - Status syncRollback(OperationContext* opCtx, const OplogInterface& localOplog, const RollbackSource& rollbackSource, diff --git a/src/mongo/db/repl/rs_rollback.h b/src/mongo/db/repl/rs_rollback.h index 74d59ae3c8f..d4e4015f279 100644 --- a/src/mongo/db/repl/rs_rollback.h +++ b/src/mongo/db/repl/rs_rollback.h @@ -371,6 +371,19 @@ private: * to be reverted. */ Status updateFixUpInfoFromLocalOplogEntry(FixUpInfo& fixUpInfo, const BSONObj& ourObj); + +/** + * This function uses the FixUpInfo struct to undo all of the operations that occurred after the + * common point on the rolling back node, checking the rollback ID and updating minValid as + * necessary. This includes refetching, updating, and deleting individual documents, resyncing + * collection data and metadata, and dropping and creating collections and indexes. Truncates the + * oplog and triggers necessary in-memory refreshes before returning. + */ +void syncFixUp(OperationContext* opCtx, + const FixUpInfo& fixUpInfo, + const RollbackSource& rollbackSource, + ReplicationCoordinator* replCoord, + ReplicationProcess* replicationProcess); } // namespace rollback_internal } // namespace repl } // namespace mongo diff --git a/src/mongo/db/repl/rs_rollback_no_uuid_test.cpp b/src/mongo/db/repl/rs_rollback_no_uuid_test.cpp index 2b4f239c54c..b48ed2596c9 100644 --- a/src/mongo/db/repl/rs_rollback_no_uuid_test.cpp +++ b/src/mongo/db/repl/rs_rollback_no_uuid_test.cpp @@ -72,7 +72,9 @@ public: const HostAndPort& getSource() const override; BSONObj getLastOperation() const override; BSONObj findOne(const NamespaceString& nss, const BSONObj& filter) const override; - BSONObj findOneByUUID(const std::string& db, UUID uuid, const BSONObj& filter) const override; + std::pair<BSONObj, NamespaceString> findOneByUUID(const std::string& db, + UUID uuid, + const BSONObj& filter) const override; void copyCollectionFromRemote(OperationContext* opCtx, const NamespaceString& nss) const override; StatusWith<BSONObj> getCollectionInfoByUUID(const std::string& db, @@ -110,10 +112,10 @@ BSONObj RollbackSourceMock::findOne(const NamespaceString& nss, const BSONObj& f return BSONObj(); } -BSONObj RollbackSourceMock::findOneByUUID(const std::string& db, - UUID uuid, - const BSONObj& filter) const { - return BSONObj(); +std::pair<BSONObj, NamespaceString> RollbackSourceMock::findOneByUUID(const std::string& db, + UUID uuid, + const BSONObj& filter) const { + return {BSONObj(), NamespaceString()}; } void RollbackSourceMock::copyCollectionFromRemote(OperationContext* opCtx, diff --git a/src/mongo/db/repl/rs_rollback_test.cpp b/src/mongo/db/repl/rs_rollback_test.cpp index ec966c4cb01..ee1caf718be 100644 --- a/src/mongo/db/repl/rs_rollback_test.cpp +++ b/src/mongo/db/repl/rs_rollback_test.cpp @@ -75,7 +75,9 @@ public: BSONObj getLastOperation() const override; BSONObj findOne(const NamespaceString& nss, const BSONObj& filter) const override; - BSONObj findOneByUUID(const std::string& db, UUID uuid, const BSONObj& filter) const override; + std::pair<BSONObj, NamespaceString> findOneByUUID(const std::string& db, + UUID uuid, + const BSONObj& filter) const override; void copyCollectionFromRemote(OperationContext* opCtx, const NamespaceString& nss) const override; @@ -114,10 +116,10 @@ BSONObj RollbackSourceMock::findOne(const NamespaceString& nss, const BSONObj& f return BSONObj(); } -BSONObj RollbackSourceMock::findOneByUUID(const std::string& db, - UUID uuid, - const BSONObj& filter) const { - return BSONObj(); +std::pair<BSONObj, NamespaceString> RollbackSourceMock::findOneByUUID(const std::string& db, + UUID uuid, + const BSONObj& filter) const { + return {BSONObj(), NamespaceString()}; } void RollbackSourceMock::copyCollectionFromRemote(OperationContext* opCtx, @@ -396,9 +398,11 @@ int _testRollbackDelete(OperationContext* opCtx, : RollbackSourceMock(std::move(oplog)), called(false), _documentAtSource(documentAtSource) {} - BSONObj findOneByUUID(const std::string& db, UUID uuid, const BSONObj& filter) const { + std::pair<BSONObj, NamespaceString> findOneByUUID(const std::string& db, + UUID uuid, + const BSONObj& filter) const override { called = true; - return _documentAtSource; + return {_documentAtSource, NamespaceString()}; } mutable bool called; @@ -1643,9 +1647,9 @@ TEST_F(RSRollbackTest, RollbackApplyOpsCommand) { RollbackSourceLocal(std::unique_ptr<OplogInterface> oplog) : RollbackSourceMock(std::move(oplog)) {} - BSONObj findOneByUUID(const std::string& db, - UUID uuid, - const BSONObj& filter) const override { + std::pair<BSONObj, NamespaceString> findOneByUUID(const std::string& db, + UUID uuid, + const BSONObj& filter) const override { int numFields = 0; for (const auto element : filter) { ++numFields; @@ -1655,11 +1659,11 @@ TEST_F(RSRollbackTest, RollbackApplyOpsCommand) { searchedIds.insert(filter.firstElement().numberInt()); switch (filter.firstElement().numberInt()) { case 1: - return BSON("_id" << 1 << "v" << 1); + return {BSON("_id" << 1 << "v" << 1), NamespaceString()}; case 2: - return BSON("_id" << 2 << "v" << 3); + return {BSON("_id" << 2 << "v" << 3), NamespaceString()}; case 3: - return BSON("_id" << 3 << "v" << 5); + return {BSON("_id" << 3 << "v" << 5), NamespaceString()}; case 4: return {}; } @@ -2048,6 +2052,92 @@ TEST_F(RSRollbackTest, LocalEntryWithTxnNumberAddsTransactionTableDocToBeRefetch ASSERT_TRUE(fui.docsToRefetch.find(expectedTxnDoc) != fui.docsToRefetch.end()); } +TEST_F(RSRollbackTest, RollbackFailsIfTransactionDocumentRefetchReturnsDifferentNamespace) { + createOplog(_opCtx.get()); + + // Create a valid FixUpInfo struct for rolling back a single CRUD operation that has a + // transaction number and session id. + FixUpInfo fui; + + auto entryWithTxnNumber = + BSON("ts" << Timestamp(Seconds(2), 0) << "t" << 1LL << "h" << 1LL << "op" + << "i" + << "ui" + << UUID::gen() + << "ns" + << "test.t" + << "o" + << BSON("_id" << 1 << "a" << 1) + << "txnNumber" + << 1LL + << "stmtId" + << 1 + << "lsid" + << makeLogicalSessionIdForTest().toBSON()); + + UUID transactionTableUUID = UUID::gen(); + fui.transactionTableUUID = transactionTableUUID; + + auto commonOperation = + std::make_pair(BSON("ts" << Timestamp(Seconds(1), 0) << "h" << 1LL), RecordId(1)); + fui.commonPoint = OpTime(Timestamp(Seconds(1), 0), 1LL); + fui.commonPointOurDiskloc = RecordId(1); + + fui.rbid = 1; + + // The FixUpInfo will have an extra doc to refetch: the corresponding transaction table entry. + ASSERT_OK(updateFixUpInfoFromLocalOplogEntry(fui, entryWithTxnNumber)); + ASSERT_EQ(fui.docsToRefetch.size(), 2U); + + { + class RollbackSourceLocal : public RollbackSourceMock { + public: + RollbackSourceLocal(std::unique_ptr<OplogInterface> oplog) + : RollbackSourceMock(std::move(oplog)) {} + std::pair<BSONObj, NamespaceString> findOneByUUID( + const std::string& db, UUID uuid, const BSONObj& filter) const override { + return {BSONObj(), NamespaceString::kSessionTransactionsTableNamespace}; + } + int getRollbackId() const override { + return 1; + } + }; + + // Should not throw, since findOneByUUID will return the expected namespace. + syncFixUp(_opCtx.get(), + fui, + RollbackSourceLocal( + std::unique_ptr<OplogInterface>(new OplogInterfaceMock({commonOperation}))), + _coordinator, + _replicationProcess.get()); + } + + { + class RollbackSourceLocal : public RollbackSourceMock { + public: + RollbackSourceLocal(std::unique_ptr<OplogInterface> oplog) + : RollbackSourceMock(std::move(oplog)) {} + std::pair<BSONObj, NamespaceString> findOneByUUID( + const std::string& db, UUID uuid, const BSONObj& filter) const override { + return {BSONObj(), NamespaceString("foo.bar")}; + } + int getRollbackId() const override { + return 1; + } + }; + + // The returned namespace will not be the expected one, implying a rename/drop of the + // transactions collection across this node and the sync source, so rollback should fail. + ASSERT_THROWS(syncFixUp(_opCtx.get(), + fui, + RollbackSourceLocal(std::unique_ptr<OplogInterface>( + new OplogInterfaceMock({commonOperation}))), + _coordinator, + _replicationProcess.get()), + RSFatalException); + } +} + TEST_F(RSRollbackTest, RollbackReturnsImmediatelyOnFailureToTransitionToRollback) { // On failing to transition to ROLLBACK, rollback() should return immediately and not call // syncRollback(). We provide an empty oplog so that if syncRollback() is called erroneously, |