From 471a0f54ad517e9ed4aca5ba4ddd7e4be7efd7a8 Mon Sep 17 00:00:00 2001 From: Akira Ajisaka Date: Thu, 16 Jul 2020 16:53:27 +0900 Subject: [PATCH 01/16] [WIP] HADOOP-17133. Implement HttpServer2 metrics --- .../fs/CommonConfigurationKeysPublic.java | 10 ++ .../org/apache/hadoop/http/HttpServer2.java | 9 ++ .../hadoop/http/HttpServer2Metrics.java | 122 ++++++++++++++++++ 3 files changed, 141 insertions(+) create mode 100644 hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeysPublic.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeysPublic.java index 3b31449decb5c..924be77e2f760 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeysPublic.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeysPublic.java @@ -949,6 +949,16 @@ public class CommonConfigurationKeysPublic { /** Defalt value for HADOOP_HTTP_LOGS_ENABLED */ public static final boolean HADOOP_HTTP_LOGS_ENABLED_DEFAULT = true; + /** + * @see + * + * core-default.xml + */ + public static final String HADOOP_HTTP_METRICS_ENABLED = + "hadoop.http.metrics.enabled"; + /** Defalt value for HADOOP_HTTP_METRIC_ENABLED */ + public static final boolean HADOOP_HTTP_METRICS_ENABLED_DEFAULT = false; + /** * @see * diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java index cdc2a74133af2..307dc01690169 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java @@ -93,6 +93,7 @@ import org.eclipse.jetty.server.handler.ContextHandlerCollection; import org.eclipse.jetty.server.handler.HandlerCollection; import org.eclipse.jetty.server.handler.RequestLogHandler; +import org.eclipse.jetty.server.handler.StatisticsHandler; import org.eclipse.jetty.server.session.SessionHandler; import org.eclipse.jetty.servlet.FilterHolder; import org.eclipse.jetty.servlet.FilterMapping; @@ -665,6 +666,14 @@ private void initializeWebServer(String name, String hostName, handlers.addHandler(requestLogHandler); } handlers.addHandler(webAppContext); + + if (conf.getBoolean(CommonConfigurationKeysPublic.HADOOP_HTTP_METRICS_ENABLED, + CommonConfigurationKeysPublic.HADOOP_HTTP_METRICS_ENABLED_DEFAULT)) { + StatisticsHandler stats = new StatisticsHandler(); + handlers.addHandler(stats); + HttpServer2Metrics.create(stats); + } + final String appDir = getWebAppsPath(name); addDefaultApps(contexts, appDir, conf); webServer.setHandler(handlers); diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java new file mode 100644 index 0000000000000..9eef38337d91e --- /dev/null +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java @@ -0,0 +1,122 @@ +/** + * 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. + */ +package org.apache.hadoop.http; + +import org.apache.hadoop.classification.InterfaceAudience; +import org.apache.hadoop.classification.InterfaceStability; +import org.apache.hadoop.metrics2.MetricsSystem; +import org.apache.hadoop.metrics2.annotation.Metric; +import org.apache.hadoop.metrics2.annotation.Metrics; +import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem; +import org.eclipse.jetty.server.handler.StatisticsHandler; + +@InterfaceAudience.Private +@InterfaceStability.Unstable +@Metrics(name="HttpServer2", about="HttpServer2 metrics", context="http") +public class HttpServer2Metrics { + + private final StatisticsHandler handler; + + @Metric public int asyncDispatches() { + return handler.getAsyncDispatches(); + } + @Metric public int asyncRequests() { + return handler.getAsyncRequests(); + } + @Metric public int asyncRequestsWaiting() { + return handler.getAsyncRequestsWaiting(); + } + @Metric public int asyncRequestsWaitingMax() { + return handler.getAsyncRequestsWaitingMax(); + } + @Metric public int dispatched() { + return handler.getDispatched(); + } + @Metric public int dispatchedActive() { + return handler.getDispatchedActive(); + } + @Metric public int dispatchedActiveMax() { + return handler.getDispatchedActiveMax(); + } + @Metric public long dispatchedTimeMax() { + return handler.getDispatchedTimeMax(); + } + @Metric public double dispatchedTimeMean() { + return handler.getDispatchedTimeMean(); + } + @Metric public double dispatchedTimeStdDev() { + return handler.getDispatchedTimeStdDev(); + } + @Metric public long dispatchedTimeTotal() { + return handler.getDispatchedTimeTotal(); + } + @Metric public int expires() { + return handler.getExpires(); + } + @Metric public int requests() { + return handler.getRequests(); + } + @Metric public int requestsActive() { + return handler.getRequestsActive(); + } + @Metric public int requestsActiveMax() { + return handler.getRequestsActiveMax(); + } + @Metric public long requestTimeMax() { + return handler.getRequestTimeMax(); + } + @Metric public double requestTimeMean() { + return handler.getRequestTimeMean(); + } + @Metric public double requestTimeStdDev() { + return handler.getRequestTimeStdDev(); + } + @Metric public long requestTimeTotal() { + return handler.getRequestTimeTotal(); + } + @Metric public int responses1xx() { + return handler.getResponses1xx(); + } + @Metric public int responses2xx() { + return handler.getResponses2xx(); + } + @Metric public int responses3xx() { + return handler.getResponses3xx(); + } + @Metric public int responses4xx() { + return handler.getResponses4xx(); + } + @Metric public int responses5xx() { + return handler.getResponses5xx(); + } + @Metric public long getResponseBytesTotal() { + return handler.getResponsesBytesTotal(); + } + @Metric public long statsOnMs() { + return handler.getStatsOnMs(); + } + + HttpServer2Metrics(StatisticsHandler handler) { + this.handler = handler; + } + + public static HttpServer2Metrics create(StatisticsHandler handler) { + MetricsSystem ms = DefaultMetricsSystem.instance(); + return ms.register(new HttpServer2Metrics(handler)); + } +} From db9675b0258f6b1bccc8b6eb74daf231cf74205f Mon Sep 17 00:00:00 2001 From: Akira Ajisaka Date: Thu, 16 Jul 2020 17:11:42 +0900 Subject: [PATCH 02/16] Init metrics system in HttpFS --- .../org/apache/hadoop/fs/http/server/HttpFSServerWebServer.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/HttpFSServerWebServer.java b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/HttpFSServerWebServer.java index 24a30f4db0efb..6b9cb30b90da4 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/HttpFSServerWebServer.java +++ b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/HttpFSServerWebServer.java @@ -30,6 +30,7 @@ import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.http.HttpServer2; +import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem; import org.apache.hadoop.security.AuthenticationFilterInitializer; import org.apache.hadoop.security.authentication.server.ProxyUserAuthenticationFilterInitializer; import org.apache.hadoop.security.authorize.AccessControlList; @@ -119,6 +120,7 @@ public class HttpFSServerWebServer { conf.set(HttpServer2.FILTER_INITIALIZER_PROPERTY, actualInitializers); } + DefaultMetricsSystem.initialize("httpfs"); httpServer = new HttpServer2.Builder() .setName(NAME) .setConf(conf) From a91aca749a61d3160c2a6dd12e2b6bc4b7f65bed Mon Sep 17 00:00:00 2001 From: Akira Ajisaka Date: Thu, 16 Jul 2020 17:56:48 +0900 Subject: [PATCH 03/16] Add document and set the default to true. --- .../apache/hadoop/fs/CommonConfigurationKeysPublic.java | 3 +-- .../java/org/apache/hadoop/http/HttpServer2Metrics.java | 2 +- .../hadoop-common/src/main/resources/core-default.xml | 9 +++++++++ 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeysPublic.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeysPublic.java index 924be77e2f760..20bb0350d191b 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeysPublic.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/CommonConfigurationKeysPublic.java @@ -956,8 +956,7 @@ public class CommonConfigurationKeysPublic { */ public static final String HADOOP_HTTP_METRICS_ENABLED = "hadoop.http.metrics.enabled"; - /** Defalt value for HADOOP_HTTP_METRIC_ENABLED */ - public static final boolean HADOOP_HTTP_METRICS_ENABLED_DEFAULT = false; + public static final boolean HADOOP_HTTP_METRICS_ENABLED_DEFAULT = true; /** * @see diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java index 9eef38337d91e..50bd6546880a2 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java @@ -104,7 +104,7 @@ public class HttpServer2Metrics { @Metric public int responses5xx() { return handler.getResponses5xx(); } - @Metric public long getResponseBytesTotal() { + @Metric public long getResponsesBytesTotal() { return handler.getResponsesBytesTotal(); } @Metric public long statsOnMs() { diff --git a/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml b/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml index 11b790408b79d..7bba8fba8cfca 100644 --- a/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml +++ b/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml @@ -56,6 +56,15 @@ + + hadoop.http.metrics.enabled + true + + If true, add Jetty's StatisticsHandler to HTTP server to collect + HTTP layer metrics and register them to Hadoop metrics system. + + + From fa8385f56164f3ffdcdbd0662bec5ce26efac4d3 Mon Sep 17 00:00:00 2001 From: Akira Ajisaka Date: Fri, 17 Jul 2020 10:59:37 +0900 Subject: [PATCH 04/16] Reverse the order in initializing HttpServer2 metrics --- .../src/main/java/org/apache/hadoop/http/HttpServer2.java | 2 +- .../org/apache/hadoop/crypto/key/kms/server/KMSWebServer.java | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java index 307dc01690169..918894f38f91e 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java @@ -670,8 +670,8 @@ private void initializeWebServer(String name, String hostName, if (conf.getBoolean(CommonConfigurationKeysPublic.HADOOP_HTTP_METRICS_ENABLED, CommonConfigurationKeysPublic.HADOOP_HTTP_METRICS_ENABLED_DEFAULT)) { StatisticsHandler stats = new StatisticsHandler(); - handlers.addHandler(stats); HttpServer2Metrics.create(stats); + handlers.addHandler(stats); } final String appDir = getWebAppsPath(name); diff --git a/hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSWebServer.java b/hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSWebServer.java index 639d85521c3ce..7775ed7fc876f 100644 --- a/hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSWebServer.java +++ b/hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSWebServer.java @@ -114,6 +114,7 @@ public class KMSWebServer { conf.set(HttpServer2.FILTER_INITIALIZER_PROPERTY, actualInitializers); } + DefaultMetricsSystem.initialize("kms"); httpServer = new HttpServer2.Builder() .setName(NAME) .setConf(conf) From 6c6a61be966b172085a68171ab1f4fa71cb2eb5c Mon Sep 17 00:00:00 2001 From: Akira Ajisaka Date: Sun, 19 Jul 2020 00:50:33 +0900 Subject: [PATCH 05/16] Fixed common test failures --- .../src/main/java/org/apache/hadoop/http/HttpServer2.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java index 918894f38f91e..23dfbb19bc08e 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java @@ -671,7 +671,7 @@ private void initializeWebServer(String name, String hostName, CommonConfigurationKeysPublic.HADOOP_HTTP_METRICS_ENABLED_DEFAULT)) { StatisticsHandler stats = new StatisticsHandler(); HttpServer2Metrics.create(stats); - handlers.addHandler(stats); + webServer.setHandler(stats); } final String appDir = getWebAppsPath(name); From 582377e2aa328f05c478e3e0bd6c13755613f06c Mon Sep 17 00:00:00 2001 From: Akira Ajisaka Date: Sun, 19 Jul 2020 00:54:45 +0900 Subject: [PATCH 06/16] Fixed HttpFS test failures --- .../org/apache/hadoop/crypto/key/kms/server/KMSWebServer.java | 1 - .../apache/hadoop/fs/http/server/HttpFSServerWebServer.java | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSWebServer.java b/hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSWebServer.java index 7775ed7fc876f..639d85521c3ce 100644 --- a/hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSWebServer.java +++ b/hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSWebServer.java @@ -114,7 +114,6 @@ public class KMSWebServer { conf.set(HttpServer2.FILTER_INITIALIZER_PROPERTY, actualInitializers); } - DefaultMetricsSystem.initialize("kms"); httpServer = new HttpServer2.Builder() .setName(NAME) .setConf(conf) diff --git a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/HttpFSServerWebServer.java b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/HttpFSServerWebServer.java index 6b9cb30b90da4..9d836fef83c3e 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/HttpFSServerWebServer.java +++ b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/HttpFSServerWebServer.java @@ -120,7 +120,6 @@ public class HttpFSServerWebServer { conf.set(HttpServer2.FILTER_INITIALIZER_PROPERTY, actualInitializers); } - DefaultMetricsSystem.initialize("httpfs"); httpServer = new HttpServer2.Builder() .setName(NAME) .setConf(conf) @@ -152,6 +151,7 @@ private static void deprecateEnv(String varName, Configuration conf, } public void start() throws IOException { + DefaultMetricsSystem.initialize("httpfs"); httpServer.start(); } @@ -160,6 +160,7 @@ public void join() throws InterruptedException { } public void stop() throws Exception { + DefaultMetricsSystem.shutdown(); httpServer.stop(); } From 993c579f80ca2386e359668217b214850a55c684 Mon Sep 17 00:00:00 2001 From: Akira Ajisaka Date: Sun, 19 Jul 2020 04:19:40 +0900 Subject: [PATCH 07/16] Support multiple HttpServer2 instances in a host --- .../org/apache/hadoop/http/HttpServer2.java | 21 ++++++++++++++++--- .../hadoop/http/HttpServer2Metrics.java | 13 +++++++++--- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java index 23dfbb19bc08e..40670f3d99cc9 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java @@ -202,6 +202,9 @@ public final class HttpServer2 implements FilterContainer { protected static final String PROMETHEUS_SINK = "PROMETHEUS_SINK"; private PrometheusMetricsSink prometheusMetricsSink; + private StatisticsHandler statsHandler; + private HttpServer2Metrics metrics; + /** * Class to construct instances of HTTP server with specific options. */ @@ -669,9 +672,8 @@ private void initializeWebServer(String name, String hostName, if (conf.getBoolean(CommonConfigurationKeysPublic.HADOOP_HTTP_METRICS_ENABLED, CommonConfigurationKeysPublic.HADOOP_HTTP_METRICS_ENABLED_DEFAULT)) { - StatisticsHandler stats = new StatisticsHandler(); - HttpServer2Metrics.create(stats); - webServer.setHandler(stats); + statsHandler = new StatisticsHandler(); + webServer.setHandler(statsHandler); } final String appDir = getWebAppsPath(name); @@ -1236,6 +1238,16 @@ public void start() throws IOException { .register("prometheus", "Hadoop metrics prometheus exporter", prometheusMetricsSink); } + if (statsHandler != null) { + // Create metrics source for each HttpServer2 instance. + // Use port number to make the metrics source name unique. + int port = -1; + for (ServerConnector connector : listeners) { + port = connector.getLocalPort(); + break; + } + metrics = HttpServer2Metrics.create(statsHandler, port); + } } catch (IOException ex) { LOG.info("HttpServer.start() threw a non Bind IOException", ex); throw ex; @@ -1418,6 +1430,9 @@ public void stop() throws Exception { try { webServer.stop(); + if (metrics != null) { + metrics.remove(); + } } catch (Exception e) { LOG.error("Error while stopping web server for webapp " + webAppContext.getDisplayName(), e); diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java index 50bd6546880a2..96336ffacce6b 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java @@ -31,6 +31,7 @@ public class HttpServer2Metrics { private final StatisticsHandler handler; + private final int port; @Metric public int asyncDispatches() { return handler.getAsyncDispatches(); @@ -111,12 +112,18 @@ public class HttpServer2Metrics { return handler.getStatsOnMs(); } - HttpServer2Metrics(StatisticsHandler handler) { + HttpServer2Metrics(StatisticsHandler handler, int port) { this.handler = handler; + this.port = port; } - public static HttpServer2Metrics create(StatisticsHandler handler) { + public static HttpServer2Metrics create(StatisticsHandler handler, int port) { MetricsSystem ms = DefaultMetricsSystem.instance(); - return ms.register(new HttpServer2Metrics(handler)); + return ms.register("HttpServer2-" + port, + "HttpServer2 metrics", new HttpServer2Metrics(handler, port)); + } + + public void remove() { + DefaultMetricsSystem.removeSourceName("HttpServer2-" + port); } } From 0f68543d63aadd84acc47d7e9c8e3513f1517050 Mon Sep 17 00:00:00 2001 From: Akira Ajisaka Date: Sun, 19 Jul 2020 04:23:07 +0900 Subject: [PATCH 08/16] Make HttpServer2Metrics methods package-private --- .../main/java/org/apache/hadoop/http/HttpServer2Metrics.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java index 96336ffacce6b..c6eb8ec0537f3 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java @@ -117,13 +117,13 @@ public class HttpServer2Metrics { this.port = port; } - public static HttpServer2Metrics create(StatisticsHandler handler, int port) { + static HttpServer2Metrics create(StatisticsHandler handler, int port) { MetricsSystem ms = DefaultMetricsSystem.instance(); return ms.register("HttpServer2-" + port, "HttpServer2 metrics", new HttpServer2Metrics(handler, port)); } - public void remove() { + void remove() { DefaultMetricsSystem.removeSourceName("HttpServer2-" + port); } } From bb428df63a71d4df860062489f59ea19349d5305 Mon Sep 17 00:00:00 2001 From: Akira Ajisaka Date: Mon, 20 Jul 2020 11:27:56 +0900 Subject: [PATCH 09/16] Fix TestHttpFSServerWebServer#testDoubleStart --- .../java/org/apache/hadoop/http/HttpServer2Metrics.java | 9 ++++++--- .../hadoop/fs/http/server/HttpFSServerWebServer.java | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java index c6eb8ec0537f3..ea657e0cce363 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java @@ -118,9 +118,12 @@ public class HttpServer2Metrics { } static HttpServer2Metrics create(StatisticsHandler handler, int port) { - MetricsSystem ms = DefaultMetricsSystem.instance(); - return ms.register("HttpServer2-" + port, - "HttpServer2 metrics", new HttpServer2Metrics(handler, port)); + final MetricsSystem ms = DefaultMetricsSystem.instance(); + final HttpServer2Metrics metrics = new HttpServer2Metrics(handler, port); + // Remove the old metrics from metrics system to avoid duplicate error + // when HttpServer2 is started twice. + metrics.remove(); + return ms.register("HttpServer2-" + port, "HttpServer2 metrics", metrics); } void remove() { diff --git a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/HttpFSServerWebServer.java b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/HttpFSServerWebServer.java index 9d836fef83c3e..a59d899ae3981 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/HttpFSServerWebServer.java +++ b/hadoop-hdfs-project/hadoop-hdfs-httpfs/src/main/java/org/apache/hadoop/fs/http/server/HttpFSServerWebServer.java @@ -160,8 +160,8 @@ public void join() throws InterruptedException { } public void stop() throws Exception { - DefaultMetricsSystem.shutdown(); httpServer.stop(); + DefaultMetricsSystem.shutdown(); } public URL getUrl() { From 07d8597caf074c38f77071cf6d0f0f51c06d4155 Mon Sep 17 00:00:00 2001 From: Akira Ajisaka Date: Mon, 20 Jul 2020 11:49:50 +0900 Subject: [PATCH 10/16] Add document --- .../hadoop/http/HttpServer2Metrics.java | 83 +++++++++++++------ .../src/main/resources/core-default.xml | 2 +- 2 files changed, 58 insertions(+), 27 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java index ea657e0cce363..db0da285e6aff 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java @@ -25,6 +25,10 @@ import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem; import org.eclipse.jetty.server.handler.StatisticsHandler; +/** + * This class collects all the metrics of Jetty's StatisticsHandler + * and expose them as Hadoop Metrics. + */ @InterfaceAudience.Private @InterfaceStability.Unstable @Metrics(name="HttpServer2", about="HttpServer2 metrics", context="http") @@ -33,82 +37,108 @@ public class HttpServer2Metrics { private final StatisticsHandler handler; private final int port; - @Metric public int asyncDispatches() { + @Metric("number of async requests requests that have expired") + public int asyncDispatches() { return handler.getAsyncDispatches(); } - @Metric public int asyncRequests() { + @Metric("total number of async requests") + public int asyncRequests() { return handler.getAsyncRequests(); } - @Metric public int asyncRequestsWaiting() { + @Metric("currently waiting async requests") + public int asyncRequestsWaiting() { return handler.getAsyncRequestsWaiting(); } - @Metric public int asyncRequestsWaitingMax() { + @Metric("maximum number of waiting async requests") + public int asyncRequestsWaitingMax() { return handler.getAsyncRequestsWaitingMax(); } - @Metric public int dispatched() { + @Metric("number of dispatches") + public int dispatched() { return handler.getDispatched(); } - @Metric public int dispatchedActive() { + @Metric("number of dispatches currently active") + public int dispatchedActive() { return handler.getDispatchedActive(); } - @Metric public int dispatchedActiveMax() { + @Metric("maximum number of active dispatches being handled") + public int dispatchedActiveMax() { return handler.getDispatchedActiveMax(); } - @Metric public long dispatchedTimeMax() { + @Metric("maximum time spend in dispatch handling (in ms)") + public long dispatchedTimeMax() { return handler.getDispatchedTimeMax(); } - @Metric public double dispatchedTimeMean() { + @Metric("mean time spent in dispatch handling (in ms)") + public double dispatchedTimeMean() { return handler.getDispatchedTimeMean(); } - @Metric public double dispatchedTimeStdDev() { + @Metric("standard deviation for dispatch handling (in ms)") + public double dispatchedTimeStdDev() { return handler.getDispatchedTimeStdDev(); } - @Metric public long dispatchedTimeTotal() { + @Metric("total time spent in dispatch handling (in ms)") + public long dispatchedTimeTotal() { return handler.getDispatchedTimeTotal(); } - @Metric public int expires() { + @Metric("number of async requests requests that have expired") + public int expires() { return handler.getExpires(); } - @Metric public int requests() { + @Metric("number of requests") + public int requests() { return handler.getRequests(); } - @Metric public int requestsActive() { + @Metric("number of requests currently active") + public int requestsActive() { return handler.getRequestsActive(); } - @Metric public int requestsActiveMax() { + @Metric("maximum number of active requests") + public int requestsActiveMax() { return handler.getRequestsActiveMax(); } - @Metric public long requestTimeMax() { + @Metric("maximum time spend handling requests (in ms)") + public long requestTimeMax() { return handler.getRequestTimeMax(); } - @Metric public double requestTimeMean() { + @Metric("mean time spent handling requests (in ms)") + public double requestTimeMean() { return handler.getRequestTimeMean(); } - @Metric public double requestTimeStdDev() { + @Metric("standard deviation for request handling (in ms)") + public double requestTimeStdDev() { return handler.getRequestTimeStdDev(); } - @Metric public long requestTimeTotal() { + @Metric("total time spend in all request handling (in ms)") + public long requestTimeTotal() { return handler.getRequestTimeTotal(); } - @Metric public int responses1xx() { + @Metric("number of requests with 1xx response status") + public int responses1xx() { return handler.getResponses1xx(); } - @Metric public int responses2xx() { + @Metric("number of requests with 2xx response status") + public int responses2xx() { return handler.getResponses2xx(); } - @Metric public int responses3xx() { + @Metric("number of requests with 3xx response status") + public int responses3xx() { return handler.getResponses3xx(); } - @Metric public int responses4xx() { + @Metric("number of requests with 4xx response status") + public int responses4xx() { return handler.getResponses4xx(); } - @Metric public int responses5xx() { + @Metric("number of requests with 5xx response status") + public int responses5xx() { return handler.getResponses5xx(); } - @Metric public long getResponsesBytesTotal() { + @Metric("total number of bytes across all responses") + public long getResponsesBytesTotal() { return handler.getResponsesBytesTotal(); } - @Metric public long statsOnMs() { + @Metric("time in milliseconds stats have been collected for") + public long statsOnMs() { return handler.getStatsOnMs(); } @@ -123,6 +153,7 @@ static HttpServer2Metrics create(StatisticsHandler handler, int port) { // Remove the old metrics from metrics system to avoid duplicate error // when HttpServer2 is started twice. metrics.remove(); + // Add port number to the suffix to allow multiple instances in a host. return ms.register("HttpServer2-" + port, "HttpServer2 metrics", metrics); } diff --git a/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml b/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml index 7bba8fba8cfca..4794bb2764c80 100644 --- a/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml +++ b/hadoop-common-project/hadoop-common/src/main/resources/core-default.xml @@ -60,7 +60,7 @@ hadoop.http.metrics.enabled true - If true, add Jetty's StatisticsHandler to HTTP server to collect + If true, set Jetty's StatisticsHandler to HTTP server to collect HTTP layer metrics and register them to Hadoop metrics system. From 6b606966402b44e1d31477d379fcf3b0618b4636 Mon Sep 17 00:00:00 2001 From: Akira Ajisaka Date: Tue, 21 Jul 2020 18:15:06 +0900 Subject: [PATCH 11/16] Organized imports --- .../main/java/org/apache/hadoop/http/HttpServer2Metrics.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java index db0da285e6aff..cbcb4da8d9e29 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java @@ -17,13 +17,14 @@ */ package org.apache.hadoop.http; +import org.eclipse.jetty.server.handler.StatisticsHandler; + import org.apache.hadoop.classification.InterfaceAudience; import org.apache.hadoop.classification.InterfaceStability; import org.apache.hadoop.metrics2.MetricsSystem; import org.apache.hadoop.metrics2.annotation.Metric; import org.apache.hadoop.metrics2.annotation.Metrics; import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem; -import org.eclipse.jetty.server.handler.StatisticsHandler; /** * This class collects all the metrics of Jetty's StatisticsHandler From 285abdc4fa4da53c7ca0c7578fb5f70e68651e4b Mon Sep 17 00:00:00 2001 From: Akira Ajisaka Date: Tue, 21 Jul 2020 20:32:55 +0900 Subject: [PATCH 12/16] Add tests. Some servlets does not close the stream and then StatisticsHandler fails. Fixed the servlets. --- .../org/apache/hadoop/http/HttpServer2.java | 9 ++++++++- .../apache/hadoop/http/HttpServer2Metrics.java | 2 +- .../java/org/apache/hadoop/log/LogLevel.java | 1 + .../hadoop/http/HttpServerFunctionalTest.java | 1 + .../org/apache/hadoop/http/TestHttpServer.java | 18 ++++++++++++++++++ 5 files changed, 29 insertions(+), 2 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java index 40670f3d99cc9..64f79294e38cf 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java @@ -50,6 +50,7 @@ import javax.servlet.http.HttpServletRequestWrapper; import javax.servlet.http.HttpServletResponse; +import org.apache.hadoop.thirdparty.com.google.common.annotations.VisibleForTesting; import org.apache.hadoop.thirdparty.com.google.common.base.Preconditions; import org.apache.hadoop.thirdparty.com.google.common.collect.ImmutableMap; import org.apache.hadoop.thirdparty.com.google.common.collect.Lists; @@ -673,7 +674,7 @@ private void initializeWebServer(String name, String hostName, if (conf.getBoolean(CommonConfigurationKeysPublic.HADOOP_HTTP_METRICS_ENABLED, CommonConfigurationKeysPublic.HADOOP_HTTP_METRICS_ENABLED_DEFAULT)) { statsHandler = new StatisticsHandler(); - webServer.setHandler(statsHandler); + handlers.addHandler(statsHandler); } final String appDir = getWebAppsPath(name); @@ -1813,4 +1814,10 @@ private Map getDefaultHeaders() { splitVal[1]); return headers; } + + @VisibleForTesting + HttpServer2Metrics getMetrics() { + return metrics; + } + } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java index cbcb4da8d9e29..000c8e900778e 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java @@ -135,7 +135,7 @@ public int responses5xx() { return handler.getResponses5xx(); } @Metric("total number of bytes across all responses") - public long getResponsesBytesTotal() { + public long responsesBytesTotal() { return handler.getResponsesBytesTotal(); } @Metric("time in milliseconds stats have been collected for") diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/log/LogLevel.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/log/LogLevel.java index 829d276ea7281..c72c91169ba20 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/log/LogLevel.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/log/LogLevel.java @@ -358,6 +358,7 @@ else if (log instanceof Jdk14Logger) { out.println(FORMS); out.println(ServletUtil.HTML_TAIL); + out.close(); } static final String FORMS = "\n

Get / Set

" diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/HttpServerFunctionalTest.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/HttpServerFunctionalTest.java index f2d5541632285..941e39e72deb8 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/HttpServerFunctionalTest.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/HttpServerFunctionalTest.java @@ -51,6 +51,7 @@ public void doGet(HttpServletRequest request, ) throws ServletException, IOException { Assert.assertEquals(63 * 1024, request.getHeader("longheader").length()); response.setStatus(HttpServletResponse.SC_OK); + response.getWriter().close(); } } diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestHttpServer.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestHttpServer.java index ad9617dca79de..f19474e6b633e 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestHttpServer.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestHttpServer.java @@ -20,6 +20,7 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.conf.Configuration.IntegerRanges; import org.apache.hadoop.fs.CommonConfigurationKeys; +import org.apache.hadoop.fs.CommonConfigurationKeysPublic; import org.apache.hadoop.http.HttpServer2.QuotingInputFilter.RequestQuoter; import org.apache.hadoop.http.resource.JerseyResource; import org.apache.hadoop.net.NetUtils; @@ -29,6 +30,8 @@ import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.security.authorize.AccessControlList; import org.apache.hadoop.test.Whitebox; + +import org.assertj.core.api.Assertions; import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.util.ajax.JSON; import org.junit.AfterClass; @@ -148,6 +151,8 @@ public void doGet(HttpServletRequest request, @BeforeClass public static void setup() throws Exception { Configuration conf = new Configuration(); conf.setInt(HttpServer2.HTTP_MAX_THREADS_KEY, MAX_THREADS); + conf.setBoolean( + CommonConfigurationKeysPublic.HADOOP_HTTP_METRICS_ENABLED, true); server = createTestServer(conf); server.addServlet("echo", "/echo", EchoServlet.class); server.addServlet("echomap", "/echomap", EchoMapServlet.class); @@ -272,6 +277,19 @@ public void testAcceptorSelectorConfigurability() throws Exception { conn.getContentType()); } + @Test + public void testHttpServer2Metrics() throws Exception { + final HttpServer2Metrics metrics = server.getMetrics(); + final int before = metrics.responses2xx(); + final URL servletUrl = new URL(baseUrl, "/echo?echo"); + final HttpURLConnection conn = + (HttpURLConnection)servletUrl.openConnection(); + conn.connect(); + Assertions.assertThat(conn.getResponseCode()).isEqualTo(200); + final int after = metrics.responses2xx(); + Assertions.assertThat(after).isGreaterThan(before); + } + @Test public void testHttpResonseContainsXFrameOptions() throws Exception { validateXFrameOption(HttpServer2.XFrameOption.SAMEORIGIN); From 426f085c570158f3198a01a4fbc93c6759ac9a14 Mon Sep 17 00:00:00 2001 From: Akira Ajisaka Date: Wed, 22 Jul 2020 14:06:51 +0900 Subject: [PATCH 13/16] Move StatisticsHandler to the first --- .../java/org/apache/hadoop/http/HttpServer2.java | 13 ++++++++----- .../main/java/org/apache/hadoop/log/LogLevel.java | 1 - .../hadoop/http/HttpServerFunctionalTest.java | 1 - 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java index 64f79294e38cf..62d8ab699fca9 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java @@ -670,17 +670,20 @@ private void initializeWebServer(String name, String hostName, handlers.addHandler(requestLogHandler); } handlers.addHandler(webAppContext); + final String appDir = getWebAppsPath(name); + addDefaultApps(contexts, appDir, conf); + webServer.setHandler(handlers); + // Jetty StatisticsHandler should be the first handler. + // The handler returns 503 if there is no next handler and the response is + // not committed. In Apache Hadoop, there are some servlets that do not + // commit (i.e. close) the response. Therefore the handler fails. if (conf.getBoolean(CommonConfigurationKeysPublic.HADOOP_HTTP_METRICS_ENABLED, CommonConfigurationKeysPublic.HADOOP_HTTP_METRICS_ENABLED_DEFAULT)) { statsHandler = new StatisticsHandler(); - handlers.addHandler(statsHandler); + webServer.insertHandler(statsHandler); } - final String appDir = getWebAppsPath(name); - addDefaultApps(contexts, appDir, conf); - webServer.setHandler(handlers); - Map xFrameParams = setHeaders(conf); addGlobalFilter("safety", QuotingInputFilter.class.getName(), xFrameParams); final FilterInitializer[] initializers = getFilterInitializers(conf); diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/log/LogLevel.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/log/LogLevel.java index c72c91169ba20..829d276ea7281 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/log/LogLevel.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/log/LogLevel.java @@ -358,7 +358,6 @@ else if (log instanceof Jdk14Logger) { out.println(FORMS); out.println(ServletUtil.HTML_TAIL); - out.close(); } static final String FORMS = "\n

Get / Set

" diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/HttpServerFunctionalTest.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/HttpServerFunctionalTest.java index 941e39e72deb8..f2d5541632285 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/HttpServerFunctionalTest.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/HttpServerFunctionalTest.java @@ -51,7 +51,6 @@ public void doGet(HttpServletRequest request, ) throws ServletException, IOException { Assert.assertEquals(63 * 1024, request.getHeader("longheader").length()); response.setStatus(HttpServletResponse.SC_OK); - response.getWriter().close(); } } From 34bb0a6855cec3faca4a5b7eec323b5ca273743d Mon Sep 17 00:00:00 2001 From: Akira Ajisaka Date: Wed, 24 Mar 2021 14:49:31 +0900 Subject: [PATCH 14/16] Add misimplementation test case. --- .../org/apache/hadoop/http/HttpServer2.java | 18 ++++++++++++---- .../apache/hadoop/http/TestHttpServer.java | 21 +++++++++++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java index 62d8ab699fca9..77236c13158ac 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java @@ -674,12 +674,22 @@ private void initializeWebServer(String name, String hostName, addDefaultApps(contexts, appDir, conf); webServer.setHandler(handlers); - // Jetty StatisticsHandler should be the first handler. - // The handler returns 503 if there is no next handler and the response is - // not committed. In Apache Hadoop, there are some servlets that do not - // commit (i.e. close) the response. Therefore the handler fails. if (conf.getBoolean(CommonConfigurationKeysPublic.HADOOP_HTTP_METRICS_ENABLED, CommonConfigurationKeysPublic.HADOOP_HTTP_METRICS_ENABLED_DEFAULT)) { + // Jetty StatisticsHandler must be inserted as the first handler. + // The tree might look like this: + // + // - StatisticsHandler (for all requests) + // - HandlerList + // - ContextHandlerCollection + // - RequestLogHandler (if enabled) + // - WebAppContext + // - SessionHandler + // - Servlets + // - Filters + // - etc.. + // + // Reference: https://www.eclipse.org/lists/jetty-users/msg06273.html statsHandler = new StatisticsHandler(); webServer.insertHandler(statsHandler); } diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestHttpServer.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestHttpServer.java index f19474e6b633e..e3cb028f5f553 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestHttpServer.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestHttpServer.java @@ -33,6 +33,7 @@ import org.assertj.core.api.Assertions; import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.server.handler.StatisticsHandler; import org.eclipse.jetty.util.ajax.JSON; import org.junit.AfterClass; import org.junit.Assert; @@ -290,6 +291,26 @@ public void testHttpServer2Metrics() throws Exception { Assertions.assertThat(after).isGreaterThan(before); } + /** + * Jetty StatisticsHandler must be inserted via Server#insertHandler + * instead of Server#setHandler. The server fails to start if + * the handler is added by setHandler. + */ + @Test + public void testSetStatisticsHandler() throws Exception { + final Configuration conf = new Configuration(); + // skip insert + conf.setBoolean( + CommonConfigurationKeysPublic.HADOOP_HTTP_METRICS_ENABLED, false); + final HttpServer2 testServer = createTestServer(conf); + testServer.webServer.setHandler(new StatisticsHandler()); + try { + testServer.start(); + fail("IOException should be thrown."); + } catch (IOException ignore) { + } + } + @Test public void testHttpResonseContainsXFrameOptions() throws Exception { validateXFrameOption(HttpServer2.XFrameOption.SAMEORIGIN); From 7f78704b9a060b4deaa9ec1dd7b30c85b614cbfa Mon Sep 17 00:00:00 2001 From: Akira Ajisaka Date: Thu, 25 Mar 2021 17:14:39 +0900 Subject: [PATCH 15/16] Fix javadoc mismatch --- .../main/java/org/apache/hadoop/http/HttpServer2Metrics.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java index 000c8e900778e..7a74e7be3f74d 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2Metrics.java @@ -38,7 +38,7 @@ public class HttpServer2Metrics { private final StatisticsHandler handler; private final int port; - @Metric("number of async requests requests that have expired") + @Metric("number of requested that have been asynchronously dispatched") public int asyncDispatches() { return handler.getAsyncDispatches(); } From 453c37ef21f21aed28fcc7805ef65ff8a7a53064 Mon Sep 17 00:00:00 2001 From: Akira Ajisaka Date: Thu, 25 Mar 2021 17:15:34 +0900 Subject: [PATCH 16/16] Fix checkstyle warning --- .../src/main/java/org/apache/hadoop/http/HttpServer2.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java index 77236c13158ac..7534cba45ee13 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/http/HttpServer2.java @@ -674,7 +674,8 @@ private void initializeWebServer(String name, String hostName, addDefaultApps(contexts, appDir, conf); webServer.setHandler(handlers); - if (conf.getBoolean(CommonConfigurationKeysPublic.HADOOP_HTTP_METRICS_ENABLED, + if (conf.getBoolean( + CommonConfigurationKeysPublic.HADOOP_HTTP_METRICS_ENABLED, CommonConfigurationKeysPublic.HADOOP_HTTP_METRICS_ENABLED_DEFAULT)) { // Jetty StatisticsHandler must be inserted as the first handler. // The tree might look like this: