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/12/02 19:04:11 UTC

[1/3] kudu git commit: [ts_tablet_manager] simplified WaitForAllBootstrapsToFinish

Repository: kudu
Updated Branches:
  refs/heads/master a6c17552a -> b15c0f6e3


[ts_tablet_manager] simplified WaitForAllBootstrapsToFinish

A minor clean-up on TSTabletManager::WaitForAllBootstrapsToFinish()

Change-Id: I3e49b7741f48e619a3d1b93d6bba65f0eb16d849
Reviewed-on: http://gerrit.cloudera.org:8080/5301
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves <dr...@apache.org>
Reviewed-by: Dinesh Bhat <di...@cloudera.com>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/5813f2d8
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/5813f2d8
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/5813f2d8

Branch: refs/heads/master
Commit: 5813f2d8f115eb69d210f1021ab4442d26eaec19
Parents: a6c1755
Author: Alexey Serbin <as...@cloudera.com>
Authored: Wed Nov 30 19:18:12 2016 -0800
Committer: Alexey Serbin <as...@cloudera.com>
Committed: Fri Dec 2 17:29:10 2016 +0000

----------------------------------------------------------------------
 src/kudu/tablet/tablet_peer.cc        | 1 +
 src/kudu/tserver/ts_tablet_manager.cc | 8 ++------
 2 files changed, 3 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/5813f2d8/src/kudu/tablet/tablet_peer.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tablet/tablet_peer.cc b/src/kudu/tablet/tablet_peer.cc
