kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mpe...@apache.org
Subject [2/2] incubator-kudu git commit: Make RequestTracker not return Status on FirstIncomplete()
Date Thu, 14 Jul 2016 06:13:30 GMT
Make RequestTracker not return Status on FirstIncomplete()

This addresses Todd's post-commit comment on the fact that RequestTracker::FirstIncomplete()
shouldn't return a Status and should return NO_SEQ_NO instead.

Change-Id: I0cdcb2b4c0d2d983bd684b5dccf75a81530da93e
Reviewed-on: http://gerrit.cloudera.org:8080/3504
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <todd@apache.org>


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

Branch: refs/heads/master
Commit: 1a1d3a8d2353766bacff17e745554c61ed1d3286
Parents: 374527a
Author: David Alves <david.alves@cloudera.com>
Authored: Mon Jun 27 01:56:49 2016 -0700
Committer: David Ribeiro Alves <dralves@apache.org>
Committed: Wed Jul 13 05:15:41 2016 +0000

----------------------------------------------------------------------
 src/kudu/rpc/request_tracker-test.cc | 22 ++++++++--------------
 src/kudu/rpc/request_tracker.cc      |  9 +++++----
 src/kudu/rpc/request_tracker.h       |  5 +++--
 3 files changed, 16 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/1a1d3a8d/src/kudu/rpc/request_tracker-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/request_tracker-test.cc b/src/kudu/rpc/request_tracker-test.cc
index 19efef6..89ea8a2 100644
--- a/src/kudu/rpc/request_tracker-test.cc
+++ b/src/kudu/rpc/request_tracker-test.cc
@@ -32,32 +32,28 @@ TEST(RequestTrackerTest, TestSequenceNumberGeneration) {
   scoped_refptr<RequestTracker> tracker_(new RequestTracker("test_client"));
 
   // A new tracker should have no incomplete RPCs
-  ASSERT_TRUE(tracker_->FirstIncomplete(nullptr).IsNotFound());
+  RequestTracker::SequenceNumber seq_no = tracker_->FirstIncomplete();
+  ASSERT_EQ(seq_no, RequestTracker::NO_SEQ_NO);
 
   vector<RequestTracker::SequenceNumber> generated_seq_nos;
 
   // Generate MAX in flight RPCs, making sure they are correctly returned.
   for (int i = 0; i < MAX; i++) {
-    RequestTracker::SequenceNumber seq_no;
     ASSERT_OK(tracker_->NewSeqNo(&seq_no));
     generated_seq_nos.push_back(seq_no);
   }
 
   // Now we should get a first incomplete.
-  RequestTracker::SequenceNumber first_incomplete;
-  ASSERT_OK(tracker_->FirstIncomplete(&first_incomplete));
-  ASSERT_EQ(generated_seq_nos[0], first_incomplete);
+  ASSERT_EQ(generated_seq_nos[0], tracker_->FirstIncomplete());
 
   // Marking 'first_incomplete' as done, should advance the first incomplete.
-  tracker_->RpcCompleted(first_incomplete);
+  tracker_->RpcCompleted(tracker_->FirstIncomplete());
 
-  ASSERT_OK(tracker_->FirstIncomplete(&first_incomplete));
-  ASSERT_EQ(generated_seq_nos[1], first_incomplete);
+  ASSERT_EQ(generated_seq_nos[1], tracker_->FirstIncomplete());
 
   // Marking a 'middle' rpc, should not advance 'first_incomplete'.
   tracker_->RpcCompleted(generated_seq_nos[5]);
-  ASSERT_OK(tracker_->FirstIncomplete(&first_incomplete));
-  ASSERT_EQ(generated_seq_nos[1], first_incomplete);
+  ASSERT_EQ(generated_seq_nos[1], tracker_->FirstIncomplete());
 
   // Marking half the rpc as complete should advance FirstIncomplete.
   // Note that this also tests that RequestTracker::RpcCompleted() is idempotent, i.e. that
@@ -66,11 +62,9 @@ TEST(RequestTrackerTest, TestSequenceNumberGeneration) {
     tracker_->RpcCompleted(generated_seq_nos[i]);
   }
 
-  ASSERT_OK(tracker_->FirstIncomplete(&first_incomplete));
-  ASSERT_EQ(generated_seq_nos[6], first_incomplete);
+  ASSERT_EQ(generated_seq_nos[6], tracker_->FirstIncomplete());
 
   for (int i = MAX / 2; i <= MAX; i++) {
-    RequestTracker::SequenceNumber seq_no;
     ASSERT_OK(tracker_->NewSeqNo(&seq_no));
     generated_seq_nos.push_back(seq_no);
   }
@@ -81,7 +75,7 @@ TEST(RequestTrackerTest, TestSequenceNumberGeneration) {
     tracker_->RpcCompleted(seq_no);
   }
 
-  ASSERT_TRUE(tracker_->FirstIncomplete(nullptr).IsNotFound());
+  ASSERT_EQ(tracker_->FirstIncomplete(), RequestTracker::NO_SEQ_NO);
 }
 
 } // namespace rpc

