You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by al...@apache.org on 2018/10/25 06:20:18 UTC

kudu git commit: [location_awareness] Add ts location to TSInfoPB

Repository: kudu
Updated Branches:
  refs/heads/master a30c9211e -> c09699ba8


[location_awareness] Add ts location to TSInfoPB

I added the 'location' field into TSInfoPB so that the tool client
can get the ts location info.

Change-Id: Ic5865cb47698817097a7f62c968c5cfd80e07259
Reviewed-on: http://gerrit.cloudera.org:8080/11727
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Alexey Serbin <as...@cloudera.com>


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

Branch: refs/heads/master
Commit: c09699ba8445be8d93aabea8a1cbed1772f374c3
Parents: a30c921
Author: fwang29 <fw...@cloudera.com>
Authored: Thu Oct 18 13:46:02 2018 -0700
Committer: Alexey Serbin <as...@cloudera.com>
Committed: Thu Oct 25 06:19:16 2018 +0000

----------------------------------------------------------------------
 src/kudu/integration-tests/CMakeLists.txt       |   3 +-
 .../integration-tests/scripts/first_argument.sh |  20 ++++
 .../integration-tests/table_locations-itest.cc  | 119 +++++++++++++++----
 src/kudu/master/catalog_manager.cc              |   1 +
 src/kudu/master/master.proto                    |   2 +
 5 files changed, 118 insertions(+), 27 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/c09699ba/src/kudu/integration-tests/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/CMakeLists.txt b/src/kudu/integration-tests/CMakeLists.txt
index 7ecb697..5b8bd81 100644
--- a/src/kudu/integration-tests/CMakeLists.txt
+++ b/src/kudu/integration-tests/CMakeLists.txt
@@ -104,7 +104,8 @@ ADD_KUDU_TEST(security-itest)
 ADD_KUDU_TEST(security-master-auth-itest)
 ADD_KUDU_TEST(security-unknown-tsk-itest PROCESSORS 4)
 ADD_KUDU_TEST(stop_tablet-itest PROCESSORS 4)
-ADD_KUDU_TEST(table_locations-itest)
+ADD_KUDU_TEST(table_locations-itest
+  DATA_FILES scripts/first_argument.sh)
 ADD_KUDU_TEST(tablet_copy-itest NUM_SHARDS 6 PROCESSORS 4)
 ADD_KUDU_TEST(tablet_copy_client_session-itest)
 ADD_KUDU_TEST(tablet_history_gc-itest)

http://git-wip-us.apache.org/repos/asf/kudu/blob/c09699ba/src/kudu/integration-tests/scripts/first_argument.sh
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/scripts/first_argument.sh b/src/kudu/integration-tests/scripts/first_argument.sh
new file mode 100755
index 0000000..832908c
--- /dev/null
+++ b/src/kudu/integration-tests/scripts/first_argument.sh
@@ -0,0 +1,20 @@
+#!/bin/bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# A script that prints the first argument and ignores all the others.
+echo "$1"

http://git-wip-us.apache.org/repos/asf/kudu/blob/c09699ba/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 71fc76d..18acc2b 100644
--- a/src/kudu/integration-tests/table_locations-itest.cc
+++ b/src/kudu/integration-tests/table_locations-itest.cc
@@ -20,6 +20,7 @@
 #include <utility>
 #include <vector>
 
+#include <gflags/gflags_declare.h>
 #include <gtest/gtest.h>
 
 #include "kudu/common/common.pb.h"
@@ -28,6 +29,7 @@
 #include "kudu/common/schema.h"
 #include "kudu/common/wire_protocol.h"
 #include "kudu/common/wire_protocol.pb.h"
+#include "kudu/gutil/strings/substitute.h"
 #include "kudu/master/master.pb.h"
 #include "kudu/master/master.proxy.h"
 #include "kudu/master/mini_master.h"
@@ -36,6 +38,7 @@
 #include "kudu/rpc/rpc_controller.h"
 #include "kudu/util/monotime.h"
 #include "kudu/util/net/sockaddr.h"
+#include "kudu/util/path_util.h"
 #include "kudu/util/pb_util.h"
 #include "kudu/util/status.h"
 #include "kudu/util/test_macros.h"
@@ -53,6 +56,8 @@ using std::string;
 using std::unique_ptr;
 using std::vector;
 
