kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From granthe...@apache.org
Subject [kudu] 01/03: [tools] Support running the master and tablet server via the kudu binary
Date Tue, 14 Jan 2020 19:41:33 GMT
This is an automated email from the ASF dual-hosted git repository.

granthenke pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit 4f82a46da97d9987139aee26614e904118bce4a9
Author: Grant Henke <granthenke@apache.org>
AuthorDate: Sun Feb 17 14:48:15 2019 -0600

    [tools] Support running the master and tablet server via the kudu binary
    
    Adds commands to the kudu tools to run the master
    and tablet server:
       - `kudu master run ...`
       - `kudu tserver run ...`
    
    This means we can ship a single binary in the kudu
    docker image or potentially the kudu-binary jar
    reducing the size by approximately 66%. Though
    follow up changes may be needed in the kudu-binary
    case.
    
    The behavior is the same as running the kudu-master
    and kudu-tserver binaries. The logging initialization was
    updated to ensure that the logging filenames are
    the same as when running via the dedicated binaries.
    A few logging fixes are included to ensure this capability
    is possible.
    
    This patch also contains a few related fixes for default
    flags ensuring that they can be overwritten when
    necessary. These changes also ensure flags are not
    incorrectly reported as non-default.
    
    Change-Id: I3717cbac930b3506a76f7a51388c64afbcbb480e
    Reviewed-on: http://gerrit.cloudera.org:8080/12517
    Tested-by: Grant Henke <granthenke@apache.org>
    Reviewed-by: Adar Dembo <adar@cloudera.com>
    Reviewed-by: Andrew Wong <awong@cloudera.com>
    Reviewed-by: Alexey Serbin <aserbin@cloudera.com>
---
 src/kudu/master/CMakeLists.txt                     |  1 +
 src/kudu/master/master-test.cc                     |  2 +
 src/kudu/master/master_main.cc                     | 73 +-----------------
 .../master/{master_main.cc => master_runner.cc}    | 89 ++++++++--------------
 src/kudu/master/master_runner.h                    | 31 ++++++++
 src/kudu/server/diagnostics_log.cc                 |  4 +-
 src/kudu/server/diagnostics_log.h                  |  3 +-
 src/kudu/server/server_base.cc                     |  4 +-
 src/kudu/tools/kudu-tool-test.cc                   | 12 +--
 src/kudu/tools/tool_action.cc                      | 27 ++++++-
 src/kudu/tools/tool_action.h                       | 15 ++++
 src/kudu/tools/tool_action_master.cc               | 40 +++++++++-
 src/kudu/tools/tool_action_tserver.cc              | 41 +++++++++-
 src/kudu/tools/tool_main.cc                        |  8 +-
 src/kudu/tserver/CMakeLists.txt                    |  1 +
 src/kudu/tserver/tablet_server.h                   |  1 +
 src/kudu/tserver/tablet_server_main.cc             | 55 +------------
 ...blet_server_main.cc => tablet_server_runner.cc} | 52 +++++--------
 src/kudu/tserver/tablet_server_runner.h            | 31 ++++++++
 src/kudu/util/flag_tags-test.cc                    | 16 ++--
 src/kudu/util/flags-test.cc                        |  9 +--
 src/kudu/util/flags.cc                             | 28 ++++---
 src/kudu/util/flags.h                              |  7 +-
 src/kudu/util/logging.cc                           | 10 ++-
 src/kudu/util/logging.h                            |  1 +
 src/kudu/util/minidump.cc                          | 12 ++-
 src/kudu/util/rolling_log-test.cc                  |  6 +-
 src/kudu/util/rolling_log.cc                       | 10 ++-
 src/kudu/util/rolling_log.h                        |  6 +-
 29 files changed, 316 insertions(+), 279 deletions(-)

