This is an automated email from the ASF dual-hosted git repository.
bankim 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 23fd58c [util] Add a function to fetch non-default flags as a map
23fd58c is described below
commit 23fd58c39177110fa64047d566a4a1c375e061f5
Author: Bankim Bhavsar <[email protected]>
AuthorDate: Tue May 4 14:06:00 2021 -0700
[util] Add a function to fetch non-default flags as a map
We already have a function that returns non-default flags
as a vector of strings. This change adds a function
that returns non-default flags as a map refactoring
and reusing the existing logic.
This function is used in add master tool to bring
up the new master as it's unnecessary to pass all
the flags to the kudu master. This is similar to
master run implementation in master_runner.cc.
Change-Id: I7e7c9fc4de9be05110dba5ca02f48d46b6b474b7
Reviewed-on: http://gerrit.cloudera.org:8080/17400
Tested-by: Kudu Jenkins
Reviewed-by: Andrew Wong <[email protected]>
Reviewed-by: Mahesh Reddy <[email protected]>
Reviewed-by: Grant Henke <[email protected]>
Reviewed-by: Alexey Serbin <[email protected]>
---
src/kudu/tools/tool_action_master.cc | 2 +-
src/kudu/util/flags.cc | 50 ++++++++++++++++++++++++------------
src/kudu/util/flags.h | 9 ++++---
3 files changed, 40 insertions(+), 21 deletions(-)
diff --git a/src/kudu/tools/tool_action_master.cc
b/src/kudu/tools/tool_action_master.cc
index 4b61e4f..3a49e25 100644
--- a/src/kudu/tools/tool_action_master.cc
+++ b/src/kudu/tools/tool_action_master.cc
@@ -369,7 +369,7 @@ Status AddMaster(const RunnerContext& context) {
if (FLAGS_fs_data_dirs.empty()) {
return Status::InvalidArgument("Flag -fs_data_dirs not supplied");
}
- GFlagsMap flags_map = GetFlagsMap();
+ GFlagsMap flags_map = GetNonDefaultFlagsMap();
// Remove the optional parameters for this command.
// Remaining optional flags need to be passed to the new master.
flags_map.erase("wait_secs");
diff --git a/src/kudu/util/flags.cc b/src/kudu/util/flags.cc
index 01a0100..949018c 100644
--- a/src/kudu/util/flags.cc
+++ b/src/kudu/util/flags.cc
@@ -21,10 +21,12 @@
#include <sys/stat.h>
#include <unistd.h> // IWYU pragma: keep
+#include <algorithm>
#include <cstdlib>
#include <cstring>
#include <functional>
#include <iostream>
+#include <iterator>
#include <map>
#include <string>
#include <unordered_set>
@@ -560,34 +562,50 @@ string CommandlineFlagsIntoString(EscapeMode mode) {
return ret_value;
}
+vector<CommandLineFlagInfo> GetNonDefaultFlagsHelper() {
+ vector<CommandLineFlagInfo> all_flags;
+ GetAllFlags(&all_flags);
+ vector<CommandLineFlagInfo> non_default_flags;
+ std::copy_if(all_flags.begin(), all_flags.end(),
std::back_inserter(non_default_flags),
+ [] (const auto& flag) {
+ // In addition to checking that the flag has been rewritten
with 'is_default',
+ // we also check that the value is different from the default
value.
+ return !flag.is_default && flag.current_value !=
flag.default_value;
+ });
+ return non_default_flags;
+}
+
string GetNonDefaultFlags() {
ostringstream args;
- vector<CommandLineFlagInfo> flags;
- GetAllFlags(&flags);
- for (const auto& flag : flags) {
- if (!flag.is_default) {
- // This only means that the flag has been rewritten.
- // We need to check that the value is different from the default value.
- if (flag.current_value != flag.default_value) {
- if (!args.str().empty()) {
- args << '\n';
- }
-
- // Redact the flags tagged as sensitive, if redaction is enabled.
- string flag_value = CheckFlagAndRedact(flag, EscapeMode::NONE);
- args << "--" << flag.name << '=' << flag_value;
- }
+ for (const auto& flag : GetNonDefaultFlagsHelper()) {
+ if (!args.str().empty()) {
+ args << '\n';
}
+ // Redact the flags tagged as sensitive, if redaction is enabled.
+ string flag_value = CheckFlagAndRedact(flag, EscapeMode::NONE);
+ args << "--" << flag.name << '=' << flag_value;
}
return args.str();
}
+GFlagsMap GetNonDefaultFlagsMap() {
+ GFlagsMap flags_by_name;
+ for (auto& flag : GetNonDefaultFlagsHelper()) {
+ // Instead of "flags_by_name.emplace(flag.name, std::move(flag))" using
+ // following approach as order of evaluation of parameters could be
different
+ // leading to unexpected value in "flag.name".
+ flags_by_name[flag.name] = std::move(flag);
+ }
+ return flags_by_name;
+}
+
GFlagsMap GetFlagsMap() {
vector<CommandLineFlagInfo> default_flags;
GetAllFlags(&default_flags);
GFlagsMap flags_by_name;
for (auto& flag : default_flags) {
- flags_by_name.emplace(flag.name, std::move(flag));
+ // See the note in GetNonDefaultFlagsMap() above.
+ flags_by_name[flag.name] = std::move(flag);
}
return flags_by_name;
}
diff --git a/src/kudu/util/flags.h b/src/kudu/util/flags.h
index 98f8cb8..b88c25d 100644
--- a/src/kudu/util/flags.h
+++ b/src/kudu/util/flags.h
@@ -21,11 +21,9 @@
#include <string>
#include <unordered_map>
-#include "kudu/util/status.h"
+#include <gflags/gflags.h>
-namespace google {
- struct CommandLineFlagInfo;
-}
+#include "kudu/util/status.h"
namespace kudu {
@@ -77,6 +75,9 @@ typedef std::unordered_map<std::string,
google::CommandLineFlagInfo> GFlagsMap;
// are tagged as sensitive, if redaction is enabled.
std::string GetNonDefaultFlags();
+// Same as 'GetNonDefaultFlags' but returns the output as a map.
+GFlagsMap GetNonDefaultFlagsMap();
+
GFlagsMap GetFlagsMap();
enum class TriStateFlag {
