You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by mp...@apache.org on 2016/07/14 06:13:30 UTC
[2/2] incubator-kudu git commit: Make RequestTracker not return
Status on FirstIncomplete()
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 <to...@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 <da...@cloudera.com>
Authored: Mon Jun 27 01:56:49 2016 -0700
Committer: David Ribeiro Alves <dr...@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);