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/08/30 18:05:53 UTC

kudu git commit: KUDU-1586. consensus: always send at least one op

Repository: kudu
Updated Branches:
  refs/heads/master 0d18a0b02 -> 5ba18b623


KUDU-1586. consensus: always send at least one op

This fixes a bug in which the LogCache would not return any operations
in the case of a cache miss on an operation larger than the configured
batch size.

This would cause consensus to get "stuck": the leader would repeatedly
send status-only messages (i.e. no operations) to the follower in a
tight loop, never making any progress.

The new unit test reproduces the problem.

Change-Id: Ibde0e833a3bf02a5f09f2b73b6ab03b188c1e697
Reviewed-on: http://gerrit.cloudera.org:8080/4168
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert <da...@cloudera.com>


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

Branch: refs/heads/master
Commit: 5ba18b623095170b55c2bab90a09258853cce1b8
Parents: 0d18a0b
Author: Todd Lipcon <to...@apache.org>
Authored: Mon Aug 29 22:12:41 2016 -0700
Committer: Todd Lipcon <to...@apache.org>
Committed: Tue Aug 30 18:05:28 2016 +0000

----------------------------------------------------------------------
 src/kudu/consensus/log_cache-test.cc | 6 ++++++
 src/kudu/consensus/log_cache.cc      | 2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/5ba18b62/src/kudu/consensus/log_cache-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/log_cache-test.cc b/src/kudu/consensus/log_cache-test.cc
index b9a71f1..d581f4f 100644
--- a/src/kudu/consensus/log_cache-test.cc
+++ b/src/kudu/consensus/log_cache-test.cc
@@ -172,6 +172,12 @@ TEST_F(LogCacheTest, TestAlwaysYieldsAtLeastOneMessage) {
   OpId preceding;
   ASSERT_OK(cache_->ReadOps(0, 100, &messages, &preceding));
   ASSERT_EQ(1, messages.size());
+
+  // Should yield one op also in the 'cache miss' case.
+  messages.clear();
+  cache_->EvictThroughOp(50);
+  ASSERT_OK(cache_->ReadOps(0, 100, &messages, &preceding));
+  ASSERT_EQ(1, messages.size());
 }
 
 // Tests that the cache returns Status::NotFound() if queried for messages after an

http://git-wip-us.apache.org/repos/asf/kudu/blob/5ba18b62/src/kudu/consensus/log_cache.cc
----------------------------------------------------------------------
diff --git a/src/kudu/consensus/log_cache.cc b/src/kudu/consensus/log_cache.cc
index 789d0fa..b0162aa 100644
--- a/src/kudu/consensus/log_cache.cc
+++ b/src/kudu/consensus/log_cache.cc
@@ -313,7 +313,7 @@ Status LogCache::ReadOps(int64_t after_op_index,
         CHECK_EQ(next_index, msg->id().index());
 
         remaining_space -= TotalByteSizeForMessage(*msg);
-        if (remaining_space > 0) {
+        if (remaining_space > 0 || messages->empty()) {
           messages->push_back(make_scoped_refptr_replicate(msg));
           next_index++;
         } else {