kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ale...@apache.org
Subject [1/3] kudu git commit: [location_awareness] Assign locations to clients
Date Thu, 03 Jan 2019 01:53:59 GMT
Repository: kudu
Updated Branches:
  refs/heads/master 236fdfe83 -> c7a2d69fb


[location_awareness] Assign locations to clients

This patch makes it so the client is assigned a location by each master
as part of the ConnectToMaster RPC (which is part of the
ConnectToCluster process). The client will store the location it is
assigned by the leader master and potentially update it every time it
reconnects to the cluster.

This assignment will be used in a follow-up to implement location-aware
semantics for CLOSEST_REPLICA scans.

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


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

Branch: refs/heads/master
Commit: 4a3e7c0006555c872d0e3a0d78b94a1edfab0d21
Parents: 236fdfe
Author: fwang29 <fwang@cloudera.com>
Authored: Mon Nov 12 23:52:42 2018 -0800
Committer: Will Berkeley <wdberkeley@gmail.com>
Committed: Fri Dec 21 23:03:08 2018 +0000

----------------------------------------------------------------------
 src/kudu/client/CMakeLists.txt     |  3 ++-
 src/kudu/client/client-internal.cc |  2 ++
 src/kudu/client/client-internal.h  |  7 +++++-
 src/kudu/client/client-test.cc     | 31 +++++++++++++++++++++--
 src/kudu/client/client.cc          |  5 ++++
 src/kudu/client/client.h           |  7 ++++++
 src/kudu/master/CMakeLists.txt     |  3 ++-
 src/kudu/master/master-test.cc     | 44 +++++++++++++++++++++++++++++++++
 src/kudu/master/master.proto       |  3 +++
 src/kudu/master/master_service.cc  | 17 +++++++++++++
 src/kudu/master/ts_descriptor.cc   |  9 +------
 src/kudu/master/ts_descriptor.h    | 12 +++++++++
 12 files changed, 130 insertions(+), 13 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/4a3e7c00/src/kudu/client/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/src/kudu/client/CMakeLists.txt b/src/kudu/client/CMakeLists.txt
index b800a9d..e10a05f 100644
--- a/src/kudu/client/CMakeLists.txt
+++ b/src/kudu/client/CMakeLists.txt
@@ -261,7 +261,8 @@ SET_KUDU_TEST_LINK_LIBS(
   itest_util
   kudu_client
   mini_cluster)
-ADD_KUDU_TEST(client-test NUM_SHARDS 8 PROCESSORS 2)
+ADD_KUDU_TEST(client-test NUM_SHARDS 8 PROCESSORS 2
+                          DATA_FILES ../scripts/first_argument.sh)
 ADD_KUDU_TEST(client-unittest)
 ADD_KUDU_TEST(predicate-test)
 ADD_KUDU_TEST(scan_token-test PROCESSORS 2)

http://git-wip-us.apache.org/repos/asf/kudu/blob/4a3e7c00/src/kudu/client/client-internal.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/client-internal.cc b/src/kudu/client/client-internal.cc
index 8300ecc..ded9787 100644
--- a/src/kudu/client/client-internal.cc
+++ b/src/kudu/client/client-internal.cc
@@ -706,6 +706,8 @@ void KuduClient::Data::ConnectedToClusterCb(
       hive_metastore_sasl_enabled_ = hive_config.hms_sasl_enabled();
       hive_metastore_uuid_ = hive_config.hms_uuid();
 
+      location_ = connect_response.client_location();
+
       master_proxy_.reset(new MasterServiceProxy(messenger_, leader_addr, leader_hostname));
       master_proxy_->set_user_credentials(user_credentials_);
     }

