You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by lv...@apache.org on 2019/08/28 00:51:13 UTC

[impala] branch master updated: IMPALA-8895: Expose daemon health endpoint

This is an automated email from the ASF dual-hosted git repository.

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


The following commit(s) were added to refs/heads/master by this push:
     new 0df613f  IMPALA-8895: Expose daemon health endpoint
0df613f is described below

commit 0df613f5e3ece63ed21fe84db6044d06546e900d
Author: Lars Volker <lv...@cloudera.com>
AuthorDate: Mon Aug 26 19:27:23 2019 -0700

    IMPALA-8895: Expose daemon health endpoint
    
    This change exposes the daemon health of an impalad via an HTTP endpoint
    '/healthz'. If the server is healthy, this endpoint will return HTTP
    code 200 (OK). If it is unhealthy, it will return 503 (Service
    Unavailable). With this change, the endpoint is equivalent to the
    'impala-server.ready' metric.
    
    Testing: I added a test in test_web_pages.py and observed manually that
    the endpoint returns 503 while starting up.
    
    Change-Id: I05889e21bbb1b97b23fad9928b9e61e5dbff2615
    Reviewed-on: http://gerrit.cloudera.org:8080/14146
    Reviewed-by: Lars Volker <lv...@cloudera.com>
    Tested-by: Lars Volker <lv...@cloudera.com>
---
 be/src/service/impala-http-handler.cc | 17 +++++++++++++++++
 be/src/service/impala-http-handler.h  |  8 ++++++++
 be/src/service/impala-server.cc       |  3 +++
 be/src/service/impala-server.h        |  3 +++
 be/src/util/metrics.cc                |  4 ++--
 be/src/util/metrics.h                 |  6 +++++-
 be/src/util/webserver-test.cc         |  3 ++-
 be/src/util/webserver.cc              |  2 +-
 be/src/util/webserver.h               |  5 +++--
 tests/webserver/test_web_pages.py     |  5 +++++
 10 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/be/src/service/impala-http-handler.cc b/be/src/service/impala-http-handler.cc
