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 32c5b9c [master] turn off client location assignment by default
32c5b9c is described below
commit 32c5b9c60bc923e27f0a4c78508fc3e2d1276e28
Author: Alexey Serbin <[email protected]>
AuthorDate: Thu Feb 4 12:18:16 2021 -0800
[master] turn off client location assignment by default
This patch turns off location assignment by default to clients
connecting to a Kudu cluster. The assigned locations are cached,
but the way how locations assignment performed is resource consuming,
see [1] for details.
There aren't many benefits in assigning locations to clients so far:
the only nice property of a client with assigned location vs a client
with no location assigned is that former case the client run from a
particular location would choose tablet servers in the same location
if performing scan in ReplicaSelection::CLOSEST_REPLICA mode.
This is a first step towards addressing KUDU-3212.
[1] https://issues.apache.org/jira/browse/KUDU-3212
Change-Id: I78474ced0a0129b3f2b1add55f6f908a136106d0
Reviewed-on: http://gerrit.cloudera.org:8080/17024
Tested-by: Kudu Jenkins
Reviewed-by: Hao Hao <[email protected]>
Reviewed-by: Grant Henke <[email protected]>
---
.../src/test/java/org/apache/kudu/client/TestKuduClient.java | 4 ++++
src/kudu/client/client-test.cc | 4 ++++
src/kudu/integration-tests/location_assignment-itest.cc | 8 +++++++-
src/kudu/master/master-test.cc | 2 ++
src/kudu/master/master_service.cc | 5 ++---
5 files changed, 19 insertions(+), 4 deletions(-)
diff --git
a/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
b/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
index 32e75e3..1d473bb 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
@@ -75,6 +75,7 @@ import org.apache.kudu.test.CapturingLogAppender;
import org.apache.kudu.test.ClientTestUtil;
import org.apache.kudu.test.KuduTestHarness;
import org.apache.kudu.test.KuduTestHarness.LocationConfig;
+import org.apache.kudu.test.KuduTestHarness.MasterServerConfig;
import org.apache.kudu.test.KuduTestHarness.TabletServerConfig;
import org.apache.kudu.test.RandomUtils;
import org.apache.kudu.util.DateUtil;
@@ -1317,6 +1318,9 @@ public class TestKuduClient {
@LocationConfig(locations = {
"/L0:6", // 3 masters, 1 client, 3 tablet servers: 3 * 1 + 3 = 6.
})
+ @MasterServerConfig(flags = {
+ "--master_client_location_assignment_enabled=true",
+ })
public void testClientLocation() throws Exception {
// Do something that will cause the client to connect to the cluster.
client.listTabletServers();
diff --git a/src/kudu/client/client-test.cc b/src/kudu/client/client-test.cc
index 91ae430..eb8a150 100644
--- a/src/kudu/client/client-test.cc
+++ b/src/kudu/client/client-test.cc
@@ -142,6 +142,7 @@ DECLARE_bool(disable_txn_system_client_init);
DECLARE_bool(fail_dns_resolution);
DECLARE_bool(location_mapping_by_uuid);
DECLARE_bool(log_inject_latency);
+DECLARE_bool(master_client_location_assignment_enabled);
DECLARE_bool(master_support_connect_to_master_rpc);
DECLARE_bool(mock_table_metrics_for_testing);
DECLARE_bool(rpc_listen_on_unix_domain_socket);
@@ -7668,6 +7669,9 @@ class ClientWithLocationTest : public ClientTest {
// Some of these tests assume no client activity, so disable the
// transaction system client.
FLAGS_disable_txn_system_client_init = true;
+
+ // By default, master doesn't assing locations to connecting clients.
+ FLAGS_master_client_location_assignment_enabled = true;
}
};
diff --git a/src/kudu/integration-tests/location_assignment-itest.cc
b/src/kudu/integration-tests/location_assignment-itest.cc
index 673b131..502ae4b 100644
--- a/src/kudu/integration-tests/location_assignment-itest.cc
+++ b/src/kudu/integration-tests/location_assignment-itest.cc
@@ -83,7 +83,13 @@ TEST_F(ClientLocationAssignmentITest, Basic) {
EmplaceOrDie(&info, Substitute("/L$0", i), i == client_loc_idx ? 2 : 1);
}
FLAGS_num_replicas = FLAGS_num_tablet_servers;
- NO_FATALS(BuildAndStart({}, {}, std::move(info)));
+ const vector<string> master_flags = {
+ // Assigning locations to clients is turned off by default, but this
+ // scenario exercises functionality related to that, so it's necessary
+ // to turn it on.
+ "--master_client_location_assignment_enabled=true",
+ };
+ NO_FATALS(BuildAndStart({}, master_flags, std::move(info)));
// Find the tablet server that is colocated with the client, if there is one.
const auto timeout = MonoDelta::FromSeconds(30);
diff --git a/src/kudu/master/master-test.cc b/src/kudu/master/master-test.cc
index a0116ec..0450585 100644
--- a/src/kudu/master/master-test.cc
+++ b/src/kudu/master/master-test.cc
@@ -118,6 +118,7 @@ using strings::Substitute;
DECLARE_bool(catalog_manager_check_ts_count_for_create_table);
DECLARE_bool(enable_per_range_hash_schemas);
+DECLARE_bool(master_client_location_assignment_enabled);
DECLARE_bool(master_support_authz_tokens);
DECLARE_bool(mock_table_metrics_for_testing);
DECLARE_bool(raft_prepare_replacement_before_eviction);
@@ -2054,6 +2055,7 @@ TEST_F(MasterTest, TestConnectToMasterAndAssignLocation) {
"testdata/first_argument.sh");
const string location = "/foo";
FLAGS_location_mapping_cmd = Substitute("$0 $1", kLocationCmdPath, location);
+ FLAGS_master_client_location_assignment_enabled = true;
{
// Restarting the master to take into account the new setting for the
// --location_mapping_cmd flag.
diff --git a/src/kudu/master/master_service.cc
b/src/kudu/master/master_service.cc
index 3a1702f..f40faa0 100644
--- a/src/kudu/master/master_service.cc
+++ b/src/kudu/master/master_service.cc
@@ -83,10 +83,9 @@ DEFINE_bool(master_non_leader_masters_propagate_tsk, false,
"tests scenarios only and should not be used elsewhere.");
TAG_FLAG(master_non_leader_masters_propagate_tsk, hidden);
-DEFINE_bool(master_client_location_assignment_enabled, true,
+DEFINE_bool(master_client_location_assignment_enabled, false,
"Whether masters assign locations to connecting clients. "
- "By default they do if the location assignment command is set, "
- "but setting this flag to 'false' makes masters assign "
+ "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);
