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 <[email protected]> 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 <[email protected]> Tested-by: Yingchun Lai <[email protected]> --- 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(); }