+DECLARE_string(location_mapping_cmd);
+
 namespace kudu {
 namespace master {
 
@@ -67,6 +72,8 @@ class TableLocationsTest : public KuduTest {
   void SetUp() override {
     KuduTest::SetUp();
 
+    SetUpConfig();
+
     InternalMiniClusterOptions opts;
     opts.num_tablet_servers = kNumTabletServers;
 
@@ -88,11 +95,16 @@ class TableLocationsTest : public KuduTest {
     KuduTest::TearDown();
   }
 
+  virtual void SetUpConfig() {}
+
   Status CreateTable(const string& table_name,
                      const Schema& schema,
                      const vector<KuduPartialRow>& split_rows,
                      const vector<pair<KuduPartialRow, KuduPartialRow>>& bounds);
 
+  // Check that the master doesn't give back partial results while the table is being created.
+  void CheckMasterTableCreation(const string &table_name, int tablet_locations_size);
+
   shared_ptr<Messenger> client_messenger_;
   unique_ptr<InternalMiniCluster> cluster_;
   unique_ptr<MasterServiceProxy> proxy_;
@@ -100,9 +112,9 @@ class TableLocationsTest : public KuduTest {
 
 Status TableLocationsTest::CreateTable(const string& table_name,
                                        const Schema& schema,
-                                       const vector<KuduPartialRow>& split_rows,
+                                       const vector<KuduPartialRow>& split_rows = {},
                                        const vector<pair<KuduPartialRow,
-                                                          KuduPartialRow>>& bounds) {
+                                                         KuduPartialRow>>& bounds = {}) {
 
   CreateTableRequestPB req;
   CreateTableResponsePB resp;
@@ -122,6 +134,43 @@ Status TableLocationsTest::CreateTable(const string& table_name,
   return proxy_->CreateTable(req, &resp, &controller);
 }
 
+void TableLocationsTest::CheckMasterTableCreation(const string &table_name,
+                                                  int tablet_locations_size) {
+  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(SecureDebugString(resp));
+
+    if (resp.has_error()) {
+      ASSERT_EQ(MasterErrorPB::TABLET_NOT_RUNNING, resp.error().code());
+      SleepFor(MonoDelta::FromMilliseconds(i * i * 100));
+    } else {
+      ASSERT_EQ(tablet_locations_size, resp.tablet_locations().size());
+      break;
+    }
+  }
+}
+
+// Test the tablet server location is properly set in the master GetTableLocations RPC.
+class TableLocationsWithTSLocationTest : public TableLocationsTest {
+ public:
+  void SetUpConfig() override {
+    const string location_cmd_path = JoinPathSegments(GetTestExecutableDirectory(),
+                                                      "scripts/first_argument.sh");
+    const string location = "/foo";
+    FLAGS_location_mapping_cmd = strings::Substitute("$0 $1", location_cmd_path, location);
+  }
+};
+
 // Test that when the client requests table locations for a non-covered
 // partition range, the master returns the first tablet previous to the begin
 // partition key, as specified in the non-covering range partitions design
@@ -147,30 +196,7 @@ 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(SecureDebugString(resp));
-
-      if (resp.has_error()) {
-        ASSERT_EQ(MasterErrorPB::TABLET_NOT_RUNNING, resp.error().code());
-        SleepFor(MonoDelta::FromMilliseconds(i * i * 100));
-      } else {
-        ASSERT_EQ(8, resp.tablet_locations().size());
-        break;
-      }
-    }
-  }
+  NO_FATALS(CheckMasterTableCreation(table_name, 8));
 
   { // from "a"
     GetTableLocationsRequestPB req;
@@ -237,6 +263,47 @@ TEST_F(TableLocationsTest, TestGetTableLocations) {
     ASSERT_EQ(1, resp.tablet_locations().size());
     EXPECT_EQ("cc", resp.tablet_locations(0).partition().partition_key_start());
   }
+
+  { // Check that the tablet server location should be an empty string if not set.
+    GetTableLocationsRequestPB req;
+    GetTableLocationsResponsePB resp;
+    RpcController controller;
+    req.mutable_table()->set_table_name(table_name);
+    req.set_max_returned_locations(1);
+    ASSERT_OK(proxy_->GetTableLocations(req, &resp, &controller));
+    SCOPED_TRACE(SecureDebugString(resp));
+
+    ASSERT_TRUE(!resp.has_error());
+    ASSERT_EQ(1, resp.tablet_locations().size());
+    ASSERT_EQ(3, resp.tablet_locations(0).replicas_size());
+    for (int i = 0; i < 3; i++) {
+      EXPECT_EQ("", resp.tablet_locations(0).replicas(i).ts_info().location());
+    }
+  }
+}
+
+TEST_F(TableLocationsWithTSLocationTest, TestGetTSLocation) {
+  const string table_name = "test";
+  Schema schema({ ColumnSchema("key", STRING) }, 1);
+  ASSERT_OK(CreateTable(table_name, schema));
+
+  NO_FATALS(CheckMasterTableCreation(table_name, 1));
+
+  {
+    GetTableLocationsRequestPB req;
+    GetTableLocationsResponsePB resp;
+    RpcController controller;
+    req.mutable_table()->set_table_name(table_name);
+    ASSERT_OK(proxy_->GetTableLocations(req, &resp, &controller));
+    SCOPED_TRACE(SecureDebugString(resp));
+
+    ASSERT_TRUE(!resp.has_error());
+    ASSERT_EQ(1, resp.tablet_locations().size());
+    ASSERT_EQ(3, resp.tablet_locations(0).replicas_size());
+    for (int i = 0; i < 3; i++) {
+      ASSERT_EQ("/foo", resp.tablet_locations(0).replicas(i).ts_info().location());
+    }
+  }
 }
 
 } // namespace master

http://git-wip-us.apache.org/repos/asf/kudu/blob/c09699ba/src/kudu/master/catalog_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/catalog_manager.cc b/src/kudu/master/catalog_manager.cc
index 6cb52df..1c75928 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -4447,6 +4447,7 @@ Status CatalogManager::BuildLocationsForTablet(
       ServerRegistrationPB reg;
       ts_desc->GetRegistration(&reg);
       tsinfo_pb->mutable_rpc_addresses()->Swap(reg.mutable_rpc_addresses());
+      if (ts_desc->location()) tsinfo_pb->set_location(*(ts_desc->location()));
     } else {
       // If we've never received a heartbeat from the tserver, we'll fall back
       // to the last known RPC address in the RaftPeerPB.

http://git-wip-us.apache.org/repos/asf/kudu/blob/c09699ba/src/kudu/master/master.proto
----------------------------------------------------------------------
diff --git a/src/kudu/master/master.proto b/src/kudu/master/master.proto
index b525786..393394f 100644
--- a/src/kudu/master/master.proto
+++ b/src/kudu/master/master.proto
@@ -377,6 +377,8 @@ message TSInfoPB {
   required bytes permanent_uuid = 1;
 
   repeated HostPortPB rpc_addresses = 2;
+
+  optional string location = 3;
 }
 
 // Selector to specify policy for listing tablet replicas in