diff --git a/src/kudu/master/CMakeLists.txt b/src/kudu/master/CMakeLists.txt
index 39092fd..d4e27be 100644
--- a/src/kudu/master/CMakeLists.txt
+++ b/src/kudu/master/CMakeLists.txt
@@ -41,6 +41,7 @@ set(MASTER_SRCS
   master_cert_authority.cc
   master_options.cc
   master_path_handlers.cc
+  master_runner.cc
   master_service.cc
   mini_master.cc
   placement_policy.cc
diff --git a/src/kudu/master/master-test.cc b/src/kudu/master/master-test.cc
index 331e502..dfb13cd 100644
--- a/src/kudu/master/master-test.cc
+++ b/src/kudu/master/master-test.cc
@@ -116,6 +116,7 @@ DECLARE_int32(master_inject_latency_on_tablet_lookups_ms);
 DECLARE_int64(live_row_count_for_testing);
 DECLARE_int64(on_disk_size_for_testing);
 DECLARE_string(location_mapping_cmd);
+DECLARE_string(log_filename);
 
 namespace kudu {
 namespace master {
@@ -913,6 +914,7 @@ TEST_F(MasterTest, TestDumpStacksOnRpcQueueOverflow) {
   // Use a new log directory so that the tserver and master don't share the
   // same one. This allows us to isolate the diagnostics log from the master.
   FLAGS_log_dir = JoinPathSegments(GetTestDataDirectory(), "master-logs");
+  FLAGS_log_filename = "kudu-master";
   Status s = env_->CreateDir(FLAGS_log_dir);
   ASSERT_TRUE(s.ok() || s.IsAlreadyPresent()) << s.ToString();
   mini_master_->Shutdown();
diff --git a/src/kudu/master/master_main.cc b/src/kudu/master/master_main.cc
index 7066925..613bdb8 100644
--- a/src/kudu/master/master_main.cc
+++ b/src/kudu/master/master_main.cc
@@ -16,95 +16,28 @@
 // under the License.
 
 #include <iostream>
-#include <string>
 
-#include <gflags/gflags.h>
 #include <glog/logging.h>
 
-#include "kudu/gutil/strings/substitute.h"
-#include "kudu/master/master.h"
-#include "kudu/util/flag_validators.h"
+#include "kudu/master/master_runner.h"
 #include "kudu/util/flags.h"
 #include "kudu/util/init.h"
 #include "kudu/util/logging.h"
-#include "kudu/util/monotime.h"
 #include "kudu/util/status.h"
-#include "kudu/util/version_info.h"
-
-using kudu::master::Master;
-
-DECLARE_bool(evict_failed_followers);
-DECLARE_int32(webserver_port);
-DECLARE_string(rpc_bind_addresses);
-
-DECLARE_bool(hive_metastore_sasl_enabled);
-DECLARE_string(keytab_file);
 
 namespace kudu {
 namespace master {
 
-namespace {
-// Validates that if the HMS is configured with SASL enabled, the server has a
-// keytab available. This is located in master.cc because the HMS module (where
-// -hive_metastore_sasl_enabled is defined) doesn't link to the server module
-// (where --keytab_file is defined), and vice-versa. The master module is the
-// first module which links to both.
-// Note: this check only needs to be run on a server. E.g. tools that run with
-// the HMS don't need to pass in a keytab.
-bool ValidateHiveMetastoreSaslEnabled() {
-  if (FLAGS_hive_metastore_sasl_enabled &&
-      FLAGS_keytab_file.empty()) {
-    LOG(ERROR) << "When the Hive Metastore has SASL enabled "
-                  "(--hive_metastore_sasl_enabled), Kudu must be configured with "
-                  "a keytab (--keytab_file).";
-    return false;
-  }
-  return true;
-}
-GROUP_FLAG_VALIDATOR(hive_metastore_sasl_enabled, ValidateHiveMetastoreSaslEnabled);
-} // anonymous namespace
-
 static int MasterMain(int argc, char** argv) {
   RETURN_MAIN_NOT_OK(InitKudu(), "InitKudu() failed", 1);
-
-  // Reset some default values before parsing gflags.
-  FLAGS_rpc_bind_addresses = strings::Substitute("0.0.0.0:$0",
-                                                 Master::kDefaultPort);
-  FLAGS_webserver_port = Master::kDefaultWebPort;
-
-  // A multi-node Master leader should not evict failed Master followers
-  // because there is no-one to assign replacement servers in order to maintain
-  // 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();
-
+  SetMasterFlagDefaults();
   ParseCommandLineFlags(&argc, &argv, true);
   if (argc != 1) {
     std::cerr << "usage: " << argv[0] << std::endl;
     return 2;
   }
-  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();
-
-  Master server({});
-  RETURN_MAIN_NOT_OK(server.Init(), "Init() failed", 3);
-  RETURN_MAIN_NOT_OK(server.Start(), "Start() failed", 4);
-
-  while (true) {
-    SleepFor(MonoDelta::FromSeconds(60));
-  }
+  RETURN_MAIN_NOT_OK(RunMasterServer(), "RunMasterServer() failed", 3);
 
   return 0;
 }
diff --git a/src/kudu/master/master_main.cc b/src/kudu/master/master_runner.cc
similarity index 50%
copy from src/kudu/master/master_main.cc
copy to src/kudu/master/master_runner.cc
index 7066925..07cec26 100644
--- a/src/kudu/master/master_main.cc
+++ b/src/kudu/master/master_runner.cc
@@ -15,6 +15,8 @@
 // specific language governing permissions and limitations
 // under the License.
 
+#include "kudu/master/master_runner.h"
+
 #include <iostream>
 #include <string>
 
@@ -23,95 +25,64 @@
 
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/master/master.h"
-#include "kudu/util/flag_validators.h"
 #include "kudu/util/flags.h"
-#include "kudu/util/init.h"
-#include "kudu/util/logging.h"
 #include "kudu/util/monotime.h"
-#include "kudu/util/status.h"
 #include "kudu/util/version_info.h"
 
 using kudu::master::Master;
+using std::string;
 
 DECLARE_bool(evict_failed_followers);
-DECLARE_int32(webserver_port);
-DECLARE_string(rpc_bind_addresses);
-
-DECLARE_bool(hive_metastore_sasl_enabled);
-DECLARE_string(keytab_file);
 
 namespace kudu {
 namespace master {
 
-namespace {
-// Validates that if the HMS is configured with SASL enabled, the server has a
-// keytab available. This is located in master.cc because the HMS module (where
-// -hive_metastore_sasl_enabled is defined) doesn't link to the server module
-// (where --keytab_file is defined), and vice-versa. The master module is the
-// first module which links to both.
-// Note: this check only needs to be run on a server. E.g. tools that run with
-// the HMS don't need to pass in a keytab.
-bool ValidateHiveMetastoreSaslEnabled() {
-  if (FLAGS_hive_metastore_sasl_enabled &&
-      FLAGS_keytab_file.empty()) {
-    LOG(ERROR) << "When the Hive Metastore has SASL enabled "
-                  "(--hive_metastore_sasl_enabled), Kudu must be configured with "
-                  "a keytab (--keytab_file).";
-    return false;
-  }
-  return true;
-}
-GROUP_FLAG_VALIDATOR(hive_metastore_sasl_enabled, ValidateHiveMetastoreSaslEnabled);
-} // anonymous namespace
-
-static int MasterMain(int argc, char** argv) {
-  RETURN_MAIN_NOT_OK(InitKudu(), "InitKudu() failed", 1);
-
+void SetMasterFlagDefaults() {
   // Reset some default values before parsing gflags.
-  FLAGS_rpc_bind_addresses = strings::Substitute("0.0.0.0:$0",
-                                                 Master::kDefaultPort);
-  FLAGS_webserver_port = Master::kDefaultWebPort;
-
-  // A multi-node Master leader should not evict failed Master followers
-  // because there is no-one to assign replacement servers in order to maintain
-  // the desired replication factor. (It's not turtles all the way down!)
-  FLAGS_evict_failed_followers = false;
+  CHECK_NE("", google::SetCommandLineOptionWithMode("rpc_bind_addresses",
+                                                    strings::Substitute(
+                                                        "0.0.0.0:$0",
+                                                        Master::kDefaultPort).c_str(),
+                                                    google::FlagSettingMode::SET_FLAGS_DEFAULT));
+  CHECK_NE("", google::SetCommandLineOptionWithMode("webserver_port",
+                                                    std::to_string(
+                                                        Master::kDefaultWebPort).c_str(),
+                                                    google::FlagSettingMode::SET_FLAGS_DEFAULT));
 
   // 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));
+                                            "false",
+                                            gflags::SET_FLAGS_DEFAULT));
 
-  GFlagsMap default_flags = GetFlagsMap();
-
-  ParseCommandLineFlags(&argc, &argv, true);
-  if (argc != 1) {
-    std::cerr << "usage: " << argv[0] << std::endl;
-    return 2;
-  }
-  std::string nondefault_flags = GetNonDefaultFlags(default_flags);
-  InitGoogleLoggingSafe(argv[0]);
+  // A multi-node Master leader should not evict failed Master followers
+  // because there is no-one to assign replacement servers in order to maintain
+  // the desired replication factor. (It's not turtles all the way down!)
+  CHECK_NE("", SetCommandLineOptionWithMode("evict_failed_followers",
+                                            "false",
+                                            gflags::SET_FLAGS_DEFAULT));
+  // SET_FLAGS_DEFAULT won't reset the flag value if it has previously been
+  // set, instead it will only change the default. Because we want to ensure
+  // evict_failed_followers is always false, we explicitly set the flag.
+  FLAGS_evict_failed_followers = false;
+}
 
+Status RunMasterServer() {
+  string nondefault_flags = GetNonDefaultFlags();
   LOG(INFO) << "Master server non-default flags:\n"
             << nondefault_flags << '\n'
             << "Master server version:\n"
             << VersionInfo::GetAllVersionInfo();
 
   Master server({});
-  RETURN_MAIN_NOT_OK(server.Init(), "Init() failed", 3);
-  RETURN_MAIN_NOT_OK(server.Start(), "Start() failed", 4);
+  RETURN_NOT_OK(server.Init());
+  RETURN_NOT_OK(server.Start());
 
   while (true) {
     SleepFor(MonoDelta::FromSeconds(60));
   }
-
-  return 0;
 }
 
 } // namespace master
 } // namespace kudu
-
-int main(int argc, char** argv) {
-  return kudu::master::MasterMain(argc, argv);
-}
diff --git a/src/kudu/master/master_runner.h b/src/kudu/master/master_runner.h
new file mode 100644
index 0000000..a3fc7cf
--- /dev/null
+++ b/src/kudu/master/master_runner.h
@@ -0,0 +1,31 @@
+// 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.
+#pragma once
+
+#include "kudu/util/status.h"
+
+namespace kudu {
+namespace master {
+
+// Adjusts the default values of some gflags for use with the master server.
+void SetMasterFlagDefaults();
+
+// Initialize and start a master server. Will run until interrupted.
+Status RunMasterServer();
+
+} // namespace master
+} // namespace kudu
diff --git a/src/kudu/server/diagnostics_log.cc b/src/kudu/server/diagnostics_log.cc
index 415b5fa..2b11540 100644
--- a/src/kudu/server/diagnostics_log.cc
+++ b/src/kudu/server/diagnostics_log.cc
@@ -110,8 +110,10 @@ class DiagnosticsLog::SymbolSet {
 };
 
 DiagnosticsLog::DiagnosticsLog(string log_dir,
+                               string program_name,
                                MetricRegistry* metric_registry) :
     log_dir_(std::move(log_dir)),
+    program_name_(std::move(program_name)),
     metric_registry_(metric_registry),
     wake_(&lock_),
     metrics_log_interval_(MonoDelta::FromSeconds(60)),
@@ -134,7 +136,7 @@ void DiagnosticsLog::DumpStacksNow(std::string reason) {
 
 
 Status DiagnosticsLog::Start() {
-  unique_ptr<RollingLog> l(new RollingLog(Env::Default(), log_dir_, "diagnostics"));
+  unique_ptr<RollingLog> l(new RollingLog(Env::Default(), log_dir_, program_name_, "diagnostics"));
   RETURN_NOT_OK_PREPEND(l->Open(), "unable to open diagnostics log");
   log_ = std::move(l);
   Status s = Thread::Create("server", "diag-logger",
diff --git a/src/kudu/server/diagnostics_log.h b/src/kudu/server/diagnostics_log.h
index 5b8cfab..3841007 100644
--- a/src/kudu/server/diagnostics_log.h
+++ b/src/kudu/server/diagnostics_log.h
@@ -39,7 +39,7 @@ namespace server {
 
 class DiagnosticsLog {
  public:
-  DiagnosticsLog(std::string log_dir, MetricRegistry* metric_registry);
+  DiagnosticsLog(std::string log_dir, std::string program_name, MetricRegistry* metric_registry);
   ~DiagnosticsLog();
 
   void SetMetricsLogInterval(MonoDelta interval);
@@ -68,6 +68,7 @@ class DiagnosticsLog {
   MonoTime ComputeNextWakeup(DiagnosticsLog::WakeupType type) const;
 
   const std::string log_dir_;
+  const std::string program_name_;
   const MetricRegistry* metric_registry_;
 
   scoped_refptr<Thread> thread_;
diff --git a/src/kudu/server/server_base.cc b/src/kudu/server/server_base.cc
index dcca690..c089eb5 100644
--- a/src/kudu/server/server_base.cc
+++ b/src/kudu/server/server_base.cc
@@ -225,6 +225,7 @@ DECLARE_bool(use_hybrid_clock);
 DECLARE_int32(dns_resolver_max_threads_num);
 DECLARE_uint32(dns_resolver_cache_capacity_mb);
 DECLARE_uint32(dns_resolver_cache_ttl_sec);
+DECLARE_string(log_filename);
 
 METRIC_DECLARE_gauge_size(merged_entities_count_of_server);
 
@@ -691,7 +692,8 @@ Status ServerBase::StartMetricsLogging() {
     LOG(INFO) << "Not starting metrics log since no log directory was specified.";
     return Status::OK();
   }
-  unique_ptr<DiagnosticsLog> l(new DiagnosticsLog(FLAGS_log_dir, metric_registry_.get()));
+  unique_ptr<DiagnosticsLog> l(new DiagnosticsLog(FLAGS_log_dir, FLAGS_log_filename,
+      metric_registry_.get()));
   l->SetMetricsLogInterval(MonoDelta::FromMilliseconds(options_.metrics_log_interval_ms));
   RETURN_NOT_OK(l->Start());
   diag_log_ = std::move(l);
diff --git a/src/kudu/tools/kudu-tool-test.cc b/src/kudu/tools/kudu-tool-test.cc
index 88684f8..0c47957 100644
--- a/src/kudu/tools/kudu-tool-test.cc
+++ b/src/kudu/tools/kudu-tool-test.cc
@@ -327,11 +327,11 @@ class ToolTest : public KuduTest {
     ASSERT_TRUE(stdout.empty());
     ASSERT_FALSE(stderr.empty());
 
-    // If it was an invalid command, the usage string is on the third line.
-    int usage_idx = 1;
+    // If it was an invalid command, the usage string is on the second line.
+    int usage_idx = 0;
     if (!expected_status.ok()) {
-      ASSERT_EQ(expected_status.ToString(), stderr[1]);
-      usage_idx = 2;
+      ASSERT_EQ(expected_status.ToString(), stderr[0]);
+      usage_idx = 1;
     }
     ASSERT_EQ(0, stderr[usage_idx].find("Usage: "));
 
@@ -354,8 +354,8 @@ class ToolTest : public KuduTest {
     vector<string> err_lines;
     RunTool(arg_str, nullptr, nullptr, nullptr, /* stderr_lines = */ &err_lines);
     ASSERT_GE(err_lines.size(), 3) << err_lines;
-    ASSERT_EQ(expected_status.ToString(), err_lines[1]);
-    ASSERT_STR_MATCHES(err_lines[3], "Usage: kudu.*");
+    ASSERT_EQ(expected_status.ToString(), err_lines[0]);
+    ASSERT_STR_MATCHES(err_lines[2], "Usage: kudu.*");
   }
 
   void RunFsCheck(const string& arg_str,
diff --git a/src/kudu/tools/tool_action.cc b/src/kudu/tools/tool_action.cc
index 5ed62fd..b1ed9ab 100644
--- a/src/kudu/tools/tool_action.cc
+++ b/src/kudu/tools/tool_action.cc
@@ -33,6 +33,8 @@
 #include "kudu/gutil/strings/split.h"
 #include "kudu/gutil/strings/stringpiece.h"
 #include "kudu/gutil/strings/substitute.h"
+#include "kudu/util/flags.h"
+#include "kudu/util/logging.h"
 #include "kudu/util/url-coding.h"
 
 using std::pair;
@@ -213,6 +215,12 @@ ActionBuilder& ActionBuilder::ExtraDescription(const string& extra_description)
   return *this;
 }
 
+ActionBuilder& ActionBuilder::ProgramName(const string& program_name) {
+  CHECK(!program_name_.is_initialized());
+  program_name_ = program_name;
+  return *this;
+}
+
 ActionBuilder& ActionBuilder::AddRequiredParameter(
     const ActionArgsDescriptor::Arg& arg) {
   args_.required.push_back(arg);
@@ -246,6 +254,7 @@ unique_ptr<Action> ActionBuilder::Build() {
   action->name_ = name_;
   action->description_ = description_;
   action->extra_description_ = extra_description_;
+  action->program_name_ = program_name_;
   action->runner_ = runner_;
   action->args_ = args_;
   return action;
@@ -255,6 +264,18 @@ Status Action::Run(const vector<Mode*>& chain,
                    const unordered_map<string, string>& required_args,
                    const vector<string>& variadic_args) const {
   SetOptionalParameterDefaultValues();
+
+  // If `program_name_` is defined,  initialize the logging as if the
+  // program binary name was `program_name_`, otherwise fallback to the
+  // default behavior of using argv0.
+  if (program_name_) {
+    CHECK_NE("", google::SetCommandLineOptionWithMode("log_filename",
+        program_name_->c_str(),
+        google::FlagSettingMode::SET_FLAGS_DEFAULT));
+  }
+  kudu::InitGoogleLoggingSafe(program_name_.get_value_or(gflags::GetArgv0()).c_str());
+  kudu::ValidateFlags();
+
   return runner_({ chain, this, required_args, variadic_args });
 }
 
@@ -396,9 +417,9 @@ string Action::BuildHelpXML(const vector<Mode*>& chain) const {
 void Action::SetOptionalParameterDefaultValues() const {
   for (const auto& param : args_.optional) {
     if (param.default_value) {
-      google::SetCommandLineOptionWithMode(param.name.c_str(),
-                                           param.default_value->c_str(),
-                                           google::FlagSettingMode::SET_FLAGS_DEFAULT);
+      CHECK_NE("", google::SetCommandLineOptionWithMode(param.name.c_str(),
+          param.default_value->c_str(),
+          google::FlagSettingMode::SET_FLAGS_DEFAULT));
     }
   }
 }
diff --git a/src/kudu/tools/tool_action.h b/src/kudu/tools/tool_action.h
index 5b7aace..b8fea5a 100644
--- a/src/kudu/tools/tool_action.h
+++ b/src/kudu/tools/tool_action.h
@@ -204,6 +204,13 @@ class ActionBuilder {
   // action's help following Description().
   ActionBuilder& ExtraDescription(const std::string& extra_description);
 
+  // Sets the program name to use when running this action. If unset,
+  // argv0 is used.
+  //
+  // This will ensure logging is initialized as if the program binary name was
+  // the passed `program_name`.
+  ActionBuilder& ProgramName(const std::string& program_name);
+
   // Add a new required parameter to this builder.
   //
   // This parameter will be parsed as a positional argument following the name
@@ -247,6 +254,8 @@ class ActionBuilder {
 
   boost::optional<std::string> extra_description_;
 
+  boost::optional<std::string> program_name_;
+
   ActionRunner runner_;
 
   ActionArgsDescriptor args_;
@@ -284,6 +293,10 @@ class Action {
     return extra_description_;
   }
 
+  const boost::optional<std::string>& program_name() const {
+    return program_name_;
+  }
+
   const ActionArgsDescriptor& args() const { return args_; }
 
  private:
@@ -301,6 +314,8 @@ class Action {
 
   boost::optional<std::string> extra_description_;
 
+  boost::optional<std::string> program_name_;
+
   ActionRunner runner_;
 
   ActionArgsDescriptor args_;
diff --git a/src/kudu/tools/tool_action_master.cc b/src/kudu/tools/tool_action_master.cc
index dff62e8..8ed5db1 100644
--- a/src/kudu/tools/tool_action_master.cc
+++ b/src/kudu/tools/tool_action_master.cc
@@ -28,7 +28,7 @@
 
 #include <boost/algorithm/string/predicate.hpp>
 #include <boost/optional/optional.hpp>
-#include <gflags/gflags_declare.h>
+#include <gflags/gflags.h>
 #include <glog/logging.h>
 
 #include "kudu/common/common.pb.h"
@@ -43,9 +43,11 @@
 #include "kudu/master/master.h"
 #include "kudu/master/master.pb.h"
 #include "kudu/master/master.proxy.h"
+#include "kudu/master/master_runner.h"
 #include "kudu/rpc/rpc_controller.h"
 #include "kudu/tools/tool_action.h"
 #include "kudu/tools/tool_action_common.h"
+#include "kudu/util/init.h"
 #include "kudu/util/monotime.h"
 #include "kudu/util/status.h"
 
@@ -87,6 +89,18 @@ Status MasterGetFlags(const RunnerContext& context) {
   return PrintServerFlags(address, Master::kDefaultPort);
 }
 
+Status MasterRun(const RunnerContext& context) {
+  RETURN_NOT_OK(InitKudu());
+
+  // Enable redaction by default. Unlike most tools, we don't want user data
+  // printed to the console/log to be shown by default.
+  CHECK_NE("", google::SetCommandLineOptionWithMode("redact",
+      "all", google::FlagSettingMode::SET_FLAGS_DEFAULT));
+
+  master::SetMasterFlagDefaults();
+  return master::RunMasterServer();
+}
+
 Status MasterSetFlag(const RunnerContext& context) {
   const string& address = FindOrDie(context.required_args, kMasterAddressArg);
   const string& flag = FindOrDie(context.required_args, kFlagArg);
@@ -343,6 +357,30 @@ unique_ptr<Mode> BuildMasterMode() {
     builder.AddAction(std::move(get_flags));
   }
   {
+    unique_ptr<Action> run =
+        ActionBuilder("run", &MasterRun)
+        .ProgramName("kudu-master")
+        .Description("Runs a Kudu Master")
+        .ExtraDescription("Note: The master server is started in this process and "
+                          "runs until interrupted.\n\n"
+                          "The most common configuration flags are described below. "
+                          "For all the configuration options pass --helpfull or see "
+                          "https://kudu.apache.org/docs/configuration_reference.html"
+                          "#kudu-master_supported")
+        .AddOptionalParameter("master_addresses")
+        // Even though fs_wal_dir is required, we don't want it to be positional argument.
+        // This allows it to be passed as a standard flag.
+        .AddOptionalParameter("fs_wal_dir")
+        .AddOptionalParameter("fs_data_dirs")
+        .AddOptionalParameter("fs_metadata_dir")
+        .AddOptionalParameter("log_dir")
+        // Unlike most tools we don't log to stderr by default to match the
+        // kudu-master binary as closely as possible.
+        .AddOptionalParameter("logtostderr", string("false"))
+        .Build();
+    builder.AddAction(std::move(run));
+  }
+  {
     unique_ptr<Action> set_flag =
         ActionBuilder("set_flag", &MasterSetFlag)
         .Description("Change a gflag value on a Kudu Master")
diff --git a/src/kudu/tools/tool_action_tserver.cc b/src/kudu/tools/tool_action_tserver.cc
index 337272a..a89a566 100644
--- a/src/kudu/tools/tool_action_tserver.cc
+++ b/src/kudu/tools/tool_action_tserver.cc
@@ -24,7 +24,7 @@
 
 #include <boost/algorithm/string/predicate.hpp>
 #include <gflags/gflags.h>
-#include <gflags/gflags_declare.h>
+#include <glog/logging.h>
 
 #include "kudu/common/common.pb.h"
 #include "kudu/common/wire_protocol.h"
@@ -38,6 +38,8 @@
 #include "kudu/tools/tool_action.h"
 #include "kudu/tools/tool_action_common.h"
 #include "kudu/tserver/tablet_server.h"
+#include "kudu/tserver/tablet_server_runner.h"
+#include "kudu/util/init.h"
 #include "kudu/util/status.h"
 
 DEFINE_bool(allow_missing_tserver, false, "If true, performs the action on the "
@@ -59,6 +61,7 @@ using master::ListTabletServersRequestPB;
 using master::ListTabletServersResponsePB;
 using master::MasterServiceProxy;
 using master::TServerStateChangePB;
+using tserver::TabletServer;
 
 namespace tools {
 namespace {
@@ -77,6 +80,18 @@ Status TServerGetFlags(const RunnerContext& context) {
   return PrintServerFlags(address, tserver::TabletServer::kDefaultPort);
 }
 
+Status TServerRun(const RunnerContext& context) {
+  RETURN_NOT_OK(InitKudu());
+
+  // Enable redaction by default. Unlike most tools, we don't want user data
+  // printed to the console/log to be shown by default.
+  CHECK_NE("", google::SetCommandLineOptionWithMode("redact",
+      "all", google::FlagSettingMode::SET_FLAGS_DEFAULT));
+
+  tserver::SetTabletServerFlagDefaults();
+  return tserver::RunTabletServer();
+}
+
 Status TServerSetFlag(const RunnerContext& context) {
   const string& address = FindOrDie(context.required_args, kTServerAddressArg);
   const string& flag = FindOrDie(context.required_args, kFlagArg);
@@ -229,6 +244,29 @@ unique_ptr<Mode> BuildTServerMode() {
       .AddOptionalParameter("flag_tags")
       .Build();
 
+  unique_ptr<Action> run =
+      ActionBuilder("run", &TServerRun)
+      .ProgramName("kudu-tserver")
+      .Description("Runs a Kudu Tablet Server")
+      .ExtraDescription("Note: The tablet server is started in this process and "
+                        "runs until interrupted.\n\n"
+                        "The most common configuration flags are described below. "
+                        "For all the configuration options pass --helpfull or see "
+                        "https://kudu.apache.org/docs/configuration_reference.html"
+                        "#kudu-tserver_supported")
+      .AddOptionalParameter("tserver_master_addrs")
+      // Even though fs_wal_dir is required, we don't want it to be positional argument.
+      .AddOptionalParameter("fs_wal_dir")
+      .AddOptionalParameter("fs_data_dirs")
+      .AddOptionalParameter("fs_metadata_dir")
+      .AddOptionalParameter("block_cache_capacity_mb")
+      .AddOptionalParameter("memory_limit_hard_bytes")
+      .AddOptionalParameter("log_dir")
+      // Unlike most tools we don't log to stderr by default to match the
+      // kudu-tserver binary as closely as possible.
+      .AddOptionalParameter("logtostderr", string("false"))
+      .Build();
+
   unique_ptr<Action> set_flag =
       ActionBuilder("set_flag", &TServerSetFlag)
       .Description("Change a gflag value on a Kudu Tablet Server")
@@ -291,6 +329,7 @@ unique_ptr<Mode> BuildTServerMode() {
       .Description("Operate on a Kudu Tablet Server")
       .AddAction(std::move(dump_memtrackers))
       .AddAction(std::move(get_flags))
+      .AddAction(std::move(run))
       .AddAction(std::move(set_flag))
       .AddAction(std::move(status))
       .AddAction(std::move(timestamp))
diff --git a/src/kudu/tools/tool_main.cc b/src/kudu/tools/tool_main.cc
index 55c5f26..edd190c 100644
--- a/src/kudu/tools/tool_main.cc
+++ b/src/kudu/tools/tool_main.cc
@@ -26,7 +26,6 @@
 
 #include <boost/optional/optional.hpp>
 #include <gflags/gflags.h>
-#include <gflags/gflags_declare.h>
 #include <glog/logging.h>
 
 #include "kudu/gutil/map-util.h"
@@ -34,7 +33,6 @@
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/tools/tool_action.h"
 #include "kudu/util/flags.h"
-#include "kudu/util/logging.h"
 #include "kudu/util/path_util.h"
 #include "kudu/util/status.h"
 
@@ -252,14 +250,16 @@ int main(int argc, char** argv) {
   CHECK_NE("",  google::SetCommandLineOptionWithMode(
       "redact", "", google::SET_FLAGS_DEFAULT));
 
+  // Set logtostderr by default given these are command line tools.
+  CHECK_NE("",  google::SetCommandLineOptionWithMode(
+      "logtostderr", "true", google::SET_FLAGS_DEFAULT));
+
   // Hide the regular gflags help unless --helpfull is used.
   //
   // Inspired by https://github.com/gflags/gflags/issues/43#issuecomment-168280647.
   gflags::ParseCommandLineNonHelpFlags(&argc, &argv, true);
 
-  FLAGS_logtostderr = true;
   const char* prog_name = argv[0];
-  kudu::InitGoogleLoggingSafe(prog_name);
   bool show_help = ParseCommandLineFlags(prog_name);
 
   return kudu::tools::RunTool(argc, argv, show_help);
diff --git a/src/kudu/tserver/CMakeLists.txt b/src/kudu/tserver/CMakeLists.txt
index bcee01a..ae74010 100644
--- a/src/kudu/tserver/CMakeLists.txt
+++ b/src/kudu/tserver/CMakeLists.txt
@@ -112,6 +112,7 @@ set(TSERVER_SRCS
   tablet_copy_source_session.cc
   tablet_server.cc
   tablet_server_options.cc
+  tablet_server_runner.cc
   tablet_service.cc
   ts_tablet_manager.cc
   tserver_path_handlers.cc
diff --git a/src/kudu/tserver/tablet_server.h b/src/kudu/tserver/tablet_server.h
index 5d802c8..bce7773 100644
--- a/src/kudu/tserver/tablet_server.h
+++ b/src/kudu/tserver/tablet_server.h
@@ -45,6 +45,7 @@ class TabletServer : public kserver::KuduServer {
   // this constant as well.
   static const uint16_t kDefaultPort = 7050;
   static const uint16_t kDefaultWebPort = 8050;
+  static const uint16_t kDefaultNumServiceThreads = 20;
 
   explicit TabletServer(const TabletServerOptions& opts);
   ~TabletServer();
diff --git a/src/kudu/tserver/tablet_server_main.cc b/src/kudu/tserver/tablet_server_main.cc
index c01770a..87de630 100644
--- a/src/kudu/tserver/tablet_server_main.cc
+++ b/src/kudu/tserver/tablet_server_main.cc
@@ -16,77 +16,28 @@
 // under the License.
 
 #include <iostream>
-#include <string>
 
-#include <gflags/gflags.h>
 #include <glog/logging.h>
 
-#include "kudu/gutil/macros.h"
-#include "kudu/gutil/strings/substitute.h"
-#include "kudu/tserver/tablet_server.h"
-#include "kudu/util/fault_injection.h"
-#include "kudu/util/flag_tags.h"
+#include "kudu/tserver/tablet_server_runner.h"
 #include "kudu/util/flags.h"
 #include "kudu/util/init.h"
 #include "kudu/util/logging.h"
-#include "kudu/util/monotime.h"
 #include "kudu/util/status.h"
-#include "kudu/util/version_info.h"
-
-using kudu::tserver::TabletServer;
-
-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. "
-              "Used to test the test infrastructure. Should never be set outside of tests.");
-TAG_FLAG(fault_before_start, hidden);
-TAG_FLAG(fault_before_start, unsafe);
 
 namespace kudu {
 namespace tserver {
 
 static int TabletServerMain(int argc, char** argv) {
   RETURN_MAIN_NOT_OK(InitKudu(), "InitKudu() failed", 1);
-
-  // Reset some default values before parsing gflags.
-  FLAGS_rpc_bind_addresses = strings::Substitute("0.0.0.0:$0",
-                                                 TabletServer::kDefaultPort);
-  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();
-
+  SetTabletServerFlagDefaults();
   ParseCommandLineFlags(&argc, &argv, true);
   if (argc != 1) {
     std::cerr << "usage: " << argv[0] << std::endl;
     return 2;
   }
-  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();
-
-  TabletServer server({});
-  RETURN_MAIN_NOT_OK(server.Init(), "Init() failed", 3);
-  MAYBE_FAULT(FLAGS_fault_before_start);
-  RETURN_MAIN_NOT_OK(server.Start(), "Start() failed", 4);
-
-  while (true) {
-    SleepFor(MonoDelta::FromSeconds(60));
-  }
-
+  RETURN_MAIN_NOT_OK(RunTabletServer(), "RunTabletServer() failed", 3);
   return 0;
 }
 
diff --git a/src/kudu/tserver/tablet_server_main.cc b/src/kudu/tserver/tablet_server_runner.cc
similarity index 67%
copy from src/kudu/tserver/tablet_server_main.cc
copy to src/kudu/tserver/tablet_server_runner.cc
index c01770a..feb7b80 100644
--- a/src/kudu/tserver/tablet_server_main.cc
+++ b/src/kudu/tserver/tablet_server_runner.cc
@@ -15,6 +15,8 @@
 // specific language governing permissions and limitations
 // under the License.
 
+#include "kudu/tserver/tablet_server_runner.h"
+
 #include <iostream>
 #include <string>
 
@@ -27,17 +29,11 @@
 #include "kudu/util/fault_injection.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"
-#include "kudu/util/status.h"
 #include "kudu/util/version_info.h"
 
 using kudu::tserver::TabletServer;
-
-DECLARE_int32(rpc_num_service_threads);
-DECLARE_int32(webserver_port);
-DECLARE_string(rpc_bind_addresses);
+using std::string;
 
 DEFINE_double(fault_before_start, 0.0,
               "Fake fault flag that always causes a crash on startup. "
@@ -48,51 +44,41 @@ TAG_FLAG(fault_before_start, unsafe);
 namespace kudu {
 namespace tserver {
 
-static int TabletServerMain(int argc, char** argv) {
-  RETURN_MAIN_NOT_OK(InitKudu(), "InitKudu() failed", 1);
-
+void SetTabletServerFlagDefaults() {
   // Reset some default values before parsing gflags.
-  FLAGS_rpc_bind_addresses = strings::Substitute("0.0.0.0:$0",
-                                                 TabletServer::kDefaultPort);
-  FLAGS_rpc_num_service_threads = 20;
-  FLAGS_webserver_port = TabletServer::kDefaultWebPort;
+  CHECK_NE("", google::SetCommandLineOptionWithMode("rpc_bind_addresses",
+      strings::Substitute("0.0.0.0:$0", TabletServer::kDefaultPort).c_str(),
+      google::FlagSettingMode::SET_FLAGS_DEFAULT));
+  CHECK_NE("", google::SetCommandLineOptionWithMode("rpc_num_service_threads",
+      std::to_string(TabletServer::kDefaultNumServiceThreads).c_str(),
+      google::FlagSettingMode::SET_FLAGS_DEFAULT));
+  CHECK_NE("", google::SetCommandLineOptionWithMode("webserver_port",
+      std::to_string(TabletServer::kDefaultWebPort).c_str(),
+      google::FlagSettingMode::SET_FLAGS_DEFAULT));
 
   // 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);
-  if (argc != 1) {
-    std::cerr << "usage: " << argv[0] << std::endl;
-    return 2;
-  }
-  std::string nondefault_flags = GetNonDefaultFlags(default_flags);
-  InitGoogleLoggingSafe(argv[0]);
+                                            "false", gflags::SET_FLAGS_DEFAULT));
+}
 
+Status RunTabletServer() {
+  string nondefault_flags = GetNonDefaultFlags();
   LOG(INFO) << "Tablet server non-default flags:\n"
             << nondefault_flags << '\n'
             << "Tablet server version:\n"
             << VersionInfo::GetAllVersionInfo();
 
   TabletServer server({});
-  RETURN_MAIN_NOT_OK(server.Init(), "Init() failed", 3);
+  RETURN_NOT_OK(server.Init());
   MAYBE_FAULT(FLAGS_fault_before_start);
-  RETURN_MAIN_NOT_OK(server.Start(), "Start() failed", 4);
+  RETURN_NOT_OK(server.Start());
 
   while (true) {
     SleepFor(MonoDelta::FromSeconds(60));
   }
-
-  return 0;
 }
 
 } // namespace tserver
 } // namespace kudu
-
-int main(int argc, char** argv) {
-  return kudu::tserver::TabletServerMain(argc, argv);
-}
diff --git a/src/kudu/tserver/tablet_server_runner.h b/src/kudu/tserver/tablet_server_runner.h
new file mode 100644
index 0000000..3b6264e
--- /dev/null
+++ b/src/kudu/tserver/tablet_server_runner.h
@@ -0,0 +1,31 @@
+// 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.
+#pragma once
+
+#include "kudu/util/status.h"
+
+namespace kudu {
+namespace tserver {
+
+// Adjusts the default values of some gflags for use with the tablet server.
+void SetTabletServerFlagDefaults();
+
+// Initialize and start a tablet server. Will run until interrupted.
+Status RunTabletServer();
+
+} // namespace tserver
+} // namespace kudu
diff --git a/src/kudu/util/flag_tags-test.cc b/src/kudu/util/flag_tags-test.cc
index 4626d0d..7b7c8a5 100644
--- a/src/kudu/util/flag_tags-test.cc
+++ b/src/kudu/util/flag_tags-test.cc
@@ -15,18 +15,18 @@
 // specific language governing permissions and limitations
 // under the License.
 
+#include "kudu/util/flag_tags.h"
+
 #include <string>
 #include <unordered_set>
 #include <vector>
 
-#include <gtest/gtest.h>
 #include <gflags/gflags.h>
-#include <gflags/gflags_declare.h>
+#include <gtest/gtest.h>
 
-#include "kudu/gutil/map-util.h"
 #include "kudu/gutil/macros.h"
+#include "kudu/gutil/map-util.h"
 #include "kudu/gutil/strings/substitute.h"
-#include "kudu/util/flag_tags.h"
 #include "kudu/util/flags.h"
 #include "kudu/util/logging.h"
 #include "kudu/util/logging_test_util.h"
@@ -83,7 +83,7 @@ TEST_F(FlagTagsTest, TestUnlockFlags) {
   {
     gflags::FlagSaver s;
     gflags::SetCommandLineOption("test_unsafe_flag", "true");
-    ASSERT_DEATH({ HandleCommonFlags(); },
+    ASSERT_DEATH({ ValidateFlags(); },
                  "Flag --test_unsafe_flag is unsafe and unsupported.*"
                  "Use --unlock_unsafe_flags to proceed");
   }
@@ -95,7 +95,7 @@ TEST_F(FlagTagsTest, TestUnlockFlags) {
     gflags::FlagSaver s;
     gflags::SetCommandLineOption("test_unsafe_flag", "true");
     gflags::SetCommandLineOption("unlock_unsafe_flags", "true");
-    HandleCommonFlags();
+    ValidateFlags();
     ASSERT_EQ(1, sink.logged_msgs().size());
     ASSERT_STR_CONTAINS(sink.logged_msgs()[0], "Enabled unsafe flag: --test_unsafe_flag");
   }
@@ -104,7 +104,7 @@ TEST_F(FlagTagsTest, TestUnlockFlags) {
   {
     gflags::FlagSaver s;
     gflags::SetCommandLineOption("test_experimental_flag", "true");
-    ASSERT_DEATH({ HandleCommonFlags(); },
+    ASSERT_DEATH({ ValidateFlags(); },
                  "Flag --test_experimental_flag is experimental and unsupported.*"
                  "Use --unlock_experimental_flags to proceed");
   }
@@ -116,7 +116,7 @@ TEST_F(FlagTagsTest, TestUnlockFlags) {
     gflags::FlagSaver s;
     gflags::SetCommandLineOption("test_experimental_flag", "true");
     gflags::SetCommandLineOption("unlock_experimental_flags", "true");
-    HandleCommonFlags();
+    ValidateFlags();
     ASSERT_EQ(1, sink.logged_msgs().size());
     ASSERT_STR_CONTAINS(sink.logged_msgs()[0],
                         "Enabled experimental flag: --test_experimental_flag");
diff --git a/src/kudu/util/flags-test.cc b/src/kudu/util/flags-test.cc
index 9ebc178..675ef6f 100644
--- a/src/kudu/util/flags-test.cc
+++ b/src/kudu/util/flags-test.cc
@@ -15,18 +15,18 @@
 // specific language governing permissions and limitations
 // under the License.
 
+#include "kudu/util/flags.h"
+
 #include <string>
 #include <vector>
 
 #include <gflags/gflags.h>
-#include <gflags/gflags_declare.h>
 #include <glog/logging.h>
 #include <gtest/gtest.h>
 
 #include "kudu/gutil/macros.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/util/env.h"
-#include "kudu/util/flags.h"
 #include "kudu/util/flag_tags.h"
 #include "kudu/util/logging.h"
 #include "kudu/util/slice.h"
@@ -53,9 +53,6 @@ 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";
@@ -92,7 +89,7 @@ TEST_F(FlagsTest, TestNonDefaultFlags) {
   // a redacted value.
   FLAGS_test_sensitive_flag = true;
   kudu::g_should_redact = kudu::RedactContext::LOG;
-  std::string result = GetNonDefaultFlags(default_flags);
+  std::string result = GetNonDefaultFlags();
 
   for (const auto& expected : expected_flags) {
     ASSERT_STR_CONTAINS(result, expected);
diff --git a/src/kudu/util/flags.cc b/src/kudu/util/flags.cc
index b966a44..b4eaf7e 100644
--- a/src/kudu/util/flags.cc
+++ b/src/kudu/util/flags.cc
@@ -17,21 +17,20 @@
 
 #include "kudu/util/flags.h"
 
+#include <sys/stat.h>
+#include <unistd.h> // IWYU pragma: keep
 
 #include <cstdlib>
 #include <functional>
 #include <iostream>
+#include <map>
 #include <string>
 #include <unordered_set>
 #include <utility>
 #include <vector>
 
-#include <sys/stat.h>
-#include <unistd.h> // IWYU pragma: keep
-
 #include <boost/algorithm/string/predicate.hpp>
 #include <gflags/gflags.h>
-#include <gflags/gflags_declare.h>
 #include <glog/logging.h>
 #ifdef TCMALLOC_ENABLED
 #include <gperftools/heap-profiler.h>
@@ -481,6 +480,7 @@ int ParseCommandLineFlags(int* argc, char*** argv, bool remove_flags) {
 
   int ret = google::ParseCommandLineNonHelpFlags(argc, argv, remove_flags);
   HandleCommonFlags();
+  ValidateFlags();
   return ret;
 }
 
@@ -497,8 +497,6 @@ void HandleCommonFlags() {
   }
 
   google::HandleCommandLineHelpFlags();
-  CheckFlagsAllowed();
-  RunCustomValidators();
 
   if (FLAGS_disable_core_dumps) {
     DisableCoreDumps();
@@ -527,6 +525,11 @@ void HandleCommonFlags() {
 #endif
 }
 
+void ValidateFlags() {
+  CheckFlagsAllowed();
+  RunCustomValidators();
+}
+
 string CommandlineFlagsIntoString(EscapeMode mode) {
   string ret_value;
   vector<CommandLineFlagInfo> flags;
@@ -546,20 +549,15 @@ string CommandlineFlagsIntoString(EscapeMode mode) {
   return ret_value;
 }
 
-string GetNonDefaultFlags(const GFlagsMap& 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. 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) {
+      // 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';
         }
diff --git a/src/kudu/util/flags.h b/src/kudu/util/flags.h
index 83cb152..033a57f 100644
--- a/src/kudu/util/flags.h
+++ b/src/kudu/util/flags.h
@@ -54,6 +54,11 @@ int ParseCommandLineFlags(int* argc, char*** argv, bool remove_flags);
 // google::ParseCommandLineNonHelpFlags().
 void HandleCommonFlags();
 
+// Verifies that the flags are allowed to be set and valid.
+// Should be called after logging is initialized. Otherwise
+// logging will write to stderr.
+void ValidateFlags();
+
 enum class EscapeMode {
   HTML,
   NONE
@@ -70,7 +75,7 @@ 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. Redact any flags that
 // are tagged as sensitive, if redaction is enabled.
-std::string GetNonDefaultFlags(const GFlagsMap& default_flags);
+std::string GetNonDefaultFlags();
 
 GFlagsMap GetFlagsMap();
 
diff --git a/src/kudu/util/logging.cc b/src/kudu/util/logging.cc
index cfda466..36850c3 100644
--- a/src/kudu/util/logging.cc
+++ b/src/kudu/util/logging.cc
@@ -14,6 +14,7 @@
 // KIND, either express or implied.  See the License for the
 // specific language governing permissions and limitations
 // under the License.
+
 #include "kudu/util/logging.h"
 
 #include <unistd.h>
@@ -23,10 +24,12 @@
 #include <cstdlib>
 #include <ctime>
 #include <fstream>
+#include <initializer_list>
 #include <mutex>
 #include <utility>
 
 #include <boost/uuid/random_generator.hpp>
+#include <boost/uuid/uuid.hpp>
 #include <boost/uuid/uuid_io.hpp>
 #include <gflags/gflags.h>
 #include <glog/logging.h>
@@ -217,13 +220,12 @@ void InitGoogleLoggingSafe(const char* arg) {
     }
   }
 
-  // This forces our logging to use /tmp rather than looking for a
+  // This forces our logging to default to /tmp rather than looking for a
   // temporary directory if none is specified. This is done so that we
   // can reliably construct the log file name without duplicating the
   // complex logic that glog uses to guess at a temporary dir.
-  if (FLAGS_log_dir.empty()) {
-    FLAGS_log_dir = "/tmp";
-  }
+  CHECK_NE("", google::SetCommandLineOptionWithMode("log_dir",
+     "/tmp", google::FlagSettingMode::SET_FLAGS_DEFAULT));
 
   if (!FLAGS_logtostderr) {
     // Verify that a log file can be created in log_dir by creating a tmp file.
diff --git a/src/kudu/util/logging.h b/src/kudu/util/logging.h
index 0d3492a..f8b03b5 100644
--- a/src/kudu/util/logging.h
+++ b/src/kudu/util/logging.h
@@ -25,6 +25,7 @@
 #include "kudu/gutil/atomicops.h"
 #include "kudu/gutil/dynamic_annotations.h"
 #include "kudu/gutil/macros.h"
+#include "kudu/gutil/port.h"
 #include "kudu/gutil/walltime.h"
 #include "kudu/util/logging_callback.h"
 #include "kudu/util/status.h"
diff --git a/src/kudu/util/minidump.cc b/src/kudu/util/minidump.cc
index d02dc5d..9a301f4 100644
--- a/src/kudu/util/minidump.cc
+++ b/src/kudu/util/minidump.cc
@@ -57,6 +57,7 @@ static constexpr bool kMinidumpPlatformSupported = false;
 #endif // defined(__linux__)
 
 DECLARE_string(log_dir);
+DECLARE_string(log_filename);
 
 DEFINE_bool(enable_minidumps, kMinidumpPlatformSupported,
             "Whether to enable minidump generation upon process crash or SIGUSR1. "
@@ -208,11 +209,14 @@ Status MinidumpExceptionHandler::InitMinidumpExceptionHandler() {
   RETURN_NOT_OK_PREPEND(CreateDirIfMissing(env, minidump_dir_),
                         "Error creating top-level minidump directory");
 
-  // Add the executable name to the path where minidumps will be written. This makes
-  // identification easier and prevents name collisions between the files.
+  // Add the program_name to the path where minidumps will be written.
+  // This makes identification easier and prevents name collisions between the files.
   // This is also consistent with how Impala organizes its minidump files.
-  const char* exe_name = gflags::ProgramInvocationShortName();
-  minidump_dir_ = JoinPathSegments(minidump_dir_, exe_name);
+  // The log_filename flag will be used if non-empty, otherwise the executable name
+  // will be used.
+  const char* program_name = FLAGS_log_filename.empty() ? gflags::ProgramInvocationShortName() :
+          FLAGS_log_filename.c_str();
+  minidump_dir_ = JoinPathSegments(minidump_dir_, program_name);
 
   // Create the directory if it is not there. The minidump doesn't get written if there is
   // no directory.
diff --git a/src/kudu/util/rolling_log-test.cc b/src/kudu/util/rolling_log-test.cc
index f4f8186..9327ac7 100644
--- a/src/kudu/util/rolling_log-test.cc
+++ b/src/kudu/util/rolling_log-test.cc
@@ -78,7 +78,7 @@ class RollingLogTest : public KuduTest {
 
 // Test with compression off.
 TEST_F(RollingLogTest, TestLog) {
-  RollingLog log(env_, log_dir_, "mylog");
+  RollingLog log(env_, log_dir_, "rolling_log-test", "mylog");
   log.SetCompressionEnabled(false);
   log.SetRollThresholdBytes(100);
 
@@ -107,7 +107,7 @@ TEST_F(RollingLogTest, TestLog) {
 
 // Test with compression on.
 TEST_F(RollingLogTest, TestCompression) {
-  RollingLog log(env_, log_dir_, "mylog");
+  RollingLog log(env_, log_dir_, "rolling_log-test", "mylog");
   ASSERT_OK(log.Open());
 
   StringPiece data = "Hello world\n";
@@ -130,7 +130,7 @@ TEST_F(RollingLogTest, TestCompression) {
 }
 
 TEST_F(RollingLogTest, TestFileCountLimit) {
-  RollingLog log(env_, log_dir_, "mylog");
+  RollingLog log(env_, log_dir_, "rolling_log-test", "mylog");
   ASSERT_OK(log.Open());
   log.SetRollThresholdBytes(100);
   log.SetMaxNumSegments(3);
diff --git a/src/kudu/util/rolling_log.cc b/src/kudu/util/rolling_log.cc
index 610aa9e..9f35e0b 100644
--- a/src/kudu/util/rolling_log.cc
+++ b/src/kudu/util/rolling_log.cc
@@ -24,10 +24,10 @@
 #include <memory>
 #include <ostream>
 #include <string>
+#include <type_traits>
 #include <utility>
 
 #include <gflags/gflags.h>
-#include <gflags/gflags_declare.h>
 #include <glog/logging.h>
 #include <zlib.h>
 
@@ -53,9 +53,11 @@ DECLARE_int32(max_log_files);
 
 namespace kudu {
 
-RollingLog::RollingLog(Env* env, string log_dir, string log_name)
+RollingLog::RollingLog(Env* env, string log_dir, string program_name, string log_name)
     : env_(env),
       log_dir_(std::move(log_dir)),
+      program_name_(program_name.empty() ?
+        google::ProgramInvocationShortName() : std::move(program_name)),
       log_name_(std::move(log_name)),
       roll_threshold_bytes_(kDefaultRollThresholdBytes),
       max_num_segments_(FLAGS_max_log_files),
@@ -121,7 +123,7 @@ string FormattedTimestamp() {
 
 string RollingLog::GetLogFileName(int sequence) const {
   return Substitute("$0.$1.$2.$3.$4.$5.$6",
-                    google::ProgramInvocationShortName(),
+                    program_name_,
                     HostnameOrUnknown(),
                     UsernameOrUnknown(),
                     log_name_,
@@ -132,7 +134,7 @@ string RollingLog::GetLogFileName(int sequence) const {
 
 string RollingLog::GetLogFilePattern() const {
   return Substitute("$0.$1.$2.$3.$4.$5.$6",
-                    google::ProgramInvocationShortName(),
+                    program_name_,
                     HostnameOrUnknown(),
                     UsernameOrUnknown(),
                     log_name_,
diff --git a/src/kudu/util/rolling_log.h b/src/kudu/util/rolling_log.h
index 0bb6755..17ca8e9 100644
--- a/src/kudu/util/rolling_log.h
+++ b/src/kudu/util/rolling_log.h
@@ -41,7 +41,8 @@ class WritableFile;
 //
 // <log_dir>/<program-name>.<hostname>.<user-name>.<log-name>.<timestamp>.<sequence>.<pid>
 //   log_dir:      the log_dir specified in the constructor
-//   program-name: argv[0], as determined by google::ProgramInvocationShortName()
+//   program-name: the name of the program specified in the constructor. If unset, defaults
+//                 to argv[0], as determined by google::ProgramInvocationShortName().
 //   hostname:     the local machine hostname
 //   user-name:    the current user name
 //   log-name:     the log_name specified in the constructor
@@ -55,7 +56,7 @@ class WritableFile;
 // This class is not thread-safe and must be externally synchronized.
 class RollingLog {
  public:
-  RollingLog(Env* env, std::string log_dir, std::string log_name);
+  RollingLog(Env* env, std::string log_dir, std::string program_name, std::string log_name);
 
   ~RollingLog();
 
@@ -111,6 +112,7 @@ class RollingLog {
 
   Env* const env_;
   const std::string log_dir_;
+  const std::string program_name_;
   const std::string log_name_;
 
   int64_t roll_threshold_bytes_;


Mime
View raw message