kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ale...@apache.org
Subject kudu git commit: [flags] run validators after processing help flags
Date Thu, 10 May 2018 19:07:34 GMT
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 <adar@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 <aserbin@cloudera.com>
Authored: Mon May 7 22:37:34 2018 -0700
Committer: Alexey Serbin <aserbin@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;


Mime
View raw message