kudu git commit: [logging] fix logging init if linking kudu client library

[al...@apache.org]

Repository: kudu
Updated Branches:
  refs/heads/master 825b964fa -> 2ff946030


[logging] fix logging init if linking kudu client library

Don't call constructor-like InitializeBasicLogging() function in
non-exported builds of the kudu_client library.  The rationale
is simple: the non-exported kudu_client library is used in Kudu tools,
benchmarks, and tests.  In tools and benchmarks, InitGoogleLoggingSafe()
is called to initialize the logging as needed.  In tests, the gtest
initializes glog on itself.

Prior to this fix, the kudu CLI tool could exit on SIGPIPE when
master/tserver abruptly closed connection.  That's because the call
to InitGoogleLoggingSafe() in the main() function of the CLI tool
didn't do anything once InitGoogleLoggingSafeBasic() has been
inadvertently called by the linked kudu_client library.

Change-Id: I6dcd8d7ccb8a9cafa79674273b95c61eee5baf5a
Reviewed-on: http://gerrit.cloudera.org:8080/10956
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/2ff94603
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/2ff94603
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/2ff94603

Branch: refs/heads/master
Commit: 2ff946030113860447931a49cfe69e8469efe5cc
Parents: 825b964
Author: Alexey Serbin <as...@cloudera.com>
Authored: Mon Jul 16 15:59:24 2018 -0700
Committer: Alexey Serbin <as...@cloudera.com>
Committed: Tue Jul 17 23:00:07 2018 +0000

----------------------------------------------------------------------
 src/kudu/client/client.cc   | 12 ++++++++----
 src/kudu/tools/tool_main.cc | 21 +++++++++++----------
 2 files changed, 19 insertions(+), 14 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/2ff94603/src/kudu/client/client.cc
----------------------------------------------------------------------
diff --git a/src/kudu/client/client.cc b/src/kudu/client/client.cc
index 912e71a..b2b4f1b 100644
--- a/src/kudu/client/client.cc
+++ b/src/kudu/client/client.cc
@@ -152,23 +152,27 @@ class ResourceMetrics;
 using internal::MetaCache;
 using sp::shared_ptr;
 
-static const char* kProgName = "kudu_client";
 const char* kVerboseEnvVar = "KUDU_CLIENT_VERBOSE";
 
+#if defined(kudu_client_exported_EXPORTS)
+static const char* kProgName = "kudu_client";
+
 // We need to reroute all logging to stderr when the client library is
 // loaded. GoogleOnceInit() can do that, but there are multiple entry
 // points into the client code, and it'd need to be called in each one.
 // So instead, let's use a constructor function.
 //
-// Should this be restricted to just the exported client build? Probably
-// not, as any application using the library probably wants stderr logging
-// more than file logging.
+// This is restricted to the exported client builds only. In case of linking
+// with non-exported kudu client library, logging must be initialized
+// from the main() function of the corresponding binary: usually, that's done
+// by calling InitGoogleLoggingSafe(argv[0]).
 __attribute__((constructor))
 static void InitializeBasicLogging() {
   InitGoogleLoggingSafeBasic(kProgName);
 
   SetVerboseLevelFromEnvVar();
 }
+#endif
 
 // Set Client logging verbose level from environment variable.
 void SetVerboseLevelFromEnvVar() {

http://git-wip-us.apache.org/repos/asf/kudu/blob/2ff94603/src/kudu/tools/tool_main.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tools/tool_main.cc b/src/kudu/tools/tool_main.cc
index f5e0d08..55c5f26 100644
--- a/src/kudu/tools/tool_main.cc
+++ b/src/kudu/tools/tool_main.cc
@@ -222,19 +222,14 @@ int RunTool(int argc, char** argv, bool show_help) {
 } // namespace tools
 } // namespace kudu
 
-static bool ParseCommandLineFlags(int* argc, char*** argv) {
-  // Hide the regular gflags help unless --helpfull is used.
-  //
-  // Inspired by https://github.com/gflags/gflags/issues/43#issuecomment-168280647.
-  bool show_help = false;
-  gflags::ParseCommandLineNonHelpFlags(argc, argv, true);
-
+static bool ParseCommandLineFlags(const char* prog_name) {
   // Leverage existing helpxml flag to print mode/action xml.
   if (FLAGS_helpxml) {
-    kudu::tools::DumpToolXML(*argv[0]);
+    kudu::tools::DumpToolXML(prog_name);
     exit(1);
   }
 
+  bool show_help = false;
   if (FLAGS_help ||
       FLAGS_helpshort ||
       !FLAGS_helpon.empty() ||
@@ -257,9 +252,15 @@ int main(int argc, char** argv) {
   CHECK_NE("",  google::SetCommandLineOptionWithMode(
       "redact", "", 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;
-  bool show_help = ParseCommandLineFlags(&argc, &argv);
+  const char* prog_name = argv[0];
+  kudu::InitGoogleLoggingSafe(prog_name);
+  bool show_help = ParseCommandLineFlags(prog_name);
 
-  kudu::InitGoogleLoggingSafe(argv[0]);
   return kudu::tools::RunTool(argc, argv, show_help);
 }