kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From danburk...@apache.org
Subject [2/2] incubator-kudu git commit: Specify guaranteed semantics of GetTableLocations RPC
Date Tue, 31 May 2016 19:59:54 GMT
Specify guaranteed semantics of GetTableLocations RPC

This commit specifies additional semantics guaranteed by the master's
GetTableLocations RPC. Technically this is a breaking change as some of the
existing behavior is changed, but in practice no known published client version
is affected. These guarantees are necessary in order to implement a meta cache
in the presence of non-covering partitions.

Change-Id: Ibc2b5b647e33c0ee8fc052192870f814eff8c178
Reviewed-on: http://gerrit.cloudera.org:8080/3240
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <adar@cloudera.com>


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

Branch: refs/heads/master
Commit: 30d76a36cc4715ee1cea26f80cdd9207239a996c
Parents: 5efc467
Author: Dan Burkert <dan@cloudera.com>
Authored: Thu May 26 12:57:36 2016 -0700
Committer: Dan Burkert <dan@cloudera.com>
Committed: Tue May 31 19:59:23 2016 +0000

----------------------------------------------------------------------
 .../integration-tests/table_locations-itest.cc  | 48 +++++++++++---------
 src/kudu/master/catalog_manager.cc              | 16 ++++++-
 src/kudu/master/catalog_manager.h               |  2 +
 src/kudu/master/master.proto                    | 16 +++++++
 4 files changed, 58 insertions(+), 24 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/30d76a36/src/kudu/integration-tests/table_locations-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/table_locations-itest.cc b/src/kudu/integration-tests/table_locations-itest.cc
index c93b388..24b6cb9 100644
--- a/src/kudu/integration-tests/table_locations-itest.cc
+++ b/src/kudu/integration-tests/table_locations-itest.cc
@@ -109,28 +109,7 @@ Status TableLocationsTest::CreateTable(const string& table_name,
     encoder.Add(RowOperationsPB::RANGE_UPPER_BOUND, bound.second);
   }
 
-  RETURN_NOT_OK(proxy_->CreateTable(req, &resp, &controller));
-  if (resp.has_error()) {
-    RETURN_NOT_OK(StatusFromPB(resp.error().status()));
-  }
-
-  IsCreateTableDoneRequestPB done_req;
-  IsCreateTableDoneResponsePB done_resp;
-  done_req.mutable_table()->set_table_name(table_name);
-
-  for (int i = 1; i <= 20; i++) {
-    controller.Reset();
-    RETURN_NOT_OK(proxy_->IsCreateTableDone(done_req, &done_resp, &controller));
-    if (done_resp.has_error()) {
-      RETURN_NOT_OK(StatusFromPB(done_resp.error().status()));
-    }
-    if (done_resp.done()) {
-      return Status::OK();
-    } else {
-      SleepFor(MonoDelta::FromMilliseconds(i * 100));
-    }
-  }
-  return Status::TimedOut("CreateTable", req.DebugString());
+  return proxy_->CreateTable(req, &resp, &controller);
 }
 
 // Test that when the client requests table locations for a non-covered
