You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by to...@apache.org on 2016/10/19 22:16:09 UTC
[6/7] kudu git commit: consensus: move ReplicaTransactionFactory into
RaftConsensus
consensus: move ReplicaTransactionFactory into RaftConsensus
The ReplicaState::txn_factory_ member was only used by a getter and
handed back to RaftConsensus, so this just moves the instance into
RaftConsensus itself.
This also enabled removal of some test cruft.
Change-Id: I45906913aed375f860276455223e43f0115a5c0a
Reviewed-on: http://gerrit.cloudera.org:8080/4711
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves <dr...@apache.org>
Reviewed-by: Mike Percy <mp...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/03d967ae
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/03d967ae
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/03d967ae
Branch: refs/heads/master
Commit: 03d967ae27f3d00690c33ac4dbe0a8bcc49651f4
Parents: 01e9168
Author: Todd Lipcon <to...@apache.org>
Authored: Wed Oct 12 19:45:09 2016 -0700
Committer: Todd Lipcon <to...@apache.org>
Committed: Wed Oct 19 22:07:55 2016 +0000
----------------------------------------------------------------------
src/kudu/consensus/consensus-test-util.h | 10 ----------
src/kudu/consensus/raft_consensus.cc | 7 +++----
src/kudu/consensus/raft_consensus.h | 4 ++++
src/kudu/consensus/raft_consensus_state-test.cc | 7 ++-----
src/kudu/consensus/raft_consensus_state.cc | 8 +-------
src/kudu/consensus/raft_consensus_state.h | 9 +--------
6 files changed, 11 insertions(+), 34 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/03d967ae/src/kudu/consensus/consensus-test-util.h
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/consensus-test-util.h b/src/kudu/consensus/consensus-test-util.h
index 80f6a6f..ce30bfb 100644
--- a/src/kudu/consensus/consensus-test-util.h
+++ b/src/kudu/consensus/consensus-test-util.h
@@ -631,16 +631,6 @@ class TestDriver {
Log* log_;
};
-// Fake ReplicaTransactionFactory that allows for instantiating and unit
-// testing RaftConsensusState. Does not actually support running transactions.
-class MockTransactionFactory : public ReplicaTransactionFactory {
- public:
- virtual Status StartReplicaTransaction(const scoped_refptr<ConsensusRound>& round) OVERRIDE {
- return StartReplicaTransactionMock(round.get());
- }
- MOCK_METHOD1(StartReplicaTransactionMock, Status(ConsensusRound* round));
-};
-
// A transaction factory for tests, usually this is implemented by TabletPeer.
class TestTransactionFactory : public ReplicaTransactionFactory {
public:
http://git-wip-us.apache.org/repos/asf/kudu/blob/03d967ae/src/kudu/consensus/raft_consensus.cc
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/raft_consensus.cc b/src/kudu/consensus/raft_consensus.cc
index 6e4b708..67021a5 100644
--- a/src/kudu/consensus/raft_consensus.cc
+++ b/src/kudu/consensus/raft_consensus.cc
@@ -231,6 +231,7 @@ RaftConsensus::RaftConsensus(
log_(log),
clock_(clock),
peer_proxy_factory_(std::move(peer_proxy_factory)),
+ txn_factory_(txn_factory),
peer_manager_(std::move(peer_manager)),
queue_(std::move(queue)),
rng_(GetRandomSeed32()),
@@ -253,8 +254,7 @@ RaftConsensus::RaftConsensus(
DCHECK(log_);
state_.reset(new ReplicaState(options,
peer_uuid,
- std::move(cmeta),
- DCHECK_NOTNULL(txn_factory)));
+ std::move(cmeta)));
}
RaftConsensus::~RaftConsensus() {
@@ -785,8 +785,7 @@ Status RaftConsensus::StartReplicaTransactionUnlocked(const ReplicateRefPtr& msg
VLOG_WITH_PREFIX_UNLOCKED(1) << "Starting transaction: " << msg->get()->id().ShortDebugString();
scoped_refptr<ConsensusRound> round(new ConsensusRound(this, msg));
ConsensusRound* round_ptr = round.get();
- RETURN_NOT_OK(state_->GetReplicaTransactionFactoryUnlocked()->
- StartReplicaTransaction(round));
+ RETURN_NOT_OK(txn_factory_->StartReplicaTransaction(round));
return AddPendingOperationUnlocked(round_ptr);
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/03d967ae/src/kudu/consensus/raft_consensus.h
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/raft_consensus.h b/src/kudu/consensus/raft_consensus.h
index 866b2a9..2cd80c6 100644
--- a/src/kudu/consensus/raft_consensus.h
+++ b/src/kudu/consensus/raft_consensus.h
@@ -444,6 +444,10 @@ class RaftConsensus : public Consensus,
scoped_refptr<server::Clock> clock_;
gscoped_ptr<PeerProxyFactory> peer_proxy_factory_;
+ // When we receive a message from a remote peer telling us to start a transaction, we use
+ // this factory to start it.
+ ReplicaTransactionFactory* txn_factory_;
+
gscoped_ptr<PeerManager> peer_manager_;
// The queue of messages that must be sent to peers.
http://git-wip-us.apache.org/repos/asf/kudu/blob/03d967ae/src/kudu/consensus/raft_consensus_state-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/raft_consensus_state-test.cc b/src/kudu/consensus/raft_consensus_state-test.cc
index f5d4fa9..0665e03 100644
--- a/src/kudu/consensus/raft_consensus_state-test.cc
+++ b/src/kudu/consensus/raft_consensus_state-test.cc
@@ -38,8 +38,7 @@ const char* kTabletId = "TestTablet";
class RaftConsensusStateTest : public KuduTest {
public:
RaftConsensusStateTest()
- : fs_manager_(env_.get(), GetTestPath("fs_root")),
- txn_factory_(new MockTransactionFactory()) {
+ : fs_manager_(env_.get(), GetTestPath("fs_root")) {
}
virtual void SetUp() OVERRIDE {
@@ -56,8 +55,7 @@ class RaftConsensusStateTest : public KuduTest {
unique_ptr<ConsensusMetadata> cmeta;
ASSERT_OK(ConsensusMetadata::Create(&fs_manager_, kTabletId, fs_manager_.uuid(),
config_, kMinimumTerm, &cmeta));
- state_.reset(new ReplicaState(ConsensusOptions(), fs_manager_.uuid(), std::move(cmeta),
- txn_factory_.get()));
+ state_.reset(new ReplicaState(ConsensusOptions(), fs_manager_.uuid(), std::move(cmeta)));
// Start up the ReplicaState.
ReplicaState::UniqueLock lock;
@@ -68,7 +66,6 @@ class RaftConsensusStateTest : public KuduTest {
protected:
FsManager fs_manager_;
RaftConfigPB config_;
- gscoped_ptr<MockTransactionFactory> txn_factory_;
gscoped_ptr<ReplicaState> state_;
};
http://git-wip-us.apache.org/repos/asf/kudu/blob/03d967ae/src/kudu/consensus/raft_consensus_state.cc
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/raft_consensus_state.cc b/src/kudu/consensus/raft_consensus_state.cc
index 366af51..9494f68 100644
--- a/src/kudu/consensus/raft_consensus_state.cc
+++ b/src/kudu/consensus/raft_consensus_state.cc
@@ -43,12 +43,10 @@ using strings::SubstituteAndAppend;
//////////////////////////////////////////////////
ReplicaState::ReplicaState(ConsensusOptions options, string peer_uuid,
- unique_ptr<ConsensusMetadata> cmeta,
- ReplicaTransactionFactory* txn_factory)
+ unique_ptr<ConsensusMetadata> cmeta)
: options_(std::move(options)),
peer_uuid_(std::move(peer_uuid)),
cmeta_(std::move(cmeta)),
- txn_factory_(txn_factory),
last_committed_op_id_(MinimumOpId()),
state_(kInitialized) {
CHECK(cmeta_) << "ConsensusMeta passed as NULL";
@@ -326,10 +324,6 @@ const std::string& ReplicaState::GetVotedForCurrentTermUnlocked() const {
return cmeta_->voted_for();
}
-ReplicaTransactionFactory* ReplicaState::GetReplicaTransactionFactoryUnlocked() const {
- return txn_factory_;
-}
-
const string& ReplicaState::GetPeerUuid() const {
return peer_uuid_;
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/03d967ae/src/kudu/consensus/raft_consensus_state.h
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/raft_consensus_state.h b/src/kudu/consensus/raft_consensus_state.h
index ed53deb..e95b499 100644
--- a/src/kudu/consensus/raft_consensus_state.h
+++ b/src/kudu/consensus/raft_consensus_state.h
@@ -96,8 +96,7 @@ class ReplicaState {
typedef IndexToRoundMap::value_type IndexToRoundEntry;
ReplicaState(ConsensusOptions options, std::string peer_uuid,
- std::unique_ptr<ConsensusMetadata> cmeta,
- ReplicaTransactionFactory* txn_factory);
+ std::unique_ptr<ConsensusMetadata> cmeta);
Status StartUnlocked(const OpId& last_id_in_wal);
@@ -227,8 +226,6 @@ class ReplicaState {
// The vote must be set; use HasVotedCurrentTermUnlocked() to check.
const std::string& GetVotedForCurrentTermUnlocked() const;
- ReplicaTransactionFactory* GetReplicaTransactionFactoryUnlocked() const;
-
// Returns the uuid of the peer to which this replica state belongs.
// Safe to call with or without locks held.
const std::string& GetPeerUuid() const;
@@ -325,10 +322,6 @@ class ReplicaState {
// The key is the index of the replicate operation.
IndexToRoundMap pending_txns_;
- // When we receive a message from a remote peer telling us to start a transaction, we use
- // this factory to start it.
- ReplicaTransactionFactory* txn_factory_;
-
// The OpId of the Apply that was last triggered when the last message from the leader
// was received. Initialized to MinimumOpId().
//