diff options
author | Judah Schvimer <judah@mongodb.com> | 2018-02-13 13:57:30 -0500 |
---|---|---|
committer | Judah Schvimer <judah@mongodb.com> | 2018-02-13 13:57:30 -0500 |
commit | 628c8d5e384baffb7f88b2b6ab1855dedcda36b2 (patch) | |
tree | 963ae215fe1d505907cae6f8c7ac52ceaf00856a | |
parent | 1e2160c8f0480a1d8be1d671b5b7e22e52986a2c (diff) | |
download | mongo-628c8d5e384baffb7f88b2b6ab1855dedcda36b2.tar.gz |
Revert "SERVER-33706 Rollback to checkpoint should set OplogTruncateAfterPoint to document after common point"
This reverts commit 358dfccf9f896116ac7d79772f91cb604672175a.
-rw-r--r-- | src/mongo/db/repl/rollback_impl.cpp | 46 | ||||
-rw-r--r-- | src/mongo/db/repl/rollback_impl.h | 10 | ||||
-rw-r--r-- | src/mongo/db/repl/rollback_impl_test.cpp | 52 | ||||
-rw-r--r-- | src/mongo/db/repl/rollback_test_fixture.cpp | 7 | ||||
-rw-r--r-- | src/mongo/db/repl/rollback_test_fixture.h | 5 |
5 files changed, 24 insertions, 96 deletions
diff --git a/src/mongo/db/repl/rollback_impl.cpp b/src/mongo/db/repl/rollback_impl.cpp index bfed02e8d60..5ed3d779bc2 100644 --- a/src/mongo/db/repl/rollback_impl.cpp +++ b/src/mongo/db/repl/rollback_impl.cpp @@ -34,7 +34,6 @@ #include "mongo/db/background.h" #include "mongo/db/concurrency/d_concurrency.h" -#include "mongo/db/db_raii.h" #include "mongo/db/logical_time_validator.h" #include "mongo/db/operation_context.h" #include "mongo/db/repl/replication_coordinator.h" @@ -105,21 +104,16 @@ Status RollbackImpl::runRollback(OperationContext* opCtx) { return commonPointSW.getStatus(); } - // During replication recovery, we truncate all oplog entries with timestamps greater than or - // equal to the oplog truncate after point. As a result, we must find the oplog entry after - // the common point so we do not truncate the common point itself. If we entered rollback, - // we are guaranteed to have at least one oplog entry after the common point. - Timestamp truncatePoint = _findTruncateTimestamp(opCtx, commonPointSW.getValue()); - - // Persist the truncate point to the 'oplogTruncateAfterPoint' document. We save this value so + // Persist the common point to the 'oplogTruncateAfterPoint' document. We save this value so // that the replication recovery logic knows where to truncate the oplog. Note that it must be // saved *durably* in case a crash occurs after the storage engine recovers to the stable // timestamp. Upon startup after such a crash, the standard replication recovery code will know // where to truncate the oplog by observing the value of the 'oplogTruncateAfterPoint' document. // Note that the storage engine timestamp recovery only restores the database *data* to a stable // timestamp, but does not revert the oplog, which must be done as part of the rollback process. - _replicationProcess->getConsistencyMarkers()->setOplogTruncateAfterPoint(opCtx, truncatePoint); - _listener->onCommonPointFound(commonPointSW.getValue().first.getTimestamp()); + _replicationProcess->getConsistencyMarkers()->setOplogTruncateAfterPoint( + opCtx, commonPointSW.getValue()); + _listener->onCommonPointFound(commonPointSW.getValue()); // Increment the Rollback ID of this node. The Rollback ID is a natural number that it is // incremented by 1 every time a rollback occurs. Note that the Rollback ID must be incremented @@ -225,7 +219,7 @@ Status RollbackImpl::_awaitBgIndexCompletion(OperationContext* opCtx) { return Status::OK(); } -StatusWith<RollBackLocalOperations::RollbackCommonPoint> RollbackImpl::_findCommonPoint() { +StatusWith<Timestamp> RollbackImpl::_findCommonPoint() { if (_isInShutdown()) { return Status(ErrorCodes::ShutdownInProgress, "rollback shutting down"); } @@ -259,35 +253,7 @@ StatusWith<RollBackLocalOperations::RollbackCommonPoint> RollbackImpl::_findComm invariant(commonPoint.getTimestamp() >= committedSnapshot.getTimestamp()); invariant(commonPoint >= committedSnapshot); - return commonPointSW.getValue(); -} - -Timestamp RollbackImpl::_findTruncateTimestamp( - OperationContext* opCtx, RollBackLocalOperations::RollbackCommonPoint commonPoint) const { - - AutoGetCollectionForRead oplog(opCtx, NamespaceString::kRsOplogNamespace); - invariant(oplog.getCollection()); - auto oplogCursor = oplog.getCollection()->getCursor(opCtx, /*forward=*/true); - - auto commonPointRecord = oplogCursor->seekExact(commonPoint.second); - // Check that we've found the right document for the common point. - invariant(commonPointRecord); - auto commonPointTime = OpTime::parseFromOplogEntry(commonPointRecord->data.releaseToBson()); - invariantOK(commonPointTime.getStatus()); - invariant(commonPointTime.getValue() == commonPoint.first, - str::stream() << "Common point: " << commonPoint.first.toString() - << ", record found: " - << commonPointTime.getValue().toString()); - - // Get the next document, which will be the first document to truncate. - auto truncatePointRecord = oplogCursor->next(); - invariant(truncatePointRecord); - auto truncatePointTime = OpTime::parseFromOplogEntry(truncatePointRecord->data.releaseToBson()); - invariantOK(truncatePointTime.getStatus()); - - log() << "Marking to truncate all oplog entries with timestamps greater than or equal to " - << truncatePointTime.getValue(); - return truncatePointTime.getValue().getTimestamp(); + return commonPoint.getTimestamp(); } Status RollbackImpl::_recoverToStableTimestamp(OperationContext* opCtx) { diff --git a/src/mongo/db/repl/rollback_impl.h b/src/mongo/db/repl/rollback_impl.h index cdbb3ee0f88..c88899e70b0 100644 --- a/src/mongo/db/repl/rollback_impl.h +++ b/src/mongo/db/repl/rollback_impl.h @@ -29,7 +29,6 @@ #pragma once #include "mongo/base/status_with.h" -#include "mongo/db/repl/roll_back_local_operations.h" #include "mongo/db/repl/rollback.h" #include "mongo/db/repl/storage_interface.h" #include "mongo/stdx/functional.h" @@ -163,14 +162,7 @@ private: /** * Finds the common point between the local and remote oplogs. */ - StatusWith<RollBackLocalOperations::RollbackCommonPoint> _findCommonPoint(); - - /** - * Finds the timestamp of the record after the common point to put into the oplog truncate - * after point. - */ - Timestamp _findTruncateTimestamp( - OperationContext* opCtx, RollBackLocalOperations::RollbackCommonPoint commonPoint) const; + StatusWith<Timestamp> _findCommonPoint(); /** * Uses the ReplicationCoordinator to transition the current member state to ROLLBACK. diff --git a/src/mongo/db/repl/rollback_impl_test.cpp b/src/mongo/db/repl/rollback_impl_test.cpp index 5cdaed9d819..5b23f7278f3 100644 --- a/src/mongo/db/repl/rollback_impl_test.cpp +++ b/src/mongo/db/repl/rollback_impl_test.cpp @@ -30,7 +30,6 @@ #include "mongo/db/repl/rollback_test_fixture.h" -#include "mongo/db/repl/oplog_interface_local.h" #include "mongo/db/repl/oplog_interface_mock.h" #include "mongo/db/repl/rollback_impl.h" #include "mongo/db/s/shard_identity_rollback_notifier.h" @@ -116,7 +115,7 @@ private: protected: std::unique_ptr<StorageInterfaceRollback> _storageInterface; - std::unique_ptr<OplogInterfaceLocal> _localOplog; + std::unique_ptr<OplogInterfaceMock> _localOplog; std::unique_ptr<OplogInterfaceMock> _remoteOplog; std::unique_ptr<RollbackImpl> _rollback; @@ -144,8 +143,7 @@ void RollbackImplTest::setUp() { // Set up test-specific storage interface. _storageInterface = stdx::make_unique<StorageInterfaceRollback>(); - _localOplog = stdx::make_unique<OplogInterfaceLocal>(_opCtx.get(), - NamespaceString::kRsOplogNamespace.ns()); + _localOplog = stdx::make_unique<OplogInterfaceMock>(); _remoteOplog = stdx::make_unique<OplogInterfaceMock>(); _listener = stdx::make_unique<Listener>(this); _rollback = stdx::make_unique<RollbackImpl>(_localOplog.get(), @@ -154,8 +152,6 @@ void RollbackImplTest::setUp() { _replicationProcess.get(), _coordinator, _listener.get()); - - createOplog(_opCtx.get()); } void RollbackImplTest::tearDown() { @@ -234,7 +230,7 @@ TEST_F(RollbackImplTest, RollbackReturnsNotSecondaryWhenFailingToTransitionToRol } TEST_F(RollbackImplTest, RollbackReturnsInvalidSyncSourceWhenNoRemoteOplog) { - ASSERT_OK(_insertOplogEntry(makeOp(1))); + _localOplog->setOperations({makeOpAndRecordId(1)}); ASSERT_EQUALS(ErrorCodes::InvalidSyncSource, _rollback->runRollback(_opCtx.get())); } @@ -247,32 +243,27 @@ TEST_F(RollbackImplTest, RollbackReturnsOplogStartMissingWhenNoLocalOplog) { TEST_F(RollbackImplTest, RollbackReturnsNoMatchingDocumentWhenNoCommonPoint) { _remoteOplog->setOperations({makeOpAndRecordId(1)}); - ASSERT_OK(_insertOplogEntry(makeOp(2))); + _localOplog->setOperations({makeOpAndRecordId(2)}); ASSERT_EQUALS(ErrorCodes::NoMatchingDocument, _rollback->runRollback(_opCtx.get())); } -TEST_F(RollbackImplTest, RollbackPersistsDocumentAfterCommonPointToOplogTruncateAfterPoint) { - auto commonPoint = makeOpAndRecordId(2); - _remoteOplog->setOperations({commonPoint}); - ASSERT_OK(_insertOplogEntry(commonPoint.first)); - - auto nextTime = 3; - ASSERT_OK(_insertOplogEntry(makeOp(nextTime))); +TEST_F(RollbackImplTest, RollbackPersistsCommonPointToOplogTruncateAfterPoint) { + _remoteOplog->setOperations({makeOpAndRecordId(2)}); + _localOplog->setOperations({makeOpAndRecordId(2)}); ASSERT_OK(_rollback->runRollback(_opCtx.get())); // Check that the common point was saved. auto truncateAfterPoint = _replicationProcess->getConsistencyMarkers()->getOplogTruncateAfterPoint(_opCtx.get()); - ASSERT_EQUALS(Timestamp(nextTime, nextTime), truncateAfterPoint); + ASSERT_EQUALS(Timestamp(2, 2), truncateAfterPoint); } TEST_F(RollbackImplTest, RollbackIncrementsRollbackID) { auto op = makeOpAndRecordId(1); _remoteOplog->setOperations({op}); - ASSERT_OK(_insertOplogEntry(op.first)); - ASSERT_OK(_insertOplogEntry(makeOp(2))); + _localOplog->setOperations({op}); // Get the initial rollback id. int initRollbackId = _replicationProcess->getRollbackID(); @@ -288,8 +279,7 @@ TEST_F(RollbackImplTest, RollbackIncrementsRollbackID) { TEST_F(RollbackImplTest, RollbackCallsRecoverToStableTimestamp) { auto op = makeOpAndRecordId(1); _remoteOplog->setOperations({op}); - ASSERT_OK(_insertOplogEntry(op.first)); - ASSERT_OK(_insertOplogEntry(makeOp(2))); + _localOplog->setOperations({op}); auto stableTimestamp = Timestamp(10, 0); auto currTimestamp = Timestamp(20, 0); @@ -311,8 +301,7 @@ TEST_F(RollbackImplTest, RollbackCallsRecoverToStableTimestamp) { TEST_F(RollbackImplTest, RollbackReturnsBadStatusIfRecoverToStableTimestampFails) { auto op = makeOpAndRecordId(1); _remoteOplog->setOperations({op}); - ASSERT_OK(_insertOplogEntry(op.first)); - ASSERT_OK(_insertOplogEntry(makeOp(2))); + _localOplog->setOperations({op}); auto stableTimestamp = Timestamp(10, 0); auto currTimestamp = Timestamp(20, 0); @@ -338,8 +327,7 @@ TEST_F(RollbackImplTest, RollbackReturnsBadStatusIfRecoverToStableTimestampFails TEST_F(RollbackImplTest, RollbackReturnsBadStatusIfIncrementRollbackIDFails) { auto op = makeOpAndRecordId(1); _remoteOplog->setOperations({op}); - ASSERT_OK(_insertOplogEntry(op.first)); - ASSERT_OK(_insertOplogEntry(makeOp(2))); + _localOplog->setOperations({op}); // Delete the rollback id collection. auto rollbackIdNss = NamespaceString(_storageInterface->kDefaultRollbackIdNamespace); @@ -355,8 +343,7 @@ TEST_F(RollbackImplTest, RollbackReturnsBadStatusIfIncrementRollbackIDFails) { TEST_F(RollbackImplTest, RollbackCallsRecoverFromOplog) { auto op = makeOpAndRecordId(1); _remoteOplog->setOperations({op}); - ASSERT_OK(_insertOplogEntry(op.first)); - ASSERT_OK(_insertOplogEntry(makeOp(2))); + _localOplog->setOperations({op}); // Run rollback. ASSERT_OK(_rollback->runRollback(_opCtx.get())); @@ -368,8 +355,7 @@ TEST_F(RollbackImplTest, RollbackCallsRecoverFromOplog) { TEST_F(RollbackImplTest, RollbackSkipsRecoverFromOplogWhenShutdownEarly) { auto op = makeOpAndRecordId(1); _remoteOplog->setOperations({op}); - ASSERT_OK(_insertOplogEntry(op.first)); - ASSERT_OK(_insertOplogEntry(makeOp(2))); + _localOplog->setOperations({op}); _onRecoverToStableTimestampFn = [this]() { _recoveredToStableTimestamp = true; @@ -388,8 +374,7 @@ TEST_F(RollbackImplTest, RollbackSkipsRecoverFromOplogWhenShutdownEarly) { TEST_F(RollbackImplTest, RollbackSucceeds) { auto op = makeOpAndRecordId(1); _remoteOplog->setOperations({op}); - ASSERT_OK(_insertOplogEntry(op.first)); - ASSERT_OK(_insertOplogEntry(makeOp(2))); + _localOplog->setOperations({op}); ASSERT_OK(_rollback->runRollback(_opCtx.get())); ASSERT_EQUALS(Timestamp(1, 1), _commonPointFound); @@ -405,8 +390,7 @@ DEATH_TEST_F(RollbackImplTest, auto op = makeOpAndRecordId(1); _remoteOplog->setOperations({op}); - ASSERT_OK(_insertOplogEntry(op.first)); - ASSERT_OK(_insertOplogEntry(makeOp(2))); + _localOplog->setOperations({op}); auto status = _rollback->runRollback(_opCtx.get()); unittest::log() << "Mongod did not crash. Status: " << status; MONGO_UNREACHABLE; @@ -420,9 +404,7 @@ DEATH_TEST_F(RollbackImplTest, auto op = makeOpAndRecordId(1); _remoteOplog->setOperations({op}); - ASSERT_OK(_insertOplogEntry(op.first)); - ASSERT_OK(_insertOplogEntry(makeOp(2))); - + _localOplog->setOperations({op}); auto status = _rollback->runRollback(_opCtx.get()); unittest::log() << "Mongod did not crash. Status: " << status; MONGO_UNREACHABLE; diff --git a/src/mongo/db/repl/rollback_test_fixture.cpp b/src/mongo/db/repl/rollback_test_fixture.cpp index 17ffebdade8..00f90b00b6b 100644 --- a/src/mongo/db/repl/rollback_test_fixture.cpp +++ b/src/mongo/db/repl/rollback_test_fixture.cpp @@ -172,13 +172,6 @@ Collection* RollbackTest::_createCollection(OperationContext* opCtx, return _createCollection(opCtx, NamespaceString(nss), options); } -Status RollbackTest::_insertOplogEntry(const BSONObj& doc) { - TimestampedBSONObj obj; - obj.obj = doc; - return _storageInterface.insertDocument( - _opCtx.get(), NamespaceString::kRsOplogNamespace, obj, 0); -} - RollbackSourceMock::RollbackSourceMock(std::unique_ptr<OplogInterface> oplog) : _oplog(std::move(oplog)) {} diff --git a/src/mongo/db/repl/rollback_test_fixture.h b/src/mongo/db/repl/rollback_test_fixture.h index ac64de6d8a0..c90acf64068 100644 --- a/src/mongo/db/repl/rollback_test_fixture.h +++ b/src/mongo/db/repl/rollback_test_fixture.h @@ -77,11 +77,6 @@ public: const CollectionOptions& options); /** - * Inserts a document into the oplog. - */ - Status _insertOplogEntry(const BSONObj& doc); - - /** * Creates an oplog entry with a recordId for a command operation. */ static std::pair<BSONObj, RecordId> makeCommandOp( |