http://git-wip-us.apache.org/repos/asf/kudu/blob/4a3e7c00/src/kudu/client/client-internal.h
----------------------------------------------------------------------
diff --git a/src/kudu/client/client-internal.h b/src/kudu/client/client-internal.h
index e359114..74b1a10 100644
--- a/src/kudu/client/client-internal.h
+++ b/src/kudu/client/client-internal.h
@@ -244,6 +244,10 @@ class KuduClient::Data {
   // The unique id of this client.
   std::string client_id_;
 
+  // The location of this client. This is an empty string if a location has not
+  // been assigned by the leader master. Protected by 'leader_master_lock_'.
+  std::string location_;
+
   // The user credentials of the client. This field is constant after the client
   // is built.
   rpc::UserCredentials user_credentials_;
@@ -292,7 +296,8 @@ class KuduClient::Data {
   std::vector<StatusCallback> leader_master_callbacks_primary_creds_;
 
   // Protects 'leader_master_rpc_{any,primary}_creds_',
-  // 'leader_master_hostport_', 'master_hostports_', and 'master_proxy_'.
+  // 'leader_master_hostport_', 'master_hostports_', 'master_proxy_', and
+  // 'location_'.
   //
   // See: KuduClient::Data::ConnectToClusterAsync for a more
   // in-depth explanation of why this is needed and how it works.

http://git-wip-us.apache.org/repos/asf/kudu/blob/4a3e7c00/src/kudu/client/client-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/client-test.cc b/src/kudu/client/client-test.cc
index 10a4c6c..dee5d45 100644
--- a/src/kudu/client/client-test.cc
+++ b/src/kudu/client/client-test.cc
@@ -130,6 +130,7 @@ DECLARE_int32(scanner_inject_latency_on_each_batch_ms);
 DECLARE_int32(scanner_max_batch_size_bytes);
 DECLARE_int32(scanner_ttl_ms);
 DECLARE_int32(table_locations_ttl_ms);
+DECLARE_string(location_mapping_cmd);
 DECLARE_string(superuser_acl);
 DECLARE_string(user_acl);
 DEFINE_int32(test_scan_num_rows, 1000, "Number of rows to insert and scan");
@@ -188,14 +189,16 @@ class ClientTest : public KuduTest {
     FLAGS_heartbeat_interval_ms = 10;
     FLAGS_scanner_gc_check_interval_us = 50 * 1000; // 50 milliseconds.
 
+    SetLocationMappingCmd();
+
     // Start minicluster and wait for tablet servers to connect to master.
     cluster_.reset(new InternalMiniCluster(env_, InternalMiniClusterOptions()));
     ASSERT_OK(cluster_->Start());
 
     // Connect to the cluster.
     ASSERT_OK(KuduClientBuilder()
-                     .add_master_server_addr(cluster_->mini_master()->bound_rpc_addr().ToString())
-                     .Build(&client_));
+        .add_master_server_addr(cluster_->mini_master()->bound_rpc_addr().ToString())
+        .Build(&client_));
 
     ASSERT_NO_FATAL_FAILURE(CreateTable(kTableName, 1, GenerateSplitRows(), {}, &client_table_));
   }
@@ -277,6 +280,10 @@ class ClientTest : public KuduTest {
   static const char *kTableName;
   static const int32_t kNoBound;
 
+  // Set the location mapping command for the test's masters. Overridden by
+  // derived classes to test client location assignment.
+  virtual void SetLocationMappingCmd() {}
+
   string GetFirstTabletId(KuduTable* table) {
     GetTableLocationsRequestPB req;
     GetTableLocationsResponsePB resp;
@@ -5779,5 +5786,25 @@ TEST_F(ClientTest, TestBlockScannerHijackingAttempts) {
   }
 }
 
+// Client test that assigns locations to clients and tablet servers.
+// For now, assigns a uniform location to all clients and tablet servers.
+class ClientWithLocationTest : public ClientTest {
+ protected:
+  void SetLocationMappingCmd() override {
+    const string location_cmd_path = JoinPathSegments(GetTestExecutableDirectory(),
+                                                      "testdata/first_argument.sh");
+    const string location = "/foo";
+    FLAGS_location_mapping_cmd = strings::Substitute("$0 $1",
+                                                     location_cmd_path, location);
+  }
+};
+
+TEST_F(ClientTest, TestClientLocationNoLocationMappingCmd) {
+  ASSERT_TRUE(client_->location().empty());
+}
+
+TEST_F(ClientWithLocationTest, TestClientLocation) {
+  ASSERT_EQ("/foo", client_->location());
+}
 } // namespace client
 } // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/4a3e7c00/src/kudu/client/client.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/client.cc b/src/kudu/client/client.cc
index c475f49..720a81d 100644
--- a/src/kudu/client/client.cc
+++ b/src/kudu/client/client.cc
@@ -663,6 +663,11 @@ string KuduClient::GetHiveMetastoreUuid() const {
   return data_->hive_metastore_uuid_;
 }
 
+string KuduClient::location() const {
+  std::lock_guard<simple_spinlock> l(data_->leader_master_lock_);
+  return data_->location_;
+}
+
 ////////////////////////////////////////////////////////////
 // KuduTableCreator
 ////////////////////////////////////////////////////////////