index 22bd189..3dee11b 100644
--- a/be/src/service/impala-http-handler.cc
+++ b/be/src/service/impala-http-handler.cc
@@ -90,6 +90,12 @@ static Status ParseIdFromRequest(const Webserver::WebRequest& req, TUniqueId* id
 void ImpalaHttpHandler::RegisterHandlers(Webserver* webserver) {
   DCHECK(webserver != NULL);
 
+  Webserver::RawUrlCallback healthz_callback =
+    [this](const auto& req, auto* data, auto* response) {
+      return this->HealthzHandler(req, data, response);
+    };
+  webserver->RegisterUrlCallback("/healthz", healthz_callback);
+
   webserver->RegisterUrlCallback("/backends", "backends.tmpl",
       MakeCallback(this, &ImpalaHttpHandler::BackendsHandler), true);
 
@@ -167,6 +173,17 @@ void ImpalaHttpHandler::RegisterHandlers(Webserver* webserver) {
   RegisterLogLevelCallbacks(webserver, true);
 }
 
+void ImpalaHttpHandler::HealthzHandler(const Webserver::WebRequest& req,
+    std::stringstream* data, HttpStatusCode* response) {
+  if (server_->IsHealthy()) {
+    (*data) << "OK";
+    *response = HttpStatusCode::Ok;
+    return;
+  }
+  *(data) << "Not Available";
+  *response = HttpStatusCode::ServiceUnavailable;
+}
+
 void ImpalaHttpHandler::HadoopVarzHandler(const Webserver::WebRequest& req,
     Document* document) {
   TGetAllHadoopConfigsResponse response;
diff --git a/be/src/service/impala-http-handler.h b/be/src/service/impala-http-handler.h
index e34ad68..380fb7d 100644
--- a/be/src/service/impala-http-handler.h
+++ b/be/src/service/impala-http-handler.h
@@ -18,11 +18,15 @@
 #ifndef IMPALA_SERVICE_IMPALA_HTTP_HANDLER_H
 #define IMPALA_SERVICE_IMPALA_HTTP_HANDLER_H
 
+#include <sstream>
 #include <rapidjson/document.h>
+#include "kudu/util/web_callback_registry.h"
 #include "util/webserver.h"
 
 #include "service/impala-server.h"
 
+using kudu::HttpStatusCode;
+
 namespace impala {
 
 /// Handles all webserver callbacks for an ImpalaServer. This class is a friend of
@@ -38,6 +42,10 @@ class ImpalaHttpHandler {
  private:
   ImpalaServer* server_;
 
+  /// Raw callback to indicate whether the server is ready to accept queries.
+  void HealthzHandler(const Webserver::WebRequest& req, std::stringstream* data,
+      HttpStatusCode* response);
+
   /// Json callback for /hadoop-varz. Produces Json with a list, 'configs', of (key,
   /// value) pairs, one for each Hadoop configuration value.
   void HadoopVarzHandler(const Webserver::WebRequest& req,
diff --git a/be/src/service/impala-server.cc b/be/src/service/impala-server.cc
index b878dbb..b3083ae 100644
--- a/be/src/service/impala-server.cc
+++ b/be/src/service/impala-server.cc
@@ -594,8 +594,11 @@ Status ImpalaServer::LogLineageRecord(const ClientRequestState& client_request_s
 }
 
 bool ImpalaServer::IsCoordinator() { return is_coordinator_; }
+
 bool ImpalaServer::IsExecutor() { return is_executor_; }
 
+bool ImpalaServer::IsHealthy() { return services_started_.load(); }
+
 int ImpalaServer::GetThriftBackendPort() {
   DCHECK(thrift_be_server_ != nullptr);
   return thrift_be_server_->port();
diff --git a/be/src/service/impala-server.h b/be/src/service/impala-server.h
index 4717225..684a18c 100644
--- a/be/src/service/impala-server.h
+++ b/be/src/service/impala-server.h
@@ -424,6 +424,9 @@ class ImpalaServer : public ImpalaServiceIf,
   /// Returns true if this is an executor, false otherwise.
   bool IsExecutor();
 
+  /// Returns whether this backend is healthy, i.e. able to accept queries.
+  bool IsHealthy();
+
   /// Returns the port that the thrift backend server is listening on. Valid to call after
   /// the server has started successfully.
   int GetThriftBackendPort();
diff --git a/be/src/util/metrics.cc b/be/src/util/metrics.cc
index d33aef8..e91c5c8 100644
--- a/be/src/util/metrics.cc
+++ b/be/src/util/metrics.cc
@@ -149,7 +149,7 @@ Status MetricGroup::Init(Webserver* webserver) {
     webserver->RegisterUrlCallback("/metrics", "metrics.tmpl", json_callback, true);
 
     Webserver::RawUrlCallback prometheus_callback =
-        bind<void>(mem_fn(&MetricGroup::PrometheusCallback), this, _1, _2);
+        bind<void>(mem_fn(&MetricGroup::PrometheusCallback), this, _1, _2, _3);
     webserver->RegisterUrlCallback("/metrics_prometheus", prometheus_callback);
   }
 
@@ -233,7 +233,7 @@ void MetricGroup::TemplateCallback(const Webserver::WebRequest& req,
 }
 
 void MetricGroup::PrometheusCallback(
-    const Webserver::WebRequest& req, stringstream* data) {
+    const Webserver::WebRequest& req, stringstream* data, HttpStatusCode* response) {
   const auto& args = req.parsed_args;
   Webserver::ArgumentMap::const_iterator metric_group = args.find("metric_group");
 
diff --git a/be/src/util/metrics.h b/be/src/util/metrics.h
index e0ec6c4..68f213f 100644
--- a/be/src/util/metrics.h
+++ b/be/src/util/metrics.h
@@ -31,11 +31,14 @@
 #include "common/logging.h"
 #include "common/object-pool.h"
 #include "common/status.h"
+#include "kudu/util/web_callback_registry.h"
 #include "util/debug-util.h"
 #include "util/metrics-fwd.h"
 #include "util/spinlock.h"
 #include "util/webserver.h"
 
+using kudu::HttpStatusCode;
+
 namespace impala {
 
 /// Singleton that provides metric definitions. Metrics are defined in metrics.json
@@ -457,7 +460,8 @@ class MetricGroup {
   /// each representing metric group, and each including a list of metrics, and a list
   /// of immediate children.  If args contains a paramater 'metric', only the json for
   /// that metric is returned.
-  void PrometheusCallback(const Webserver::WebRequest& req, std::stringstream* data);
+  void PrometheusCallback(const Webserver::WebRequest& req, std::stringstream* data,
+      HttpStatusCode* response);
 
   /// Legacy webpage callback for CM 5.0 and earlier. Produces a flattened map of (key,
   /// value) pairs for all metrics in this hierarchy.
diff --git a/be/src/util/webserver-test.cc b/be/src/util/webserver-test.cc
index f56bc68..3b1c61a 100644
--- a/be/src/util/webserver-test.cc
+++ b/be/src/util/webserver-test.cc
@@ -422,7 +422,8 @@ TEST(Webserver, FrameAllowEmbeddingTest) {
 
 const string STRING_WITH_NULL = "123456789\0ABCDE";
 
-void NullCharCallback(const Webserver::WebRequest& req, stringstream* out) {
+void NullCharCallback(const Webserver::WebRequest& req, stringstream* out,
+    kudu::HttpStatusCode* response) {
   (*out) << STRING_WITH_NULL;
 }
 
diff --git a/be/src/util/webserver.cc b/be/src/util/webserver.cc
index 559346b..d5f299a 100644
--- a/be/src/util/webserver.cc
+++ b/be/src/util/webserver.cc
@@ -557,7 +557,7 @@ sq_callback_result_t Webserver::BeginRequestCallback(struct sq_connection* conne
   stringstream output;
   if (!url_handler->use_templates()) {
     content_type = PLAIN;
-    url_handler->raw_callback()(req, &output);
+    url_handler->raw_callback()(req, &output, &response);
   } else {
     RenderUrlWithTemplate(req, *url_handler, &output, &content_type);
   }
diff --git a/be/src/util/webserver.h b/be/src/util/webserver.h
index 7d36894..e062e26 100644
--- a/be/src/util/webserver.h
+++ b/be/src/util/webserver.h
@@ -49,11 +49,12 @@ class Webserver {
  public:
   using ArgumentMap = kudu::WebCallbackRegistry::ArgumentMap;
   using WebRequest = kudu::WebCallbackRegistry::WebRequest;
+  using HttpStatusCode = kudu::HttpStatusCode;
 
   typedef boost::function<void (const WebRequest& req, rapidjson::Document* json)>
       UrlCallback;
-  typedef boost::function<void (const WebRequest& req, std::stringstream* output)>
-      RawUrlCallback;
+  typedef boost::function<void (const WebRequest& req, std::stringstream* output,
+      HttpStatusCode* response)> RawUrlCallback;
 
   /// Any callback may add a member to their Json output with key ENABLE_RAW_HTML_KEY;
   /// this causes the result of the template rendering process to be sent to the browser
diff --git a/tests/webserver/test_web_pages.py b/tests/webserver/test_web_pages.py
index 8deda42..df12ad4 100644
--- a/tests/webserver/test_web_pages.py
+++ b/tests/webserver/test_web_pages.py
@@ -650,3 +650,8 @@ class TestWebPage(ImpalaTestSuite):
     assert len(resp) == 3
     # check if metric shows up
     assert 'impala_statestore_subscriber_heartbeat_interval_time_min' in resp[0].text
+
+  def test_healthz_endpoint(self):
+    """Test to check that the /healthz endpoint returns 200 OK."""
+    page = requests.get("http://localhost:25000/healthz")
+    assert page.status_code == requests.codes.ok