kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ale...@apache.org
Subject kudu git commit: KUDU-2005: actionable error messages from webserver
Date Thu, 11 May 2017 20:30:39 GMT
Repository: kudu
Updated Branches:
  refs/heads/branch-1.3.x ea87fdd8e -> 1767cb01c


KUDU-2005: actionable error messages from webserver

As it turned out, squeasel outputs only errors via the log_message
callback.  Let's output them with ERROR severity into Kudu log
(they were output as INFO messages prior to this patch).

Also, if the embedded squeasel webserver fails to start, add the last
error message from it (if any) into the RuntimeError status message
returned from Webserver::Start().

Change-Id: Ia2315f7ee88c8835a36e5174cb25132967429a77
Reviewed-on: http://gerrit.cloudera.org:8080/6848
Tested-by: Kudu Jenkins
Reviewed-by: Will Berkeley <wdberkeley@gmail.com>
(cherry picked from commit 83885ffffa47a77a7206414987d07d5945525e66)
Reviewed-on: http://gerrit.cloudera.org:8080/6861


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

Branch: refs/heads/branch-1.3.x
Commit: 1767cb01c9b83549c31320f4665fba0a60d65795
Parents: ea87fdd
Author: Alexey Serbin <aserbin@cloudera.com>
Authored: Wed May 10 15:51:28 2017 -0700
Committer: Will Berkeley <wdberkeley@gmail.com>
Committed: Thu May 11 20:09:40 2017 +0000

----------------------------------------------------------------------
 src/kudu/server/webserver-test.cc |  2 +-
 src/kudu/server/webserver.cc      | 27 +++++++++++++++++++++++----
 src/kudu/server/webserver.h       |  2 +-
 3 files changed, 25 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/1767cb01/src/kudu/server/webserver-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/server/webserver-test.cc b/src/kudu/server/webserver-test.cc
index 6142319..a93fe9c 100644
--- a/src/kudu/server/webserver-test.cc
+++ b/src/kudu/server/webserver-test.cc
@@ -248,7 +248,7 @@ class WebserverNegativeTests : public KuduTest {
     func(&opts);
     Webserver server(opts);
     Status s = server.Start();
-    ASSERT_FALSE(s.ok());
+    ASSERT_FALSE(s.ok()) << s.ToString();
   }
 };
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/1767cb01/src/kudu/server/webserver.cc
----------------------------------------------------------------------
diff --git a/src/kudu/server/webserver.cc b/src/kudu/server/webserver.cc
index 743eee8..f7a3757 100644
--- a/src/kudu/server/webserver.cc
+++ b/src/kudu/server/webserver.cc
@@ -40,6 +40,7 @@
 #include "kudu/gutil/strings/split.h"
 #include "kudu/gutil/strings/stringpiece.h"
 #include "kudu/gutil/strings/strip.h"
+#include "kudu/gutil/strings/substitute.h"
 #include "kudu/security/openssl_util.h"
 #include "kudu/util/env.h"
 #include "kudu/util/flag_tags.h"
@@ -57,6 +58,7 @@ using std::make_pair;
 using std::ostringstream;
 using std::string;
 using std::vector;
+using strings::Substitute;
 
 DEFINE_int32(webserver_max_post_length_bytes, 1024 * 1024,
              "The maximum length of a POST request that will be accepted by "
@@ -69,6 +71,12 @@ DEFINE_string(webserver_x_frame_options, "DENY",
               "to all responses. This can help prevent clickjacking attacks.");
 TAG_FLAG(webserver_x_frame_options, advanced);
 
+
+namespace {
+  // Last error message from the webserver.
+  string kWebserverLastErrMsg;
+}  // anonymous namespace
+
 namespace kudu {
 
 Webserver::Webserver(const WebserverOptions& opts)
@@ -233,12 +241,14 @@ Status Webserver::Start() {
   signal(SIGCHLD, sig_chld);
 
   if (context_ == nullptr) {
-    ostringstream error_msg;
-    error_msg << "Webserver: Could not start on address " << http_address_;
     Sockaddr addr;
     addr.set_port(opts_.port);
     TryRunLsof(addr);
-    return Status::NetworkError(error_msg.str());
+    string err_msg = Substitute("Webserver: could not start on address $0", http_address_);
+    if (!kWebserverLastErrMsg.empty()) {
+      err_msg = Substitute("$0: $1", err_msg, kWebserverLastErrMsg);
+    }
+    return Status::RuntimeError(err_msg);
   }
 
   PathHandlerCallback default_callback =
@@ -294,7 +304,16 @@ Status Webserver::GetBoundAddresses(std::vector<Sockaddr>* addrs)
const {
 int Webserver::LogMessageCallbackStatic(const struct sq_connection* connection,
                                         const char* message) {
   if (message != nullptr) {
-    LOG(INFO) << "Webserver: " << message;
+    // Using the ERROR severity for squeasel messages: as per source code at
+    // https://github.com/cloudera/squeasel/blob/\
+    //     c304d3f3481b07bf153979155f02e0aab24d01de/squeasel.c#L392
+    // the squeasel server uses the log callback to report on errors.
+    {
+      static simple_spinlock kErrMsgLock_;
+      std::unique_lock<simple_spinlock> l(kErrMsgLock_);
+      kWebserverLastErrMsg = message;
+    }
+    LOG(ERROR) << "Webserver: " << message;
     return 1;
   }
   return 0;

http://git-wip-us.apache.org/repos/asf/kudu/blob/1767cb01/src/kudu/server/webserver.h
----------------------------------------------------------------------
diff --git a/src/kudu/server/webserver.h b/src/kudu/server/webserver.h
index 2ef4646..aea89ad 100644
--- a/src/kudu/server/webserver.h
+++ b/src/kudu/server/webserver.h
@@ -46,7 +46,7 @@ class Webserver : public WebCallbackRegistry {
 
   // Starts a webserver on the port passed to the constructor. The webserver runs in a
   // separate thread, so this call is non-blocking.
-  Status Start();
+  Status Start() WARN_UNUSED_RESULT;
 
   // Stops the webserver synchronously.
   void Stop();


Mime
View raw message