http://git-wip-us.apache.org/repos/asf/kudu/blob/4a3e7c00/src/kudu/client/client.h
----------------------------------------------------------------------
diff --git a/src/kudu/client/client.h b/src/kudu/client/client.h
index f8a9fc1..f3486df 100644
--- a/src/kudu/client/client.h
+++ b/src/kudu/client/client.h
@@ -585,6 +585,13 @@ class KUDU_EXPORT KuduClient : public sp::enable_shared_from_this<KuduClient>
{
   ///   arbitrary value if the Hive Metastore integration is not enabled.
   std::string GetHiveMetastoreUuid() const KUDU_NO_EXPORT;
 
+  /// Private API.
+  ///
+  /// @return The location of the client, assigned when it first connects to
+  ///   a cluster. An empty string will be returned if no location has been
+  ///   assigned yet, or if the leader master did not assign a location to
+  ///   the client.
+  std::string location() const KUDU_NO_EXPORT;
   /// @endcond
 
  private:

http://git-wip-us.apache.org/repos/asf/kudu/blob/4a3e7c00/src/kudu/master/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/src/kudu/master/CMakeLists.txt b/src/kudu/master/CMakeLists.txt
index 9b536b7..89fd9c6 100644
--- a/src/kudu/master/CMakeLists.txt
+++ b/src/kudu/master/CMakeLists.txt
@@ -79,7 +79,8 @@ SET_KUDU_TEST_LINK_LIBS(
 
 ADD_KUDU_TEST(catalog_manager-test)
 ADD_KUDU_TEST(hms_notification_log_listener-test)
-ADD_KUDU_TEST(master-test RESOURCE_LOCK "master-web-port")
+ADD_KUDU_TEST(master-test RESOURCE_LOCK "master-web-port"
+                          DATA_FILES ../scripts/first_argument.sh)
 ADD_KUDU_TEST(mini_master-test RESOURCE_LOCK "master-web-port")
 ADD_KUDU_TEST(placement_policy-test)
 ADD_KUDU_TEST(sentry_authz_provider-test)

http://git-wip-us.apache.org/repos/asf/kudu/blob/4a3e7c00/src/kudu/master/master-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/master-test.cc b/src/kudu/master/master-test.cc
index 5916448..54dac9d 100644
--- a/src/kudu/master/master-test.cc
+++ b/src/kudu/master/master-test.cc
@@ -1619,6 +1619,50 @@ TEST_F(MasterTest, TestConnectToMaster) {
   ASSERT_EQ(1, resp.master_addrs_size());
   ASSERT_EQ("127.0.0.1", resp.master_addrs(0).host());
   ASSERT_NE(0, resp.master_addrs(0).port());
+
+  // The returned location should be empty because no location mapping command
+  // is defined.
+  ASSERT_TRUE(resp.client_location().empty());
+}
+
+TEST_F(MasterTest, TestConnectToMasterAndAssignLocation) {
+  // Test first with a valid location mapping command.
+  const string kLocationCmdPath = JoinPathSegments(GetTestExecutableDirectory(),
+                                                   "testdata/first_argument.sh");
+  const string location = "/foo";
+  FLAGS_location_mapping_cmd = Substitute("$0 $1", kLocationCmdPath, location);
+  {
+    ConnectToMasterRequestPB req;
+    ConnectToMasterResponsePB resp;
+    RpcController rpc;
+    ASSERT_OK(proxy_->ConnectToMaster(req, &resp, &rpc));
+    ASSERT_FALSE(resp.has_error());
+    ASSERT_EQ(location, resp.client_location());
+  }
+
+  // Now try again with an invalid command. The RPC should succeed but no
+  // location should be assigned.
+  FLAGS_location_mapping_cmd = "false";
+  {
+    ConnectToMasterRequestPB req;
+    ConnectToMasterResponsePB resp;
+    RpcController rpc;
+    ASSERT_OK(proxy_->ConnectToMaster(req, &resp, &rpc));
+    ASSERT_FALSE(resp.has_error());
+    ASSERT_TRUE(resp.client_location().empty());
+  }
+
+  // Finally, use a command returning a different location.
+  const string new_location = "/bar";
+  FLAGS_location_mapping_cmd = Substitute("$0 $1", kLocationCmdPath, new_location);
+  {
+    ConnectToMasterRequestPB req;
+    ConnectToMasterResponsePB resp;
+    RpcController rpc;
+    ASSERT_OK(proxy_->ConnectToMaster(req, &resp, &rpc));
+    ASSERT_FALSE(resp.has_error());
+    ASSERT_EQ(new_location, resp.client_location());
+  }
 }
 
 // Test that the master signs its on server certificate when it becomes the leader,

http://git-wip-us.apache.org/repos/asf/kudu/blob/4a3e7c00/src/kudu/master/master.proto
----------------------------------------------------------------------
diff --git a/src/kudu/master/master.proto b/src/kudu/master/master.proto
index 393394f..583644c 100644
--- a/src/kudu/master/master.proto
+++ b/src/kudu/master/master.proto
@@ -673,6 +673,9 @@ message ConnectToMasterResponsePB {
   // If the master is configured with the Hive Metastore integration enabled,
   // this field will include the configuration options.
   optional HiveMetastoreConfig hms_config = 6;
+
+  // The location of the client assigned by the master.
+  optional string client_location = 7;
 }
 
 // Hive Metastore integration options and configuration.

http://git-wip-us.apache.org/repos/asf/kudu/blob/4a3e7c00/src/kudu/master/master_service.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/master_service.cc b/src/kudu/master/master_service.cc
index ef1edb5..b87e681 100644
--- a/src/kudu/master/master_service.cc
+++ b/src/kudu/master/master_service.cc
@@ -57,6 +57,7 @@
 DECLARE_bool(hive_metastore_sasl_enabled);
 DECLARE_bool(raft_prepare_replacement_before_eviction);
 DECLARE_string(hive_metastore_uris);
+DECLARE_string(location_mapping_cmd);
 
 DEFINE_int32(master_inject_latency_on_tablet_lookups_ms, 0,
              "Number of milliseconds that the master will sleep before responding to "
@@ -522,6 +523,22 @@ void MasterServiceImpl::ConnectToMaster(const ConnectToMasterRequestPB*
/*req*/,
     // TODO(dan): set the hms_uuid field.
   }
 
+  // Assign a location to the client if needed.
+  if (!FLAGS_location_mapping_cmd.empty()) {
+    string location;
+    Status s = GetLocationFromLocationMappingCmd(FLAGS_location_mapping_cmd,
+                                                 rpc->remote_address().host(),
+                                                 &location);
+    if (s.ok()) {
+      resp->set_client_location(location);
+    } else {
+      LOG(WARNING) << Substitute("unable to assign location to client $0@$1: $2",
+                                 rpc->remote_user().ToString(),
+                                 rpc->remote_address().ToString(),
+                                 s.ToString());
+    }
+  }
+
   // Rather than consulting the current consensus role, instead base it
   // on the catalog manager's view. This prevents us from advertising LEADER
   // until we have taken over all the associated responsibilities.

http://git-wip-us.apache.org/repos/asf/kudu/blob/4a3e7c00/src/kudu/master/ts_descriptor.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/ts_descriptor.cc b/src/kudu/master/ts_descriptor.cc
index 3a400b2..321b2fd 100644
--- a/src/kudu/master/ts_descriptor.cc
+++ b/src/kudu/master/ts_descriptor.cc
@@ -95,13 +95,8 @@ bool IsValidLocation(const string& location) {
   }
   return true;
 }
+} // anonymous namespace
 
-// Resolves 'host', which is the IP address or hostname of a tablet server or
-// client, into a location using the command 'cmd'. The result will be stored
-// in 'location', which must not be null. If there is an error running the
-// command or the output is invalid, an error Status will be returned.
-// TODO(wdberkeley): Eventually we may want to get multiple locations at once
-// by giving the script multiple arguments (like Hadoop).
 Status GetLocationFromLocationMappingCmd(const string& cmd,
                                          const string& host,
                                          string* location) {
@@ -132,7 +127,6 @@ Status GetLocationFromLocationMappingCmd(const string& cmd,
   *location = std::move(location_temp);
   return Status::OK();
 }
-} // anonymous namespace
 
 Status TSDescriptor::RegisterNew(const NodeInstancePB& instance,
                                  const ServerRegistrationPB& registration,
@@ -404,6 +398,5 @@ string TSDescriptor::ToString() const {
   const auto& addr = registration_->rpc_addresses(0);
   return Substitute("$0 ($1:$2)", permanent_uuid_, addr.host(), addr.port());
 }
-
 } // namespace master
 } // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/4a3e7c00/src/kudu/master/ts_descriptor.h
----------------------------------------------------------------------
diff --git a/src/kudu/master/ts_descriptor.h b/src/kudu/master/ts_descriptor.h
index c7fdd60..65724fe 100644
--- a/src/kudu/master/ts_descriptor.h
+++ b/src/kudu/master/ts_descriptor.h
@@ -53,6 +53,18 @@ class TabletServerAdminServiceProxy;
 
 namespace master {
 
+// Resolves 'host', which is the IP address or hostname of a tablet server or
+// client, into a location using the command 'cmd'. The result will be stored
+// in 'location', which must not be null. If there is an error running the
+// command or the output is invalid, an error Status will be returned.
+// TODO(wdberkeley): Refactor into a separate class and implement a caching
+// policy.
+// TODO(wdberkeley): Eventually we may want to get multiple locations at once
+// by giving the script multiple arguments (like Hadoop).
+Status GetLocationFromLocationMappingCmd(const std::string& cmd,
+                                         const std::string& host,
+                                         std::string* location);
+
 // Master-side view of a single tablet server.
 //
 // Tracks the last heartbeat, status, instance identifier, location, etc.


Mime
View raw message