@@ -158,6 +137,31 @@ TEST_F(TableLocationsTest, TestGetTableLocations) {
 
   ASSERT_OK(CreateTable(table_name, schema, splits, bounds));
 
+  { // Check that the master doesn't give back partial results while the table is being created.
+    GetTableLocationsRequestPB req;
+    GetTableLocationsResponsePB resp;
+    RpcController controller;
+    req.mutable_table()->set_table_name(table_name);
+
+    for (int i = 1; ; i++) {
+      if (i > 10) {
+        FAIL() << "Create table timed out";
+      }
+
+      controller.Reset();
+      ASSERT_OK(proxy_->GetTableLocations(req, &resp, &controller));
+      SCOPED_TRACE(resp.DebugString());
+
+      if (resp.has_error()) {
+        ASSERT_EQ(MasterErrorPB::TABLET_NOT_RUNNING, resp.error().code());
+        SleepFor(MonoDelta::FromMilliseconds(i * 100));
+      } else {
+        ASSERT_EQ(8, resp.tablet_locations().size());
+        break;
+      }
+    }
+  }
+
   { // from "a"
     GetTableLocationsRequestPB req;
     GetTableLocationsResponsePB resp;

http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/30d76a36/src/kudu/master/catalog_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/catalog_manager.cc b/src/kudu/master/catalog_manager.cc
index c142a08..aac64d8 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -3100,9 +3100,21 @@ Status CatalogManager::GetTableLocations(const GetTableLocationsRequestPB*
req,
   TSRegistrationPB reg;
   vector<TabletReplica> locs;
   for (const scoped_refptr<TabletInfo>& tablet : tablets_in_range) {
-    if (!BuildLocationsForTablet(tablet, resp->add_tablet_locations()).ok()) {
-      // Not running.
+    Status s = BuildLocationsForTablet(tablet, resp->add_tablet_locations());
+    if (s.ok()) {
+      continue;
+    } else if (s.IsNotFound()) {
+      // The tablet has been deleted; filter it from the results.
       resp->mutable_tablet_locations()->RemoveLast();
+    } else if (s.IsServiceUnavailable()) {
+      // The tablet is not yet running; fail the request.
+      resp->Clear();
+      resp->mutable_error()->set_code(MasterErrorPB_Code::MasterErrorPB_Code_TABLET_NOT_RUNNING);
+      StatusToPB(Status::ServiceUnavailable("Tablet not running"),
+                 resp->mutable_error()->mutable_status());
+      break;
+    } else {
+      LOG(FATAL) << "Unexpected error while building tablet locations: " << s.ToString();
     }
   }
   return Status::OK();

http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/30d76a36/src/kudu/master/catalog_manager.h
----------------------------------------------------------------------
diff --git a/src/kudu/master/catalog_manager.h b/src/kudu/master/catalog_manager.h
index e003994..a4e07a3 100644
--- a/src/kudu/master/catalog_manager.h
+++ b/src/kudu/master/catalog_manager.h
@@ -346,6 +346,8 @@ class CatalogManager : public tserver::TabletPeerLookupIf {
   Status ListTables(const ListTablesRequestPB* req,
                     ListTablesResponsePB* resp);
 
+  // Lookup the tablets contained in the partition range of the request.
+  // Returns an error if any of the tablets are not running.
   Status GetTableLocations(const GetTableLocationsRequestPB* req,
                            GetTableLocationsResponsePB* resp);
 

http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/30d76a36/src/kudu/master/master.proto
----------------------------------------------------------------------
diff --git a/src/kudu/master/master.proto b/src/kudu/master/master.proto
index 0fe415d..fbe3506 100644
--- a/src/kudu/master/master.proto
+++ b/src/kudu/master/master.proto
@@ -60,6 +60,9 @@ message MasterErrorPB {
     // The number of replicas requested is greater than the number of live servers
     // in the cluster.
     REPLICATION_FACTOR_TOO_HIGH = 8;
+
+    // The request or response involved a tablet which is not yet running.
+    TABLET_NOT_RUNNING = 9;
   }
 
   // The error code.
@@ -400,6 +403,19 @@ message GetTableLocationsRequestPB {
   optional uint32 max_returned_locations = 5 [ default = 10 ];
 }
 
+// The response to a GetTableLocations RPC. The master guarantees that:
+//
+// * The response contains a location for all tablets in the requested range,
+//   limited by the request's 'max_returned_locations'.
+// * The tablet locations are returned in sorted order by the partition key range.
+// * If *any* tablet in the response is not running, then the entire response
+//   will fail with MasterErrorPB::TABLET_NOT_RUNNING, and the tablet_locations
+//   field will be empty.
+// * A gap between the partition key ranges of consecutive tablets indicates a
+//   non-covered partition range.
+// * If the request's start partition key falls in a non-covered partition
+//   range, the response will contain the tablet immediately before the
+//   non-covered range, if it exists.
 message GetTableLocationsResponsePB {
   // The error, if an error occurred with this request.
   optional MasterErrorPB error = 1;


Mime
View raw message