Repository: kudu Updated Branches: refs/heads/master abd87164f -> ba36a0b7d
KUDU-1696. Daemons should dump their version info to the INFO log at startup In addition to adding full version info on tserver and master startup, I've added logging of command line arguments as suggested in the ticket. Using flags before ParseCommandLineFlags() as defaults, I've been able to filter out from output any flags set explicitly in source code or values set to their defaults in command line. Here is what it looks like in ts log now, for example: I1016 23:17:21.764128 10547 tablet_server_main.cc:55] Tablet server executed with: --fs_data_dirs=/tmp/kudu/5VmpWrWo/ts --fs_wal_dir=/tmp/kudu/5VmpWrWo/ts --rpc_bind_addresses=127.0.0.1 --webserver_interface=localhost --webserver_port=8051 --tserver_master_addrs=127.0.0.1 --heap_profile_path=/tmp/kudu-tserver.7060 --unlock_experimental_flags=true --local_ip_for_outbound_sockets=127.0.0.1 --log_dir=/tmp/kudu/5VmpWrWo Tablet server version: kudu 1.1.0-SNAPSHOT revision de38975ba220d57f241416b61ac1ecb40f2de75a-dirty build type RELEASE built by max at 19 Oct 2016 00:56:30 MSK on destr-workspace Change-Id: I846827c53e74ca364af61c690bf85af8a8f28601 Reviewed-on: http://gerrit.cloudera.org:8080/4733 Reviewed-by: Todd Lipcon <[email protected]> Tested-by: Kudu Jenkins Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/ba36a0b7 Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/ba36a0b7 Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/ba36a0b7 Branch: refs/heads/master Commit: ba36a0b7d122545ae99fb2a2e7e9c7e0035669b1 Parents: abd8716 Author: Maxim Smyatkin <[email protected]> Authored: Sun Oct 16 23:19:09 2016 +0300 Committer: Todd Lipcon <[email protected]> Committed: Thu Oct 20 20:31:26 2016 +0000 ---------------------------------------------------------------------- src/kudu/master/master_main.cc | 9 +++ src/kudu/tserver/tablet_server_main.cc | 9 +++ src/kudu/util/CMakeLists.txt | 1 + src/kudu/util/flags-test.cc | 91 +++++++++++++++++++++++++++++ src/kudu/util/flags.cc | 36 ++++++++++++ src/kudu/util/flags.h | 12 ++++ src/kudu/util/test_macros.h | 8 +++ 7 files changed, 166 insertions(+) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/ba36a0b7/src/kudu/master/master_main.cc ---------------------------------------------------------------------- diff --git a/src/kudu/master/master_main.cc b/src/kudu/master/master_main.cc index 2539dec..6d37784 100644 --- a/src/kudu/master/master_main.cc +++ b/src/kudu/master/master_main.cc @@ -23,6 +23,7 @@ #include "kudu/util/flags.h" #include "kudu/util/init.h" #include "kudu/util/logging.h" +#include "kudu/util/version_info.h" using kudu::master::Master; @@ -46,13 +47,21 @@ static int MasterMain(int argc, char** argv) { // the desired replication factor. (It's not turtles all the way down!) FLAGS_evict_failed_followers = false; + GFlagsMap default_flags = GetFlagsMap(); + ParseCommandLineFlags(&argc, &argv, true); if (argc != 1) { std::cerr << "usage: " << argv[0] << std::endl; return 1; } + std::string nondefault_flags = GetNonDefaultFlags(default_flags); InitGoogleLoggingSafe(argv[0]); + LOG(INFO) << "Master server non-default flags:\n" + << nondefault_flags << '\n' + << "Master server version:\n" + << VersionInfo::GetAllVersionInfo(); + MasterOptions opts; Master server(opts); LOG(INFO) << "Initializing master server..."; http://git-wip-us.apache.org/repos/asf/kudu/blob/ba36a0b7/src/kudu/tserver/tablet_server_main.cc ---------------------------------------------------------------------- diff --git a/src/kudu/tserver/tablet_server_main.cc b/src/kudu/tserver/tablet_server_main.cc index 8e35ae5..759fc82 100644 --- a/src/kudu/tserver/tablet_server_main.cc +++ b/src/kudu/tserver/tablet_server_main.cc @@ -23,6 +23,7 @@ #include "kudu/util/flags.h" #include "kudu/util/init.h" #include "kudu/util/logging.h" +#include "kudu/util/version_info.h" using kudu::tserver::TabletServer; @@ -42,13 +43,21 @@ static int TabletServerMain(int argc, char** argv) { FLAGS_rpc_num_service_threads = 20; FLAGS_webserver_port = TabletServer::kDefaultWebPort; + GFlagsMap default_flags = GetFlagsMap(); + ParseCommandLineFlags(&argc, &argv, true); if (argc != 1) { std::cerr << "usage: " << argv[0] << std::endl; return 1; } + std::string nondefault_flags = GetNonDefaultFlags(default_flags); InitGoogleLoggingSafe(argv[0]); + LOG(INFO) << "Tablet server non-default flags:\n" + << nondefault_flags << '\n' + << "Tablet server version:\n" + << VersionInfo::GetAllVersionInfo(); + TabletServerOptions opts; TabletServer server(opts); LOG(INFO) << "Initializing tablet server..."; http://git-wip-us.apache.org/repos/asf/kudu/blob/ba36a0b7/src/kudu/util/CMakeLists.txt ---------------------------------------------------------------------- diff --git a/src/kudu/util/CMakeLists.txt b/src/kudu/util/CMakeLists.txt index 1a04e12..64847fe 100644 --- a/src/kudu/util/CMakeLists.txt +++ b/src/kudu/util/CMakeLists.txt @@ -290,6 +290,7 @@ ADD_KUDU_TEST(env_util-test) ADD_KUDU_TEST(errno-test) ADD_KUDU_TEST(failure_detector-test) ADD_KUDU_TEST(flag_tags-test) +ADD_KUDU_TEST(flags-test) ADD_KUDU_TEST(group_varint-test) ADD_KUDU_TEST(hash_util-test) ADD_KUDU_TEST(hdr_histogram-test) http://git-wip-us.apache.org/repos/asf/kudu/blob/ba36a0b7/src/kudu/util/flags-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/util/flags-test.cc b/src/kudu/util/flags-test.cc new file mode 100644 index 0000000..440c9f4 --- /dev/null +++ b/src/kudu/util/flags-test.cc @@ -0,0 +1,91 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include <gflags/gflags.h> +#include <gtest/gtest.h> +#include <string> + +#include "kudu/gutil/strings/substitute.h" +#include "kudu/util/env.h" +#include "kudu/util/flags.h" +#include "kudu/util/test_util.h" + +// Test gflags +DEFINE_string(test_nondefault_ff, "default", + "Check if we track non defaults from flagfile"); +DEFINE_string(test_nondefault_explicit, "default", + "Check if we track explicitly set non defaults"); +DEFINE_string(test_default_ff, "default", + "Check if we track defaults from flagfile"); +DEFINE_string(test_default_explicit, "default", + "Check if we track explicitly set defaults"); +DECLARE_string(flagfile); + + +namespace kudu { + +class FlagsTest : public KuduTest { +}; + +TEST_F(FlagsTest, TestNonDefaultFlags) { + // Memorize the default flags + GFlagsMap default_flags = GetFlagsMap(); + + std::string flagfile_path(GetTestPath("test_nondefault_flags")); + std::string flagfile_contents = "--test_nondefault_ff=nondefault\n" + "--test_default_ff=default"; + + CHECK_OK(WriteStringToFile(Env::Default(), + Slice(flagfile_contents.data(), + flagfile_contents.size()), + flagfile_path)); + + std::string flagfile_flag = strings::Substitute("--flagfile=$0", flagfile_path); + int argc = 4; + const char* argv[4] = { + "some_executable_file", + "--test_nondefault_explicit=nondefault", + "--test_default_explicit=default", + flagfile_flag.c_str() + }; + + char** casted_argv = const_cast<char**>(argv); + ParseCommandLineFlags(&argc, &casted_argv, true); + + std::vector<const char*> expected_flags = { + "--test_nondefault_explicit=nondefault", + "--test_nondefault_ff=nondefault", + flagfile_flag.c_str() + }; + + std::vector<const char*> unexpected_flags = { + "--test_default_explicit", + "--test_default_ff" + }; + + std::string result = GetNonDefaultFlags(default_flags); + + for (const auto& expected : expected_flags) { + ASSERT_STR_CONTAINS(result, expected) + } + + for (const auto& unexpected : unexpected_flags) { + ASSERT_STR_NOT_CONTAINS(result, unexpected) + } +} + +} // namespace kudu http://git-wip-us.apache.org/repos/asf/kudu/blob/ba36a0b7/src/kudu/util/flags.cc ---------------------------------------------------------------------- diff --git a/src/kudu/util/flags.cc b/src/kudu/util/flags.cc index 5677dc0..1d8580b 100644 --- a/src/kudu/util/flags.cc +++ b/src/kudu/util/flags.cc @@ -18,6 +18,7 @@ #include "kudu/util/flags.h" #include <iostream> +#include <sstream> #include <string> #include <unordered_set> #include <vector> @@ -38,6 +39,7 @@ using google::CommandLineFlagInfo; using std::cout; using std::endl; using std::string; +using std::stringstream; using std::unordered_set; // Because every binary initializes its flags here, we use it as a convenient place @@ -355,4 +357,38 @@ void HandleCommonFlags() { #endif } +string GetNonDefaultFlags(const GFlagsMap& default_flags) { + stringstream args; + vector<CommandLineFlagInfo> flags; + GetAllFlags(&flags); + for (const auto& flag : flags) { + if (!flag.is_default) { + // This only means that the flag has been rewritten. It doesn't + // mean that this has been done in the command line, or even + // that it's truly different from the default value. + // Next, we try to check both. + const auto& default_flag = default_flags.find(flag.name); + // it's very unlikely, but still possible that we don't have the flag in defaults + if (default_flag == default_flags.end() || + flag.current_value != default_flag->second.current_value) { + if (!args.str().empty()) { + args << '\n'; + } + args << "--" << flag.name << '=' << flag.current_value; + } + } + } + return args.str(); +} + +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)); + } + return flags_by_name; +} + } // namespace kudu http://git-wip-us.apache.org/repos/asf/kudu/blob/ba36a0b7/src/kudu/util/flags.h ---------------------------------------------------------------------- diff --git a/src/kudu/util/flags.h b/src/kudu/util/flags.h index a9cd93b..8c29367 100644 --- a/src/kudu/util/flags.h +++ b/src/kudu/util/flags.h @@ -17,6 +17,10 @@ #ifndef KUDU_UTIL_FLAGS_H #define KUDU_UTIL_FLAGS_H +#include <gflags/gflags.h> +#include <string> +#include <unordered_map> + #include "kudu/gutil/macros.h" namespace kudu { @@ -42,5 +46,13 @@ int ParseCommandLineFlags(int* argc, char*** argv, bool remove_flags); // google::ParseCommandLineNonHelpFlags(). void HandleCommonFlags(); +typedef std::unordered_map<std::string, google::CommandLineFlagInfo> GFlagsMap; + +// Get all the flags different from their defaults. The output is a nicely +// formatted string with --flag=value pairs per line. +std::string GetNonDefaultFlags(const GFlagsMap& default_flags); + +GFlagsMap GetFlagsMap(); + } // namespace kudu #endif /* KUDU_UTIL_FLAGS_H */ http://git-wip-us.apache.org/repos/asf/kudu/blob/ba36a0b7/src/kudu/util/test_macros.h ---------------------------------------------------------------------- diff --git a/src/kudu/util/test_macros.h b/src/kudu/util/test_macros.h index ce28aa0..7da83e5 100644 --- a/src/kudu/util/test_macros.h +++ b/src/kudu/util/test_macros.h @@ -58,6 +58,14 @@ } \ } while (0); +#define ASSERT_STR_NOT_CONTAINS(str, substr) do { \ + const std::string& _s = (str); \ + if (_s.find((substr)) != std::string::npos) { \ + FAIL() << "Expected not to find substring '" << (substr) \ + << "'. Got: '" << _s << "'"; \ + } \ +} while (0); + // Substring regular expressions in extended regex (POSIX) syntax. #define ASSERT_STR_MATCHES(str, pattern) \ ASSERT_THAT(str, testing::ContainsRegex(pattern))