http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/1a1d3a8d/src/kudu/rpc/request_tracker.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/request_tracker.cc b/src/kudu/rpc/request_tracker.cc
index 79e4578..1958664 100644
--- a/src/kudu/rpc/request_tracker.cc
+++ b/src/kudu/rpc/request_tracker.cc
@@ -24,6 +24,8 @@
 namespace kudu {
 namespace rpc {
 
+const RequestTracker::SequenceNumber RequestTracker::NO_SEQ_NO = -1;
+
 RequestTracker::RequestTracker(const string& client_id)
     : client_id_(client_id),
       next_(0) {}
@@ -36,11 +38,10 @@ Status RequestTracker::NewSeqNo(SequenceNumber* seq_no) {
   return Status::OK();
 }
 
-Status RequestTracker::FirstIncomplete(SequenceNumber* seq_no) {
+RequestTracker::SequenceNumber RequestTracker::FirstIncomplete() {
   std::lock_guard<simple_spinlock> l(lock_);
-  if (incomplete_rpcs_.empty()) return Status::NotFound("There are no incomplete RPCs");
-  *seq_no = *incomplete_rpcs_.begin();
-  return Status::OK();
+  if (incomplete_rpcs_.empty()) return NO_SEQ_NO;
+  return *incomplete_rpcs_.begin();
 }
 
 void RequestTracker::RpcCompleted(const SequenceNumber& seq_no) {

http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/1a1d3a8d/src/kudu/rpc/request_tracker.h
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/request_tracker.h b/src/kudu/rpc/request_tracker.h
index 8147e21..99f8d6c 100644
--- a/src/kudu/rpc/request_tracker.h
+++ b/src/kudu/rpc/request_tracker.h
@@ -48,6 +48,7 @@ namespace rpc {
 class RequestTracker : public RefCountedThreadSafe<RequestTracker> {
  public:
   typedef int64_t SequenceNumber;
+  static const RequestTracker::SequenceNumber NO_SEQ_NO;
   explicit RequestTracker(const std::string& client_id);
 
   // Creates a new, unique, sequence number.
@@ -58,8 +59,8 @@ class RequestTracker : public RefCountedThreadSafe<RequestTracker>
{
   Status NewSeqNo(SequenceNumber* seq_no);
 
   // Returns the sequence number of the first incomplete RPC.
-  // If there is no incomplete RPC returns Status::NotFound. 'seq_no' is not set.
-  Status FirstIncomplete(SequenceNumber* seq_no);
+  // If there is no incomplete RPC returns NO_SEQ_NO.
+  SequenceNumber FirstIncomplete();
 
   // Marks the rpc with 'seq_no' as completed.
   void RpcCompleted(const SequenceNumber& seq_no);


Mime
View raw message