index f636f9c..0eaaefe 100644
--- a/src/kudu/tablet/tablet_peer.cc
+++ b/src/kudu/tablet/tablet_peer.cc
@@ -387,6 +387,7 @@ string TabletPeer::last_status() const {
 
 void TabletPeer::SetFailed(const Status& error) {
   std::lock_guard<simple_spinlock> lock(lock_);
+  CHECK(!error.ok());
   state_ = FAILED;
   error_ = error;
   last_status_ = error.ToString();

http://git-wip-us.apache.org/repos/asf/kudu/blob/5813f2d8/src/kudu/tserver/ts_tablet_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/ts_tablet_manager.cc b/src/kudu/tserver/ts_tablet_manager.cc
index fc637e7..42ed06f 100644
--- a/src/kudu/tserver/ts_tablet_manager.cc
+++ b/src/kudu/tserver/ts_tablet_manager.cc
@@ -231,18 +231,14 @@ Status TSTabletManager::WaitForAllBootstrapsToFinish() {
 
   open_tablet_pool_->Wait();
 
-  Status s = Status::OK();
-
   shared_lock<rw_spinlock> l(lock_);
   for (const TabletMap::value_type& entry : tablet_map_) {
     if (entry.second->state() == tablet::FAILED) {
-      if (s.ok()) {
-        s = entry.second->error();
-      }
+      return entry.second->error();
     }
   }
 
-  return s;
+  return Status::OK();
 }
 
 Status TSTabletManager::CreateNewTablet(const string& table_id,


[3/3] kudu git commit: KUDU-1778. Fix LMP mismatch behavior after a replica restarts

Posted by to...@apache.org.
KUDU-1778. Fix LMP mismatch behavior after a replica restarts

This fixes an issue seen in a stress test after a cluster restart. Both
replicas had an LMP mismatch with the leader, and the tablet was unable
to make progress.

The issue turned out to be that the followers were returning 0 as their
committed index, and the leader then tried to fall back to index 1. That
index had already been GCed, and thus the leader was unable to send any
operations to the followers.

I tested this patch in the same stress test environment and the issue
didn't reproduce. This also includes a test which failed without the
fix. I looped the new test 500 times and it passed.

Change-Id: I8f1332d605f7f846a01923b3ab92f12d73462bba
Reviewed-on: http://gerrit.cloudera.org:8080/5309
Tested-by: Kudu Jenkins
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/b15c0f6e
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/b15c0f6e
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/b15c0f6e

Branch: refs/heads/master
Commit: b15c0f6e3fe0550da07a6665f17415acec0bb042
Parents: fd7048e
Author: Todd Lipcon <to...@apache.org>
Authored: Thu Dec 1 11:57:26 2016 -0800
Committer: Todd Lipcon <to...@apache.org>
Committed: Fri Dec 2 19:01:30 2016 +0000

----------------------------------------------------------------------
 src/kudu/consensus/consensus_peers-test.cc      |  8 +-
 src/kudu/consensus/consensus_queue-test.cc      | 22 ++---
 src/kudu/consensus/consensus_queue.cc           |  4 +-
 src/kudu/consensus/consensus_queue.h            |  3 +-
 src/kudu/consensus/raft_consensus.cc            |  2 +-
 src/kudu/integration-tests/log_verifier.cc      | 47 ++++++++---
 src/kudu/integration-tests/log_verifier.h       | 15 ++++
 .../integration-tests/raft_consensus-itest.cc   | 88 ++++++++++++++++++--
 8 files changed, 155 insertions(+), 34 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/b15c0f6e/src/kudu/consensus/consensus_peers-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/consensus_peers-test.cc b/src/kudu/consensus/consensus_peers-test.cc
index 7b2240a..a41f8b8 100644
--- a/src/kudu/consensus/consensus_peers-test.cc
+++ b/src/kudu/consensus/consensus_peers-test.cc
@@ -137,7 +137,7 @@ class ConsensusPeersTest : public KuduTest {
 TEST_F(ConsensusPeersTest, TestRemotePeer) {
   // We use a majority size of 2 since we make one fake remote peer
   // in addition to our real local log.
-  message_queue_->Init(MinimumOpId());
+  message_queue_->Init(MinimumOpId(), MinimumOpId());
   message_queue_->SetLeaderMode(kMinimumOpIdIndex,
                                 kMinimumTerm,
                                 BuildRaftConfigPBForTests(3));
@@ -161,7 +161,7 @@ TEST_F(ConsensusPeersTest, TestRemotePeer) {
 }
 
 TEST_F(ConsensusPeersTest, TestRemotePeers) {
-  message_queue_->Init(MinimumOpId());
+  message_queue_->Init(MinimumOpId(), MinimumOpId());
   message_queue_->SetLeaderMode(kMinimumOpIdIndex,
                                 kMinimumTerm,
                                 BuildRaftConfigPBForTests(3));
@@ -220,7 +220,7 @@ TEST_F(ConsensusPeersTest, TestRemotePeers) {
 // Regression test for KUDU-699: even if a peer isn't making progress,
 // and thus always has data pending, we should be able to close the peer.
 TEST_F(ConsensusPeersTest, TestCloseWhenRemotePeerDoesntMakeProgress) {
-  message_queue_->Init(MinimumOpId());
+  message_queue_->Init(MinimumOpId(), MinimumOpId());
   message_queue_->SetLeaderMode(kMinimumOpIdIndex,
                                 kMinimumTerm,
                                 BuildRaftConfigPBForTests(3));
@@ -258,7 +258,7 @@ TEST_F(ConsensusPeersTest, TestCloseWhenRemotePeerDoesntMakeProgress) {
 }
 
 TEST_F(ConsensusPeersTest, TestDontSendOneRpcPerWriteWhenPeerIsDown) {
-  message_queue_->Init(MinimumOpId());
+  message_queue_->Init(MinimumOpId(), MinimumOpId());
   message_queue_->SetLeaderMode(kMinimumOpIdIndex,
                                 kMinimumTerm,
                                 BuildRaftConfigPBForTests(3));

http://git-wip-us.apache.org/repos/asf/kudu/blob/b15c0f6e/src/kudu/consensus/consensus_queue-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/consensus_queue-test.cc b/src/kudu/consensus/consensus_queue-test.cc
index 42fa0c0..7f5b464 100644
--- a/src/kudu/consensus/consensus_queue-test.cc
+++ b/src/kudu/consensus/consensus_queue-test.cc
@@ -196,7 +196,7 @@ class ConsensusQueueTest : public KuduTest {
 // with several messages and then starts to track a peer whose watermark
 // falls in the middle of the current messages in the queue.
 TEST_F(ConsensusQueueTest, TestStartTrackingAfterStart) {
-  queue_->Init(MinimumOpId());
+  queue_->Init(MinimumOpId(), MinimumOpId());
   queue_->SetLeaderMode(kMinimumOpIdIndex, kMinimumTerm, BuildRaftConfigPBForTests(2));
   AppendReplicateMessagesToQueue(queue_.get(), clock_, 1, 100);
 
@@ -236,7 +236,7 @@ TEST_F(ConsensusQueueTest, TestStartTrackingAfterStart) {
 // Tests that the peers gets the messages pages, with the size of a page
 // being 'consensus_max_batch_size_bytes'
 TEST_F(ConsensusQueueTest, TestGetPagedMessages) {
-  queue_->Init(MinimumOpId());
+  queue_->Init(MinimumOpId(), MinimumOpId());
   queue_->SetLeaderMode(kMinimumOpIdIndex, kMinimumTerm, BuildRaftConfigPBForTests(2));
 
   // helper to estimate request size so that we can set the max batch size appropriately
@@ -300,7 +300,7 @@ TEST_F(ConsensusQueueTest, TestGetPagedMessages) {
 }
 
 TEST_F(ConsensusQueueTest, TestPeersDontAckBeyondWatermarks) {
-  queue_->Init(MinimumOpId());
+  queue_->Init(MinimumOpId(), MinimumOpId());
   queue_->SetLeaderMode(kMinimumOpIdIndex, kMinimumTerm, BuildRaftConfigPBForTests(3));
   AppendReplicateMessagesToQueue(queue_.get(), clock_, 1, 100);
 
@@ -367,7 +367,7 @@ TEST_F(ConsensusQueueTest, TestPeersDontAckBeyondWatermarks) {
 }
 
 TEST_F(ConsensusQueueTest, TestQueueAdvancesCommittedIndex) {
-  queue_->Init(MinimumOpId());
+  queue_->Init(MinimumOpId(), MinimumOpId());
   queue_->SetLeaderMode(kMinimumOpIdIndex, kMinimumTerm, BuildRaftConfigPBForTests(5));
   // Track 4 additional peers (in addition to the local peer)
   queue_->TrackPeer("peer-1");
@@ -466,7 +466,7 @@ TEST_F(ConsensusQueueTest, TestQueueLoadsOperationsForPeer) {
   // the last operation in the log.
   CloseAndReopenQueue();
 
-  queue_->Init(leader_last_op);
+  queue_->Init(leader_last_op, leader_last_op);
   queue_->SetLeaderMode(leader_last_op.index(),
                         leader_last_op.term(),
                         BuildRaftConfigPBForTests(3));
@@ -539,7 +539,7 @@ TEST_F(ConsensusQueueTest, TestQueueHandlesOperationOverwriting) {
   OpId last_in_log;
   log_->GetLatestEntryOpId(&last_in_log);
   int64_t committed_index = 15;
-  queue_->Init(last_in_log);
+  queue_->Init(last_in_log, MakeOpId(2, committed_index));
   queue_->SetLeaderMode(committed_index,
                         last_in_log.term(),
                         BuildRaftConfigPBForTests(3));
@@ -619,7 +619,7 @@ TEST_F(ConsensusQueueTest, TestQueueHandlesOperationOverwriting) {
 // operations, which would cause a check failure on the write immediately
 // following the overwriting write.
 TEST_F(ConsensusQueueTest, TestQueueMovesWatermarksBackward) {
-  queue_->Init(MinimumOpId());
+  queue_->Init(MinimumOpId(), MinimumOpId());
   queue_->SetNonLeaderMode();
   // Append a bunch of messages.
   AppendReplicateMessagesToQueue(queue_.get(), clock_, 1, 10);
@@ -679,8 +679,8 @@ TEST_F(ConsensusQueueTest, TestQueueMovesWatermarksBackward) {
 TEST_F(ConsensusQueueTest, TestOnlyAdvancesWatermarkWhenPeerHasAPrefixOfOurLog) {
   FLAGS_consensus_max_batch_size_bytes = 1024 * 10;
 
-  const int kInitialCommittedIndex = 31;
-  queue_->Init(MakeOpId(72, 30));
+  const int kInitialCommittedIndex = 30;
+  queue_->Init(MakeOpId(72, 30), MakeOpId(82, 30));
   queue_->SetLeaderMode(kInitialCommittedIndex, 76, BuildRaftConfigPBForTests(3));
 
   ConsensusRequestPB request;
@@ -774,7 +774,7 @@ TEST_F(ConsensusQueueTest, TestOnlyAdvancesWatermarkWhenPeerHasAPrefixOfOurLog)
 
 // Test that Tablet Copy is triggered when a "tablet not found" error occurs.
 TEST_F(ConsensusQueueTest, TestTriggerTabletCopyIfTabletNotFound) {
-  queue_->Init(MinimumOpId());
+  queue_->Init(MinimumOpId(), MinimumOpId());
   queue_->SetLeaderMode(kMinimumOpIdIndex, kMinimumTerm, BuildRaftConfigPBForTests(3));
   AppendReplicateMessagesToQueue(queue_.get(), clock_, 1, 100);
 
@@ -814,7 +814,7 @@ TEST_F(ConsensusQueueTest, TestTriggerTabletCopyIfTabletNotFound) {
 }
 
 TEST_F(ConsensusQueueTest, TestFollowerCommittedIndexAndMetrics) {
-  queue_->Init(MinimumOpId());
+  queue_->Init(MinimumOpId(), MinimumOpId());
   queue_->SetNonLeaderMode();
 
   AppendReplicateMessagesToQueue(queue_.get(), clock_, 1, 10);

http://git-wip-us.apache.org/repos/asf/kudu/blob/b15c0f6e/src/kudu/consensus/consensus_queue.cc
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/consensus_queue.cc b/src/kudu/consensus/consensus_queue.cc
index 1d06adb..69bc212 100644
--- a/src/kudu/consensus/consensus_queue.cc
+++ b/src/kudu/consensus/consensus_queue.cc
@@ -119,12 +119,14 @@ PeerMessageQueue::PeerMessageQueue(const scoped_refptr<MetricEntity>& metric_ent
   CHECK_OK(ThreadPoolBuilder("queue-observers-pool").set_max_threads(1).Build(&observers_pool_));
 }
 
-void PeerMessageQueue::Init(const OpId& last_locally_replicated) {
+void PeerMessageQueue::Init(const OpId& last_locally_replicated,
+                            const OpId& last_locally_committed) {
   std::lock_guard<simple_spinlock> lock(queue_lock_);
   CHECK_EQ(queue_state_.state, kQueueConstructed);
   log_cache_.Init(last_locally_replicated);
   queue_state_.last_appended = last_locally_replicated;
   queue_state_.state = kQueueOpen;
+  queue_state_.committed_index = last_locally_committed.index();
   TrackPeerUnlocked(local_peer_pb_.permanent_uuid());
 }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/b15c0f6e/src/kudu/consensus/consensus_queue.h
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/consensus_queue.h b/src/kudu/consensus/consensus_queue.h
index 58904d5..761e258 100644
--- a/src/kudu/consensus/consensus_queue.h
+++ b/src/kudu/consensus/consensus_queue.h
@@ -133,7 +133,8 @@ class PeerMessageQueue {
                    const std::string& tablet_id);
 
   // Initialize the queue.
-  void Init(const OpId& last_locally_replicated);
+  void Init(const OpId& last_locally_replicated,
+            const OpId& last_locally_committed);
 
   // Changes the queue to leader mode, meaning it tracks majority replicated
   // operations and notifies observers when those change.

http://git-wip-us.apache.org/repos/asf/kudu/blob/b15c0f6e/src/kudu/consensus/raft_consensus.cc
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/raft_consensus.cc b/src/kudu/consensus/raft_consensus.cc
index 385a2c1..3ee0efc 100644
--- a/src/kudu/consensus/raft_consensus.cc
+++ b/src/kudu/consensus/raft_consensus.cc
@@ -294,7 +294,7 @@ Status RaftConsensus::Start(const ConsensusBootstrapInfo& info) {
 
     pending_.SetInitialCommittedOpId(info.last_committed_id);
 
-    queue_->Init(info.last_id);
+    queue_->Init(info.last_id, info.last_committed_id);
   }
 
   {

http://git-wip-us.apache.org/repos/asf/kudu/blob/b15c0f6e/src/kudu/integration-tests/log_verifier.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/log_verifier.cc b/src/kudu/integration-tests/log_verifier.cc
index cbd1933..9a8f919 100644
--- a/src/kudu/integration-tests/log_verifier.cc
+++ b/src/kudu/integration-tests/log_verifier.cc
@@ -26,6 +26,7 @@
 
 #include <glog/stl_logging.h>
 
+#include "kudu/consensus/consensus.pb.h"
 #include "kudu/consensus/log_index.h"
 #include "kudu/consensus/log_reader.h"
 #include "kudu/consensus/log_util.h"
@@ -40,11 +41,13 @@ using std::map;
 using std::set;
 using std::shared_ptr;
 using std::string;
+using std::unique_ptr;
 using std::vector;
 using strings::Substitute;
 
 namespace kudu {
 
+using consensus::OpId;
 using log::LogReader;
 using itest::ExternalMiniClusterFsInspector;
 
@@ -55,6 +58,19 @@ LogVerifier::LogVerifier(ExternalMiniCluster* cluster)
 LogVerifier::~LogVerifier() {
 }
 
+Status LogVerifier::OpenFsManager(ExternalTabletServer* ets,
+                                  unique_ptr<FsManager>* fs) {
+  const string& data_dir = ets->data_dir();
+  FsManagerOpts fs_opts;
+  fs_opts.read_only = true;
+  fs_opts.wal_path = data_dir;
+  unique_ptr<FsManager> ret(new FsManager(Env::Default(), fs_opts));
+  RETURN_NOT_OK_PREPEND(ret->Open(),
+                        Substitute("Couldn't initialize FS Manager for $0", data_dir));
+  fs->swap(ret);
+  return Status::OK();
+}
+
 Status LogVerifier::ScanForCommittedOpIds(FsManager* fs, const string& tablet_id,
                                           map<int64_t, int64_t>* index_to_term) {
 
@@ -84,6 +100,23 @@ Status LogVerifier::ScanForCommittedOpIds(FsManager* fs, const string& tablet_id
   return Status::OK();
 }
 
+Status LogVerifier::ScanForHighestCommittedOpIdInLog(ExternalTabletServer* ets,
+                                                     const string& tablet_id,
+                                                     OpId* commit_id) {
+  unique_ptr<FsManager> fs;
+  RETURN_NOT_OK(OpenFsManager(ets, &fs));
+  const string& wal_dir = fs->GetTabletWalDir(tablet_id);
+  map<int64_t, int64_t> index_to_term;
+  RETURN_NOT_OK_PREPEND(ScanForCommittedOpIds(fs.get(), tablet_id, &index_to_term),
+                        Substitute("Couldn't scan log in dir $0", wal_dir));
+  if (index_to_term.empty()) {
+    return Status::NotFound("no COMMITs in log");
+  }
+  commit_id->set_index(index_to_term.rbegin()->first);
+  commit_id->set_term(index_to_term.rbegin()->second);
+  return Status::OK();
+}
+
 Status LogVerifier::VerifyCommittedOpIdsMatch() {
   ExternalMiniClusterFsInspector inspect(cluster_);
   Env* env = Env::Default();
@@ -99,18 +132,12 @@ Status LogVerifier::VerifyCommittedOpIdsMatch() {
     // Gather the [index->term] map for each of the tablet servers
     // hosting this tablet.
     for (int i = 0; i < cluster_->num_tablet_servers(); i++) {
-      const string& data_dir = cluster_->tablet_server(i)->data_dir();
-
-      FsManagerOpts fs_opts;
-      fs_opts.read_only = true;
-      fs_opts.wal_path = data_dir;
-      FsManager fs(env, fs_opts);
-      RETURN_NOT_OK_PREPEND(fs.Open(),
-                            Substitute("Couldn't initialize FS Manager for $0", data_dir));
-      const string& wal_dir = fs.GetTabletWalDir(tablet_id);
+      unique_ptr<FsManager> fs;
+      RETURN_NOT_OK(OpenFsManager(cluster_->tablet_server(i), &fs));
+      const string& wal_dir = fs->GetTabletWalDir(tablet_id);
       if (!env->FileExists(wal_dir)) continue;
       map<int64_t, int64_t> index_to_term;
-      RETURN_NOT_OK_PREPEND(ScanForCommittedOpIds(&fs, tablet_id, &index_to_term),
+      RETURN_NOT_OK_PREPEND(ScanForCommittedOpIds(fs.get(), tablet_id, &index_to_term),
                             Substitute("Couldn't scan log for TS $0", i));
       for (const auto& index_term : index_to_term) {
         all_op_indexes.insert(index_term.first);

http://git-wip-us.apache.org/repos/asf/kudu/blob/b15c0f6e/src/kudu/integration-tests/log_verifier.h
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/log_verifier.h b/src/kudu/integration-tests/log_verifier.h
index cb33958..d029863 100644
--- a/src/kudu/integration-tests/log_verifier.h
+++ b/src/kudu/integration-tests/log_verifier.h
@@ -17,6 +17,7 @@
 #pragma once
 
 #include <map>
+#include <memory>
 #include <string>
 
 #include "kudu/gutil/macros.h"
@@ -25,8 +26,13 @@
 namespace kudu {
 
 class ExternalMiniCluster;
+class ExternalTabletServer;
 class FsManager;
 
+namespace consensus {
+class OpId;
+} // namespace consensus
+
 // Verifies correctness of the logs in an external mini-cluster.
 class LogVerifier {
  public:
@@ -46,7 +52,16 @@ class LogVerifier {
   // loop and retry on failure.
   Status VerifyCommittedOpIdsMatch();
 
+  // Scans the WAL on the given tablet server to find the COMMIT message with the highest
+  // index.
+  Status ScanForHighestCommittedOpIdInLog(ExternalTabletServer* ets,
+                                          const std::string& tablet_id,
+                                          consensus::OpId* commit_id);
+
  private:
+  // Open an FsManager for the given tablet server.
+  Status OpenFsManager(ExternalTabletServer* ets, std::unique_ptr<FsManager>* fs);
+
   // Scan the WALs for tablet 'tablet_id' on the given 'fs'. Sets entries
   // in '*index_to_term' for each COMMIT entry found in the WALs.
   Status ScanForCommittedOpIds(FsManager* fs, const std::string& tablet_id,

http://git-wip-us.apache.org/repos/asf/kudu/blob/b15c0f6e/src/kudu/integration-tests/raft_consensus-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/raft_consensus-itest.cc b/src/kudu/integration-tests/raft_consensus-itest.cc
index 6582626..8d016b0 100644
--- a/src/kudu/integration-tests/raft_consensus-itest.cc
+++ b/src/kudu/integration-tests/raft_consensus-itest.cc
@@ -37,6 +37,7 @@
 #include "kudu/gutil/strings/strcat.h"
 #include "kudu/gutil/strings/util.h"
 #include "kudu/integration-tests/cluster_verifier.h"
+#include "kudu/integration-tests/log_verifier.h"
 #include "kudu/integration-tests/test_workload.h"
 #include "kudu/integration-tests/ts_itest-base.h"
 #include "kudu/server/server_base.pb.h"
@@ -354,6 +355,10 @@ class RaftConsensusITest : public TabletServerIntegrationTestBase {
   static const bool WITHOUT_NOTIFICATION_LATENCY = false;
   void DoTestChurnyElections(bool with_latency);
 
+  // Prepare for a test where a single replica of a 3-server cluster is left
+  // running as a follower.
+  void SetupSingleReplicaTest(TServerDetails** replica_ts);
+
  protected:
   // Flags needed for CauseFollowerToFallBehindLogGC() to work well.
   void AddFlagsForLogRolls(vector<string>* extra_tserver_flags);
@@ -1181,10 +1186,7 @@ void RaftConsensusITest::AddOp(const OpId& id, ConsensusRequestPB* req) {
                  id.ShortDebugString(), write_req->mutable_row_operations());
 }
 
-// Regression test for KUDU-644:
-// Triggers some complicated scenarios on the replica involving aborting and
-// replacing transactions.
-TEST_F(RaftConsensusITest, TestReplicaBehaviorViaRPC) {
+void RaftConsensusITest::SetupSingleReplicaTest(TServerDetails** replica_ts) {
   FLAGS_num_replicas = 3;
   FLAGS_num_tablet_servers = 3;
   vector<string> ts_flags, master_flags;
@@ -1193,7 +1195,6 @@ TEST_F(RaftConsensusITest, TestReplicaBehaviorViaRPC) {
   BuildAndStart(ts_flags, master_flags);
 
   // Kill all the servers but one.
-  TServerDetails *replica_ts;
   vector<TServerDetails*> tservers;
   AppendValuesFromMap(tablet_servers_, &tservers);
   ASSERT_EQ(3, tservers.size());
@@ -1202,11 +1203,86 @@ TEST_F(RaftConsensusITest, TestReplicaBehaviorViaRPC) {
   ASSERT_OK(StartElection(tservers[2], tablet_id_, MonoDelta::FromSeconds(10)));
   ASSERT_OK(WaitForServersToAgree(MonoDelta::FromSeconds(10), tablet_servers_, tablet_id_, 1));
 
-  replica_ts = tservers[0];
   cluster_->tablet_server_by_uuid(tservers[1]->uuid())->Shutdown();
   cluster_->tablet_server_by_uuid(tservers[2]->uuid())->Shutdown();
 
+  *replica_ts = tservers[0];
   LOG(INFO) << "================================== Cluster setup complete.";
+}
+
+// Regression test for KUDU-1775: when a replica is restarted, and the first
+// request it receives from a leader results in a LMP mismatch error, the
+// replica should still respond with the correct 'last_committed_idx'.
+TEST_F(RaftConsensusITest, TestLMPMismatchOnRestartedReplica) {
+  TServerDetails* replica_ts;
+  NO_FATALS(SetupSingleReplicaTest(&replica_ts));
+  auto* replica_ets = cluster_->tablet_server_by_uuid(replica_ts->uuid());
+
+  ConsensusServiceProxy* c_proxy = CHECK_NOTNULL(replica_ts->consensus_proxy.get());
+  ConsensusRequestPB req;
+  ConsensusResponsePB resp;
+  RpcController rpc;
+
+  req.set_tablet_id(tablet_id_);
+  req.set_dest_uuid(replica_ts->uuid());
+  req.set_caller_uuid("fake_caller");
+  req.set_caller_term(2);
+  req.set_all_replicated_index(0);
+  req.mutable_preceding_id()->CopyFrom(MakeOpId(1, 1));
+
+  ASSERT_OK(c_proxy->UpdateConsensus(req, &resp, &rpc));
+  ASSERT_FALSE(resp.has_error()) << resp.DebugString();
+
+  // Send operations 2.1 through 2.3, committing through 2.2.
+  AddOp(MakeOpId(2, 1), &req);
+  AddOp(MakeOpId(2, 2), &req);
+  AddOp(MakeOpId(2, 3), &req);
+  req.set_committed_index(2);
+  rpc.Reset();
+  ASSERT_OK(c_proxy->UpdateConsensus(req, &resp, &rpc));
+  ASSERT_FALSE(resp.has_error()) << resp.DebugString();
+
+  // The COMMIT messages end up in the WAL asynchronously, so loop reading the
+  // tablet server's WAL until it shows up.
+  AssertEventually([&]() {
+      LogVerifier lv(cluster_.get());
+      OpId commit;
+      ASSERT_OK(lv.ScanForHighestCommittedOpIdInLog(replica_ets, tablet_id_, &commit));
+      ASSERT_EQ("2.2", OpIdToString(commit));
+    });
+
+  // Restart the replica.
+  replica_ets->Shutdown();
+  ASSERT_OK(replica_ets->Restart());
+
+  // Send an operation 3.4 with preceding OpId 3.3.
+  // We expect an LMP mismatch, since the replica has operation 2.3.
+  // We use 'AssertEventually' here because the replica
+  // may need a few retries while it's in BOOTSTRAPPING state.
+  req.set_caller_term(3);
+  req.mutable_preceding_id()->CopyFrom(MakeOpId(3, 3));
+  req.clear_ops();
+  AddOp(MakeOpId(3, 4), &req);
+  AssertEventually([&]() {
+      rpc.Reset();
+      ASSERT_OK(c_proxy->UpdateConsensus(req, &resp, &rpc));
+      ASSERT_EQ(resp.status().error().code(),
+                consensus::ConsensusErrorPB::PRECEDING_ENTRY_DIDNT_MATCH) << resp.DebugString();
+    });
+  SCOPED_TRACE(resp.DebugString());
+  EXPECT_EQ(2, resp.status().last_committed_idx());
+  EXPECT_EQ("0.0", OpIdToString(resp.status().last_received_current_leader()));
+  // Even though the replica previously received operations through 2.3, the LMP mismatch
+  // above causes us to truncate operation 2.3, so 2.2 remains.
+  EXPECT_EQ("2.2", OpIdToString(resp.status().last_received()));
+}
+
+// Regression test for KUDU-644:
+// Triggers some complicated scenarios on the replica involving aborting and
+// replacing transactions.
+TEST_F(RaftConsensusITest, TestReplicaBehaviorViaRPC) {
+  TServerDetails* replica_ts;
+  NO_FATALS(SetupSingleReplicaTest(&replica_ts));
 
   // Check that the 'term' metric is correctly exposed.
   {


[2/3] kudu git commit: java tests: Remove duplicate "Starting masters" message

Posted by to...@apache.org.
java tests: Remove duplicate "Starting masters" message

This message is printed twice when tests are running, and it's a little
confusing. This patch removes one of the instances of this message.

Change-Id: I8286c4b310f991217e385f452df231dc7d949f40
Reviewed-on: http://gerrit.cloudera.org:8080/5329
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert <da...@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/fd7048e2
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/fd7048e2
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/fd7048e2

Branch: refs/heads/master
Commit: fd7048e21211bb76b215572231245125e6b48c75
Parents: 5813f2d
Author: Mike Percy <mp...@apache.org>
Authored: Fri Dec 2 16:37:49 2016 +0000
Committer: Mike Percy <mp...@apache.org>
Committed: Fri Dec 2 18:40:54 2016 +0000

----------------------------------------------------------------------
 .../src/test/java/org/apache/kudu/client/MiniKuduCluster.java      | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/fd7048e2/java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java b/java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java
index 8e79dca..ed49a1c 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java
@@ -190,6 +190,7 @@ public class MiniKuduCluster implements AutoCloseable {
     long now = System.currentTimeMillis();
     LOG.info("Starting {} masters...", numMasters);
     int startPort = startMasters(PORT_START, numMasters, baseDirPath, bindHost);
+
     LOG.info("Starting {} tablet servers...", numTservers);
     List<Integer> ports = TestUtils.findFreePorts(startPort, numTservers * 2);
     for (int i = 0; i < numTservers; i++) {
@@ -243,7 +244,6 @@ public class MiniKuduCluster implements AutoCloseable {
                            int numMasters,
                            String baseDirPath,
                            String bindHost) throws Exception {
-    LOG.info("Starting {} masters...", numMasters);
     // Get the list of web and RPC ports to use for the master consensus configuration:
     // request NUM_MASTERS * 2 free ports as we want to also reserve the web
     // ports for the consensus configuration.