[kudu] branch master updated: [master] update tags for --master_client_location_assignment_enabled

[alexey]

This is an automated email from the ASF dual-hosted git repository.

alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/master by this push:
     new 9f6e957  [master] update tags for 
--master_client_location_assignment_enabled
9f6e957 is described below

commit 9f6e9576bcca8bf6d6ac1803d3d69c2de135e911
Author: Alexey Serbin <ale...@apache.org>
AuthorDate: Fri Sep 6 10:39:37 2019 -0700

    [master] update tags for --master_client_location_assignment_enabled
    
    The --master_client_location_assignment_enabled flag was originally
    purposed for test scenarios.  However, there is nothing unsafe
    in assigning locations only to tablet servers, but not clients.
    
    This patch removes 'unsafe' tag from the flag and adds 'advanced' tag
    instead.  In addition, this patch cleans up the related code a bit,
    adding TODO for easier tracking of further improvements
    in the scope of KUDU-2771.
    
    Change-Id: Iaeb7391174a37f1a774f83b964bee854922cadcd
    Reviewed-on: http://gerrit.cloudera.org:8080/14190
    Reviewed-by: Adar Dembo <a...@cloudera.com>
    Tested-by: Kudu Jenkins
---
 src/kudu/master/location_cache.cc | 1 +
 src/kudu/master/master_service.cc | 9 ++++-----
 src/kudu/master/ts_manager.cc     | 4 ++--
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/kudu/master/location_cache.cc 
b/src/kudu/master/location_cache.cc
index f4f5378..77b2f91 100644
--- a/src/kudu/master/location_cache.cc
+++ b/src/kudu/master/location_cache.cc
@@ -112,6 +112,7 @@ Status LocationCache::GetLocation(const string& key, 
string* location) {
     std::lock_guard<rw_spinlock> l(location_map_lock_);
     // This simple implementation doesn't protect against multiple runs of the
     // location-mapping command for the same key.
+    // TODO(KUDU-2771): queue concurrent requests for the same key
     InsertIfNotPresent(&location_map_, key, value);
     *location = value;
   }
diff --git a/src/kudu/master/master_service.cc 
b/src/kudu/master/master_service.cc
index 7c5d916..9b9af6d 100644
--- a/src/kudu/master/master_service.cc
+++ b/src/kudu/master/master_service.cc
@@ -83,11 +83,10 @@ TAG_FLAG(master_non_leader_masters_propagate_tsk, hidden);
 DEFINE_bool(master_client_location_assignment_enabled, true,
             "Whether masters assign locations to connecting clients. "
             "By default they do if the location assignment command is set, "
-            "but in some test scenarios it's useful to make masters assign "
-            "locations only to tablet servers, but not clients.");
-TAG_FLAG(master_client_location_assignment_enabled, hidden);
+            "but setting this flag to 'false' makes masters assign "
+            "locations only to tablet servers, not clients.");
+TAG_FLAG(master_client_location_assignment_enabled, advanced);
 TAG_FLAG(master_client_location_assignment_enabled, runtime);
-TAG_FLAG(master_client_location_assignment_enabled, unsafe);
 
 DEFINE_bool(master_support_authz_tokens, true,
             "Whether the master supports generating authz tokens. Used for "
@@ -571,7 +570,7 @@ void MasterServiceImpl::ConnectToMaster(const 
ConnectToMasterRequestPB* /*req*/,
   // Assign a location to the client if needed.
   auto* location_cache = server_->location_cache();
   if (location_cache != nullptr &&
-      PREDICT_TRUE(FLAGS_master_client_location_assignment_enabled)) {
+      FLAGS_master_client_location_assignment_enabled) {
     string location;
     const auto s = location_cache->GetLocation(
         rpc->remote_address().host(), &location);
diff --git a/src/kudu/master/ts_manager.cc b/src/kudu/master/ts_manager.cc
index de4a1ac..f7f8bbf 100644
--- a/src/kudu/master/ts_manager.cc
+++ b/src/kudu/master/ts_manager.cc
@@ -117,11 +117,11 @@ Status TSManager::RegisterTS(const NodeInstancePB& 
instance,
   // a location involves calling the location mapping script which is 
relatively
   // long and expensive operation.
   boost::optional<string> location;
-  if (PREDICT_TRUE(location_cache_ != nullptr)) {
+  if (location_cache_) {
     // In some test scenarios the location is assigned per tablet server UUID.
     // That's the case when multiple (or even all) tablet servers have the same
     // IP address for their RPC endpoint.
-    const auto& cmd_arg = FLAGS_location_mapping_by_uuid
+    const auto& cmd_arg = PREDICT_FALSE(FLAGS_location_mapping_by_uuid)
         ? uuid : registration.rpc_addresses(0).host();
     TRACE(Substitute("tablet server $0: assigning location", uuid));
     string location_str;