Skip to content

Commit

Permalink
Revert "SERVER-27659 Persist Rollback Id"
Browse files Browse the repository at this point in the history
This reverts commit ac6f185.
  • Loading branch information
ADAM David Alan Martin committed Apr 28, 2017
1 parent 1780706 commit b1054a0
Show file tree
Hide file tree
Showing 21 changed files with 99 additions and 375 deletions.
3 changes: 1 addition & 2 deletions src/mongo/db/commands/dbcommands.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1313,8 +1313,7 @@ void appendReplyMetadata(OperationContext* opCtx,
// Attach our own last opTime.
repl::OpTime lastOpTimeFromClient =
repl::ReplClientInfo::forClient(opCtx->getClient()).getLastOp();
replCoord->prepareReplMetadata(
opCtx, request.getMetadata(), lastOpTimeFromClient, metadataBob);
replCoord->prepareReplMetadata(request.getMetadata(), lastOpTimeFromClient, metadataBob);
// For commands from mongos, append some info to help getLastError(w) work.
// TODO: refactor out of here as part of SERVER-18236
if (isShardingAware || isConfig) {
Expand Down
12 changes: 0 additions & 12 deletions src/mongo/db/repl/SConscript
Original file line number Diff line number Diff line change
Expand Up @@ -134,17 +134,6 @@ env.CppUnitTest(
],
)

env.Library(
target='rollback_idl',
source=[
env.Idlc('rollback.idl')[0],
],
LIBDEPS=[
'$BUILD_DIR/mongo/base',
'$BUILD_DIR/mongo/idl/idl_parser',
],
)

env.Library(
target='storage_interface',
source=[
Expand All @@ -164,7 +153,6 @@ env.Library(
'collection_bulk_loader_impl.cpp',
],
LIBDEPS=[
'rollback_idl',
'storage_interface',
'$BUILD_DIR/mongo/db/common',
'$BUILD_DIR/mongo/db/exec/exec',
Expand Down
9 changes: 2 additions & 7 deletions src/mongo/db/repl/repl_set_commands.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -169,13 +169,8 @@ class CmdReplSetGetRBID : public ReplSetCommand {
if (!status.isOK())
return appendCommandStatus(result, status);

auto rbid = StorageInterface::get(opCtx)->getRollbackID(opCtx);

// We should always have a Rollback ID since it is created at startup.
fassertStatusOK(40426, rbid.getStatus());

result.append("rbid", rbid.getValue());
return appendCommandStatus(result, Status::OK());
status = getGlobalReplicationCoordinator()->processReplSetGetRBID(&result);
return appendCommandStatus(result, status);
}
} cmdReplSetRBID;

Expand Down
14 changes: 12 additions & 2 deletions src/mongo/db/repl/replication_coordinator.h
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,17 @@ class ReplicationCoordinator : public SyncSourceSelector {
const BSONObj& configObj,
BSONObjBuilder* resultObj) = 0;

/*
* Handles an incoming replSetGetRBID command.
* Adds BSON to 'resultObj'; returns a Status with either OK or an error message.
*/
virtual Status processReplSetGetRBID(BSONObjBuilder* resultObj) = 0;

/**
* Increments this process's rollback id. Called every time a rollback occurs.
*/
virtual void incrementRollbackID() = 0;

/**
* Arguments to the replSetFresh command.
*/
Expand Down Expand Up @@ -754,8 +765,7 @@ class ReplicationCoordinator : public SyncSourceSelector {
* Prepares a metadata object with the ReplSetMetadata and the OplogQueryMetadata depending
* on what has been requested.
*/
virtual void prepareReplMetadata(OperationContext* opCtx,
const BSONObj& metadataRequestObj,
virtual void prepareReplMetadata(const BSONObj& metadataRequestObj,
const OpTime& lastOpTimeFromClient,
BSONObjBuilder* builder) const = 0;

Expand Down
44 changes: 22 additions & 22 deletions src/mongo/db/repl/replication_coordinator_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,13 @@ ReplicationCoordinatorImpl::ReplicationCoordinatorImpl(
return;
}

std::unique_ptr<SecureRandom> rbidGenerator(SecureRandom::create());
_rbid = static_cast<int>(rbidGenerator->nextInt64());
if (_rbid < 0) {
// Ensure _rbid is always positive
_rbid = -_rbid;
}

// Make sure there is always an entry in _slaveInfo for ourself.
SlaveInfo selfInfo;
selfInfo.self = true;
Expand Down Expand Up @@ -423,19 +430,6 @@ bool ReplicationCoordinatorImpl::_startLoadLocalConfig(OperationContext* opCtx)
_topCoord->loadLastVote(lastVote.getValue());
}

// Check that we have a local Rollback ID. If we do not have one, create one.
auto rbid = _storage->getRollbackID(opCtx);
if (!rbid.isOK()) {
if (rbid.getStatus() == ErrorCodes::NamespaceNotFound) {
log() << "Did not find local Rollback ID document at startup. Creating one.";
auto initializingStatus = _storage->initializeRollbackID(opCtx);
fassertStatusOK(40424, initializingStatus);
} else {
severe() << "Error loading local Rollback ID document at startup; " << rbid.getStatus();
fassertFailedNoTrace(40428);
}
}

StatusWith<BSONObj> cfg = _externalState->loadLocalConfigDocument(opCtx);
if (!cfg.isOK()) {
log() << "Did not find local replica set configuration document at startup; "
Expand Down Expand Up @@ -2873,6 +2867,17 @@ void ReplicationCoordinatorImpl::_enterDrainMode_inlock() {
_externalState->stopProducer();
}

Status ReplicationCoordinatorImpl::processReplSetGetRBID(BSONObjBuilder* resultObj) {
stdx::lock_guard<stdx::mutex> lk(_mutex);
resultObj->append("rbid", _rbid);
return Status::OK();
}

void ReplicationCoordinatorImpl::incrementRollbackID() {
stdx::lock_guard<stdx::mutex> lk(_mutex);
++_rbid;
}

Status ReplicationCoordinatorImpl::processReplSetFresh(const ReplSetFreshArgs& args,
BSONObjBuilder* resultObj) {
stdx::lock_guard<stdx::mutex> lk(_mutex);
Expand Down Expand Up @@ -3324,8 +3329,7 @@ Status ReplicationCoordinatorImpl::processReplSetRequestVotes(
return Status::OK();
}

void ReplicationCoordinatorImpl::prepareReplMetadata(OperationContext* opCtx,
const BSONObj& metadataRequestObj,
void ReplicationCoordinatorImpl::prepareReplMetadata(const BSONObj& metadataRequestObj,
const OpTime& lastOpTimeFromClient,
BSONObjBuilder* builder) const {

Expand All @@ -3336,17 +3340,14 @@ void ReplicationCoordinatorImpl::prepareReplMetadata(OperationContext* opCtx,
return;
}

auto rbid = _storage->getRollbackID(opCtx);
fassertStatusOK(40427, rbid.getStatus());

stdx::lock_guard<stdx::mutex> lk(_mutex);

if (hasReplSetMetadata) {
_prepareReplSetMetadata_inlock(lastOpTimeFromClient, builder);
}

if (hasOplogQueryMetadata) {
_prepareOplogQueryMetadata_inlock(rbid.getValue(), builder);
_prepareOplogQueryMetadata_inlock(builder);
}
}

Expand All @@ -3358,11 +3359,10 @@ void ReplicationCoordinatorImpl::_prepareReplSetMetadata_inlock(const OpTime& la
metadata.writeToMetadata(builder);
}

void ReplicationCoordinatorImpl::_prepareOplogQueryMetadata_inlock(int rbid,
BSONObjBuilder* builder) const {
void ReplicationCoordinatorImpl::_prepareOplogQueryMetadata_inlock(BSONObjBuilder* builder) const {
OpTime lastAppliedOpTime = _getMyLastAppliedOpTime_inlock();
auto metadata =
_topCoord->prepareOplogQueryMetadata(_lastCommittedOpTime, lastAppliedOpTime, rbid);
_topCoord->prepareOplogQueryMetadata(_lastCommittedOpTime, lastAppliedOpTime, _rbid);
metadata.writeToMetadata(builder);
}

Expand Down
13 changes: 10 additions & 3 deletions src/mongo/db/repl/replication_coordinator_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,10 @@ class ReplicationCoordinatorImpl : public ReplicationCoordinator {
const BSONObj& configObj,
BSONObjBuilder* resultObj) override;

virtual Status processReplSetGetRBID(BSONObjBuilder* resultObj) override;

virtual void incrementRollbackID() override;

virtual Status processReplSetFresh(const ReplSetFreshArgs& args,
BSONObjBuilder* resultObj) override;

Expand Down Expand Up @@ -278,8 +282,7 @@ class ReplicationCoordinatorImpl : public ReplicationCoordinator {
const ReplSetRequestVotesArgs& args,
ReplSetRequestVotesResponse* response) override;

virtual void prepareReplMetadata(OperationContext* opCtx,
const BSONObj& metadataRequestObj,
virtual void prepareReplMetadata(const BSONObj& metadataRequestObj,
const OpTime& lastOpTimeFromClient,
BSONObjBuilder* builder) const override;

Expand Down Expand Up @@ -1088,7 +1091,7 @@ class ReplicationCoordinatorImpl : public ReplicationCoordinator {
/**
* Prepares a metadata object for OplogQueryMetadata.
*/
void _prepareOplogQueryMetadata_inlock(int rbid, BSONObjBuilder* builder) const;
void _prepareOplogQueryMetadata_inlock(BSONObjBuilder* builder) const;

/**
* Blesses a snapshot to be used for new committed reads.
Expand Down Expand Up @@ -1287,6 +1290,10 @@ class ReplicationCoordinatorImpl : public ReplicationCoordinator {
// updates upstream. Set once in startReplication() and then never modified again.
OID _myRID; // (M)

// Rollback ID. Used to check if a rollback happened during some interval of time
// TODO: ideally this should only change on rollbacks NOT on mongod restarts also.
int _rbid; // (M)

// list of information about clients waiting on replication. Does *not* own the WaiterInfos.
WaiterList _replicationWaiterList; // (M)

Expand Down
23 changes: 19 additions & 4 deletions src/mongo/db/repl/replication_coordinator_impl_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ TEST_F(ReplCoordTest, NodeEntersStartupStateWhenStartingUpWithNoLocalConfig) {
startCapturingLogMessages();
start();
stopCapturingLogMessages();
ASSERT_EQUALS(3, countLogLinesContaining("Did not find local "));
ASSERT_EQUALS(2, countLogLinesContaining("Did not find local "));
ASSERT_EQUALS(MemberState::RS_STARTUP, getReplCoord()->getMemberState().s);
}

Expand Down Expand Up @@ -583,6 +583,19 @@ TEST_F(ReplCoordTest, NodeReturnsOkWhenCheckReplEnabledForCommandAfterReceivingA
ASSERT_TRUE(result.obj().isEmpty());
}

TEST_F(ReplCoordTest, RollBackIDShouldIncreaseByOneWhenIncrementRollbackIDIsCalled) {
start();
BSONObjBuilder result;
getReplCoord()->processReplSetGetRBID(&result);
long long initialValue = result.obj()["rbid"].Int();
getReplCoord()->incrementRollbackID();

BSONObjBuilder result2;
getReplCoord()->processReplSetGetRBID(&result2);
long long incrementedValue = result2.obj()["rbid"].Int();
ASSERT_EQUALS(incrementedValue, initialValue + 1);
}

TEST_F(ReplCoordTest, NodeReturnsImmediatelyWhenAwaitReplicationIsRanAgainstAStandaloneNode) {
init("");
auto opCtx = makeOperationContext();
Expand Down Expand Up @@ -4058,11 +4071,13 @@ TEST_F(ReplCoordTest, PrepareOplogQueryMetadata) {
getReplCoord()->advanceCommitPoint(optime1);
getReplCoord()->setMyLastAppliedOpTime(optime2);

auto opCtx = makeOperationContext();
// Get current rbid to check against.
BSONObjBuilder result;
getReplCoord()->processReplSetGetRBID(&result);
int initialValue = result.obj()["rbid"].Int();

BSONObjBuilder metadataBob;
getReplCoord()->prepareReplMetadata(
opCtx.get(),
BSON(rpc::kOplogQueryMetadataFieldName << 1 << rpc::kReplSetMetadataFieldName << 1),
OpTime(),
&metadataBob);
Expand All @@ -4074,7 +4089,7 @@ TEST_F(ReplCoordTest, PrepareOplogQueryMetadata) {
ASSERT_OK(oqMetadata.getStatus());
ASSERT_EQ(oqMetadata.getValue().getLastOpCommitted(), optime1);
ASSERT_EQ(oqMetadata.getValue().getLastOpApplied(), optime2);
ASSERT_EQ(oqMetadata.getValue().getRBID(), 100);
ASSERT_EQ(oqMetadata.getValue().getRBID(), initialValue);
ASSERT_EQ(oqMetadata.getValue().getSyncSourceIndex(), -1);
ASSERT_EQ(oqMetadata.getValue().getPrimaryIndex(), -1);

Expand Down
9 changes: 7 additions & 2 deletions src/mongo/db/repl/replication_coordinator_mock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,12 @@ Status ReplicationCoordinatorMock::processReplSetInitiate(OperationContext* opCt
return Status::OK();
}

Status ReplicationCoordinatorMock::processReplSetGetRBID(BSONObjBuilder* resultObj) {
return Status::OK();
}

void ReplicationCoordinatorMock::incrementRollbackID() {}

Status ReplicationCoordinatorMock::processReplSetFresh(const ReplSetFreshArgs& args,
BSONObjBuilder* resultObj) {
return Status::OK();
Expand Down Expand Up @@ -410,8 +416,7 @@ Status ReplicationCoordinatorMock::processReplSetRequestVotes(
return Status::OK();
}

void ReplicationCoordinatorMock::prepareReplMetadata(OperationContext* opCtx,
const BSONObj& metadataRequestObj,
void ReplicationCoordinatorMock::prepareReplMetadata(const BSONObj& metadataRequestObj,
const OpTime& lastOpTimeFromClient,
BSONObjBuilder* builder) const {}

Expand Down
7 changes: 5 additions & 2 deletions src/mongo/db/repl/replication_coordinator_mock.h
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,10 @@ class ReplicationCoordinatorMock : public ReplicationCoordinator {
const BSONObj& configObj,
BSONObjBuilder* resultObj);

virtual Status processReplSetGetRBID(BSONObjBuilder* resultObj);

virtual void incrementRollbackID();

virtual Status processReplSetFresh(const ReplSetFreshArgs& args, BSONObjBuilder* resultObj);

virtual Status processReplSetElect(const ReplSetElectArgs& args, BSONObjBuilder* resultObj);
Expand Down Expand Up @@ -222,8 +226,7 @@ class ReplicationCoordinatorMock : public ReplicationCoordinator {
const ReplSetRequestVotesArgs& args,
ReplSetRequestVotesResponse* response);

void prepareReplMetadata(OperationContext* opCtx,
const BSONObj& metadataRequestObj,
void prepareReplMetadata(const BSONObj& metadataRequestObj,
const OpTime& lastOpTimeFromClient,
BSONObjBuilder* builder) const override;

Expand Down
7 changes: 1 addition & 6 deletions src/mongo/db/repl/replication_info.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
#include "mongo/db/repl/oplog.h"
#include "mongo/db/repl/oplogreader.h"
#include "mongo/db/repl/replication_coordinator_global.h"
#include "mongo/db/repl/storage_interface.h"
#include "mongo/db/server_options.h"
#include "mongo/db/server_parameters.h"
#include "mongo/db/storage/storage_options.h"
Expand Down Expand Up @@ -169,11 +168,7 @@ class ReplicationInfoServerStatus : public ServerStatusSection {

BSONObjBuilder result;
appendReplicationInfo(opCtx, result, level);

auto rbid = StorageInterface::get(opCtx)->getRollbackID(opCtx);
if (rbid.isOK()) {
result.append("rbid", rbid.getValue());
}
getGlobalReplicationCoordinator()->processReplSetGetRBID(&result);

return result.obj();
}
Expand Down
44 changes: 0 additions & 44 deletions src/mongo/db/repl/rollback.idl

This file was deleted.

1 change: 0 additions & 1 deletion src/mongo/db/repl/rollback_test_fixture.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ void RollbackTest::setUp() {
setOplogCollectionName();
_storageInterface.setAppliedThrough(_opCtx.get(), OpTime{});
_storageInterface.setMinValid(_opCtx.get(), OpTime{});
_storageInterface.initializeRollbackID(_opCtx.get());

_threadPoolExecutorTest.launchExecutorThread();
}
Expand Down
5 changes: 1 addition & 4 deletions src/mongo/db/repl/rs_rollback.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -826,10 +826,7 @@ Status _syncRollback(OperationContext* opCtx,
log() << "rollback common point is " << how.commonPoint;
log() << "rollback 3 fixup";
try {
ON_BLOCK_EXIT([&] {
auto status = storageInterface->incrementRollbackID(opCtx);
fassertStatusOK(40425, status);
});
ON_BLOCK_EXIT([&] { replCoord->incrementRollbackID(); });
syncFixUp(opCtx, how, rollbackSource, replCoord, storageInterface);
} catch (const RSFatalException& e) {
return Status(ErrorCodes::UnrecoverableRollbackError, e.what(), 18753);
Expand Down
Loading

0 comments on commit b1054a0

Please sign in to comment.