You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by la...@apache.org on 2023/01/16 04:27:12 UTC
[kudu] 02/02: [KUDU-3430] Implement getting flags filter logic in tool side
This is an automated email from the ASF dual-hosted git repository.
laiyingchun pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 76ac5b89af3f0fe6e79541fb24ca734d2a8a30de
Author: xinghuayu007 <14...@qq.com>
AuthorDate: Thu Dec 29 11:49:33 2022 +0800
[KUDU-3430] Implement getting flags filter logic in tool side
When using new version of Kudu tool (version: 1.16.0) to
execute command: "kudu tserver get_flags localhost:7050
-flags=block_cache_capacity_mb" on the old version of Kudu
TServer (version: 1.10.1)，it returned all flags not the
specified flags.
That because the filter logic was not implemented on the
server side (Kudu version 1.10.1).
So it is better to implement the filter logic on the Kudu
tool side also.
Change-Id: I0a2ddc2540d7aa70d3dd2e6c4101bb227e94c858
Reviewed-on: http://gerrit.cloudera.org:8080/19393
Reviewed-by: Yingchun Lai <ac...@gmail.com>
Tested-by: Yingchun Lai <ac...@gmail.com>
---
src/kudu/server/generic_service.cc | 26 +++++++++++++++-----------
src/kudu/tools/kudu-tool-test.cc | 17 ++++++++++++++++-
src/kudu/tools/tool_action_common.cc | 29 ++++++++++++++++++++++++++---
3 files changed, 57 insertions(+), 15 deletions(-)
diff --git a/src/kudu/server/generic_service.cc b/src/kudu/server/generic_service.cc
index eddc1d495..b4d38c9cd 100644
--- a/src/kudu/server/generic_service.cc
+++ b/src/kudu/server/generic_service.cc
@@ -46,6 +46,8 @@
DECLARE_string(time_source);
DECLARE_bool(use_hybrid_clock);
+DEFINE_bool(disable_gflag_filter_logic_for_testing, false,
+ "Whether to disable filter logic on the server side for test purpose.");
using std::string;
using std::unordered_set;
@@ -87,21 +89,23 @@ void GenericServiceImpl::GetFlags(const GetFlagsRequestPB* req,
if (entry.second.is_default && !all_flags && flags.empty()) {
continue;
}
-
- if (!flags.empty() && !ContainsKey(flags, entry.first)) {
- continue;
- }
-
unordered_set<string> tags;
GetFlagTags(entry.first, &tags);
- bool matches = req->tags().empty();
- for (const auto& tag : req->tags()) {
- if (ContainsKey(tags, tag)) {
- matches = true;
- break;
+
+ if (!FLAGS_disable_gflag_filter_logic_for_testing) {
+ if (!flags.empty() && !ContainsKey(flags, entry.first)) {
+ continue;
+ }
+
+ bool matches = req->tags().empty();
+ for (const auto& tag : req->tags()) {
+ if (ContainsKey(tags, tag)) {
+ matches = true;
+ break;
+ }
}
+ if (!matches) continue;
}
- if (!matches) continue;
auto* flag = resp->add_flags();
flag->set_name(entry.first);
diff --git a/src/kudu/tools/kudu-tool-test.cc b/src/kudu/tools/kudu-tool-test.cc
index dce3f7118..21eb85101 100644
--- a/src/kudu/tools/kudu-tool-test.cc
+++ b/src/kudu/tools/kudu-tool-test.cc
@@ -143,6 +143,7 @@
DECLARE_bool(enable_tablet_orphaned_block_deletion);
DECLARE_bool(encrypt_data_at_rest);
+DECLARE_bool(disable_gflag_filter_logic_for_testing);
DECLARE_bool(fs_data_dirs_consider_available_space);
DECLARE_bool(hive_metastore_sasl_enabled);
DECLARE_bool(show_values);
@@ -7913,8 +7914,22 @@ TEST_F(ToolTest, TestReplaceTablet) {
ASSERT_GE(workload.rows_inserted(), CountTableRows(workload_table.get()));
}
-TEST_F(ToolTest, TestGetFlags) {
+class GetFlagsTest :
+ public ToolTest,
+ public ::testing::WithParamInterface<bool> {
+};
+
+INSTANTIATE_TEST_SUITE_P(DisableFlagFilterLogic, GetFlagsTest, ::testing::Bool());
+TEST_P(GetFlagsTest, TestGetFlags) {
ExternalMiniClusterOptions opts;
+ const string disable_flag_filter_logic_on_server =
+ Substitute("--disable_gflag_filter_logic_for_testing=$0", GetParam());
+ opts.extra_master_flags = {
+ disable_flag_filter_logic_on_server,
+ };
+ opts.extra_tserver_flags = {
+ disable_flag_filter_logic_on_server,
+ };
opts.num_tablet_servers = 1;
NO_FATALS(StartExternalMiniCluster(std::move(opts)));
diff --git a/src/kudu/tools/tool_action_common.cc b/src/kudu/tools/tool_action_common.cc
index 02828345a..f54ac1aee 100644
--- a/src/kudu/tools/tool_action_common.cc
+++ b/src/kudu/tools/tool_action_common.cc
@@ -253,6 +253,7 @@ using std::setw;
using std::shared_ptr;
using std::string;
using std::unique_ptr;
+using std::unordered_set;
using std::vector;
using strings::a2b_hex;
using strings::Split;
@@ -437,17 +438,39 @@ Status GetServerFlags(const string& address,
rpc.set_timeout(MonoDelta::FromMilliseconds(FLAGS_timeout_ms));
req.set_all_flags(all_flags);
- for (StringPiece tag : strings::Split(flag_tags, ",", strings::SkipEmpty())) {
+ const vector<string> filter_tags = strings::Split(flag_tags, ",", strings::SkipEmpty());
+ for (StringPiece tag : filter_tags) {
req.add_tags(tag.as_string());
}
- for (StringPiece flag: strings::Split(flags_to_get, ",", strings::SkipEmpty())) {
+ const unordered_set<string> filter_flags =
+ strings::Split(flags_to_get, ",", strings::SkipEmpty());
+ for (StringPiece flag: filter_flags) {
req.add_flags(flag.as_string());
}
RETURN_NOT_OK(proxy->GetFlags(req, &resp, &rpc));
flags->clear();
- std::move(resp.flags().begin(), resp.flags().end(), std::back_inserter(*flags));
+ // Filter flags according to -flags_to_get and -flags_tags. Currently, the filter
+ // logic has been implemented on the server side, but some old versions do not support
+ // the filter logic on the server side, which makes new version kudu tool can not be
+ // used in old version of Kudu server. Therefore, it is better to implement filter
+ // logic on the kudu tool side again.
+ for (const auto& flag : resp.flags()) {
+ if (!filter_flags.empty() && !ContainsKey(filter_flags, flag.name())) {
+ continue;
+ }
+ bool matches = filter_tags.empty();
+ unordered_set<string> contained_tags(flag.tags().begin(), flag.tags().end());
+ for (const auto& tag : filter_tags) {
+ if (ContainsKey(contained_tags, tag)) {
+ matches = true;
+ break;
+ }
+ }
+ if (!matches) continue;
+ flags->push_back(flag);
+ }
return Status::OK();
}