Repository: kudu Updated Branches: refs/heads/master 26af97c8f -> 3f43c03a1
[flags] run validators after processing help flags Run flag validators (both individual and group ones) after processing the help flags. Also, this is a follow-up for 297b72bd26cdd546f0a73cda7487c80566388492: the default value for the --force_block_cache_capacity flag is set to 'true' everywhere but in master and tablet server. Otherwise, corresponding group validator could strike while running binaries that link the cfile library (e.g. the kudu CLI tool) at machines with less than 256 MB of available memory. Change-Id: I51f263890c91251f0d13c826c40dfd20fe284b59 Reviewed-on: http://gerrit.cloudera.org:8080/10342 Reviewed-by: Adar Dembo <a...@cloudera.com> 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/3f43c03a Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/3f43c03a Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/3f43c03a Branch: refs/heads/master Commit: 3f43c03a1e4130cde6556a298ad8bb3f32897993 Parents: 26af97c Author: Alexey Serbin <aser...@cloudera.com> Authored: Mon May 7 22:37:34 2018 -0700 Committer: Alexey Serbin <aser...@cloudera.com> Committed: Thu May 10 19:05:50 2018 +0000 ---------------------------------------------------------------------- src/kudu/cfile/block_cache.cc | 6 +++++- src/kudu/consensus/CMakeLists.txt | 2 +- src/kudu/integration-tests/CMakeLists.txt | 4 ++-- src/kudu/master/master_main.cc | 11 +++++++++-- src/kudu/tools/CMakeLists.txt | 27 ++++++++++++-------------- src/kudu/tserver/CMakeLists.txt | 2 +- src/kudu/tserver/tablet_server_main.cc | 10 ++++++++-- src/kudu/util/flags.cc | 22 +++++++++------------ src/kudu/util/metrics.cc | 4 +--- src/kudu/util/metrics.h | 4 ++-- 10 files changed, 50 insertions(+), 42 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/3f43c03a/src/kudu/cfile/block_cache.cc ---------------------------------------------------------------------- diff --git a/src/kudu/cfile/block_cache.cc b/src/kudu/cfile/block_cache.cc index 42c4201..ff7219d 100644 --- a/src/kudu/cfile/block_cache.cc +++ b/src/kudu/cfile/block_cache.cc @@ -38,7 +38,11 @@ DEFINE_int64(block_cache_capacity_mb, 512, "block cache capacity in MB"); TAG_FLAG(block_cache_capacity_mb, stable); -DEFINE_bool(force_block_cache_capacity, false, +// Yes, it's strange: the default value is 'true' but that's intentional. +// The idea is to avoid the corresponding group flag validator striking +// while running anything but master and tserver. As for the master and tserver, +// those have the default value for this flag set to 'false'. +DEFINE_bool(force_block_cache_capacity, true, "Force Kudu to accept the block cache size, even if it is unsafe."); TAG_FLAG(force_block_cache_capacity, unsafe); TAG_FLAG(force_block_cache_capacity, hidden); http://git-wip-us.apache.org/repos/asf/kudu/blob/3f43c03a/src/kudu/consensus/CMakeLists.txt ---------------------------------------------------------------------- diff --git a/src/kudu/consensus/CMakeLists.txt b/src/kudu/consensus/CMakeLists.txt index 11463a6..ca21394 100644 --- a/src/kudu/consensus/CMakeLists.txt +++ b/src/kudu/consensus/CMakeLists.txt @@ -120,7 +120,7 @@ target_link_libraries(consensus set(KUDU_TEST_LINK_LIBS log consensus - tserver + tserver_proto cfile tablet kudu_util http://git-wip-us.apache.org/repos/asf/kudu/blob/3f43c03a/src/kudu/integration-tests/CMakeLists.txt ---------------------------------------------------------------------- diff --git a/src/kudu/integration-tests/CMakeLists.txt b/src/kudu/integration-tests/CMakeLists.txt index 60ff72a..f5ed494 100644 --- a/src/kudu/integration-tests/CMakeLists.txt +++ b/src/kudu/integration-tests/CMakeLists.txt @@ -34,9 +34,9 @@ set(INTEGRATION_TESTS_SRCS add_library(itest_util ${INTEGRATION_TESTS_SRCS}) target_link_libraries(itest_util - tserver + tserver_proto tserver_test_util - master + master_proto mini_cluster ksck kudu_client http://git-wip-us.apache.org/repos/asf/kudu/blob/3f43c03a/src/kudu/master/master_main.cc ---------------------------------------------------------------------- diff --git a/src/kudu/master/master_main.cc b/src/kudu/master/master_main.cc index 2dbcac2..1abb837 100644 --- a/src/kudu/master/master_main.cc +++ b/src/kudu/master/master_main.cc @@ -18,6 +18,7 @@ #include <iostream> #include <string> +#include <gflags/gflags.h> #include <gflags/gflags_declare.h> #include <glog/logging.h> @@ -33,9 +34,9 @@ using kudu::master::Master; -DECLARE_string(rpc_bind_addresses); -DECLARE_int32(webserver_port); DECLARE_bool(evict_failed_followers); +DECLARE_int32(webserver_port); +DECLARE_string(rpc_bind_addresses); namespace kudu { namespace master { @@ -53,6 +54,12 @@ static int MasterMain(int argc, char** argv) { // the desired replication factor. (It's not turtles all the way down!) FLAGS_evict_failed_followers = false; + // Setting the default value of the 'force_block_cache_capacity' flag to + // 'false' makes the corresponding group validator enforce proper settings + // for the memory limit and the cfile cache capacity. + CHECK_NE("", SetCommandLineOptionWithMode("force_block_cache_capacity", + "false", gflags::SET_FLAGS_DEFAULT)); + GFlagsMap default_flags = GetFlagsMap(); ParseCommandLineFlags(&argc, &argv, true); http://git-wip-us.apache.org/repos/asf/kudu/blob/3f43c03a/src/kudu/tools/CMakeLists.txt ---------------------------------------------------------------------- diff --git a/src/kudu/tools/CMakeLists.txt b/src/kudu/tools/CMakeLists.txt index 3ee1634..832bb1d 100644 --- a/src/kudu/tools/CMakeLists.txt +++ b/src/kudu/tools/CMakeLists.txt @@ -15,20 +15,6 @@ # specific language governing permissions and limitations # under the License. -set(LINK_LIBS - cfile - consensus - gutil - kudu_client - kudu_common - kudu_fs - kudu_util - log - tablet - tserver - ${KUDU_BASE_LIBS} -) - ####################################### # tool_proto ####################################### @@ -58,8 +44,19 @@ add_library(kudu_tools_util tool_action_common.cc ) target_link_libraries(kudu_tools_util + cfile + consensus + gutil + kudu_client + kudu_common + kudu_fs + kudu_util + log + server_process + tablet tool_proto - ${LINK_LIBS}) + ${KUDU_BASE_LIBS} +) ####################################### # ksck http://git-wip-us.apache.org/repos/asf/kudu/blob/3f43c03a/src/kudu/tserver/CMakeLists.txt ---------------------------------------------------------------------- diff --git a/src/kudu/tserver/CMakeLists.txt b/src/kudu/tserver/CMakeLists.txt index 3a67f44..c411e92 100644 --- a/src/kudu/tserver/CMakeLists.txt +++ b/src/kudu/tserver/CMakeLists.txt @@ -132,8 +132,8 @@ target_link_libraries(tserver server_process tablet tablet_copy_proto - tserver_proto tserver_admin_proto + tserver_proto tserver_service_proto) ######################################### http://git-wip-us.apache.org/repos/asf/kudu/blob/3f43c03a/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 0df91e8..17989be 100644 --- a/src/kudu/tserver/tablet_server_main.cc +++ b/src/kudu/tserver/tablet_server_main.cc @@ -27,8 +27,8 @@ #include "kudu/tserver/tablet_server.h" #include "kudu/tserver/tablet_server_options.h" #include "kudu/util/fault_injection.h" -#include "kudu/util/flags.h" #include "kudu/util/flag_tags.h" +#include "kudu/util/flags.h" #include "kudu/util/init.h" #include "kudu/util/logging.h" #include "kudu/util/monotime.h" @@ -37,9 +37,9 @@ using kudu::tserver::TabletServer; -DECLARE_string(rpc_bind_addresses); DECLARE_int32(rpc_num_service_threads); DECLARE_int32(webserver_port); +DECLARE_string(rpc_bind_addresses); DEFINE_double(fault_before_start, 0.0, "Fake fault flag that always causes a crash on startup. " @@ -59,6 +59,12 @@ static int TabletServerMain(int argc, char** argv) { FLAGS_rpc_num_service_threads = 20; FLAGS_webserver_port = TabletServer::kDefaultWebPort; + // Setting the default value of the 'force_block_cache_capacity' flag to + // 'false' makes the corresponding group validator enforce proper settings + // for the memory limit and the cfile cache capacity. + CHECK_NE("", SetCommandLineOptionWithMode("force_block_cache_capacity", + "false", gflags::SET_FLAGS_DEFAULT)); + GFlagsMap default_flags = GetFlagsMap(); ParseCommandLineFlags(&argc, &argv, true); http://git-wip-us.apache.org/repos/asf/kudu/blob/3f43c03a/src/kudu/util/flags.cc ---------------------------------------------------------------------- diff --git a/src/kudu/util/flags.cc b/src/kudu/util/flags.cc index 3520be5..047c893 100644 --- a/src/kudu/util/flags.cc +++ b/src/kudu/util/flags.cc @@ -384,12 +384,6 @@ void DumpFlagsXML() { } cout << "</AllFlags>" << endl; - exit(1); -} - -void ShowVersionAndExit() { - cout << VersionInfo::GetAllVersionInfo() << endl; - exit(0); } // Check that, if any flags tagged with 'tag' have been specified to @@ -493,19 +487,21 @@ int ParseCommandLineFlags(int* argc, char*** argv, bool remove_flags) { } void HandleCommonFlags() { - CheckFlagsAllowed(); - RunCustomValidators(); - if (FLAGS_helpxml) { DumpFlagsXML(); + exit(1); } else if (FLAGS_dump_metrics_json) { - MetricPrototypeRegistry::get()->WriteAsJsonAndExit(); + MetricPrototypeRegistry::get()->WriteAsJson(); + exit(0); } else if (FLAGS_version) { - ShowVersionAndExit(); - } else { - google::HandleCommandLineHelpFlags(); + cout << VersionInfo::GetAllVersionInfo() << endl; + exit(0); } + google::HandleCommandLineHelpFlags(); + CheckFlagsAllowed(); + RunCustomValidators(); + if (FLAGS_disable_core_dumps) { DisableCoreDumps(); } http://git-wip-us.apache.org/repos/asf/kudu/blob/3f43c03a/src/kudu/util/metrics.cc ---------------------------------------------------------------------- diff --git a/src/kudu/util/metrics.cc b/src/kudu/util/metrics.cc index bc73d08..dc30360 100644 --- a/src/kudu/util/metrics.cc +++ b/src/kudu/util/metrics.cc @@ -16,7 +16,6 @@ // under the License. #include "kudu/util/metrics.h" -#include <cstdlib> #include <iostream> #include <map> #include <utility> @@ -437,12 +436,11 @@ void MetricPrototypeRegistry::WriteAsJson(JsonWriter* writer) const { writer->EndObject(); } -void MetricPrototypeRegistry::WriteAsJsonAndExit() const { +void MetricPrototypeRegistry::WriteAsJson() const { std::ostringstream s; JsonWriter w(&s, JsonWriter::PRETTY); WriteAsJson(&w); std::cout << s.str() << std::endl; - exit(0); } // http://git-wip-us.apache.org/repos/asf/kudu/blob/3f43c03a/src/kudu/util/metrics.h ---------------------------------------------------------------------- diff --git a/src/kudu/util/metrics.h b/src/kudu/util/metrics.h index ee9bad4..d49afac 100644 --- a/src/kudu/util/metrics.h +++ b/src/kudu/util/metrics.h @@ -698,8 +698,8 @@ class MetricPrototypeRegistry { void WriteAsJson(JsonWriter* writer) const; // Convenience wrapper around WriteAsJson(...). This dumps the JSON information - // to stdout and then exits. - void WriteAsJsonAndExit() const; + // to stdout. + void WriteAsJson() const; private: friend class Singleton<MetricPrototypeRegistry>; friend class MetricPrototype;
