Run tests for branch-2.11 with 4 cherry-picked changes

conf/proxy.conf

@@ -361,5 +361,7 @@ zooKeeperCacheExpirySeconds=-1
 enableProxyStatsEndpoints=true
 # Whether the '/metrics' endpoint requires authentication. Defaults to true
 authenticateMetricsEndpoint=true
-# Enable cache metrics data, default value is false
-metricsBufferResponse=false
+# Time in milliseconds that metrics endpoint would time out. Default is 30s.
+# Set it to 0 to disable timeout.
+metricsServletTimeoutMs=30000
+

pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java

@@ -327,6 +327,19 @@ public class ServiceConfiguration implements PulsarConfiguration {
             + "(0 to disable limiting)")
     private int maxHttpServerConnections = 2048;
 
+    @FieldContext(category = CATEGORY_SERVER, doc =
+            "Gzip compression is enabled by default. Specific paths can be excluded from compression.\n"
+                    + "There are 2 syntaxes supported, Servlet url-pattern based, and Regex based.\n"
+                    + "If the spec starts with '^' the spec is assumed to be a regex based path spec and will match "
+                    + "with normal Java regex rules.\n"
+                    + "If the spec starts with '/' then spec is assumed to be a Servlet url-pattern rules path spec "
+                    + "for either an exact match or prefix based match.\n"
+                    + "If the spec starts with '*.' then spec is assumed to be a Servlet url-pattern rules path spec "
+                    + "for a suffix based match.\n"
+                    + "All other syntaxes are unsupported.\n"
+                    + "Disable all compression with ^.* or ^.*$")
+    private List<String> httpServerGzipCompressionExcludedPaths = new ArrayList<>();
+
     @FieldContext(category = CATEGORY_SERVER, doc = "Whether to enable the delayed delivery for messages.")
     private boolean delayedDeliveryEnabled = true;
 

pulsar-broker-common/src/main/java/org/apache/pulsar/broker/stats/prometheus/PrometheusMetricsGeneratorUtils.java

@@ -76,7 +76,7 @@ public static void generateSystemMetrics(SimpleTextOutputStream stream, String c
                 }
                 for (int j = 0; j < sample.labelNames.size(); j++) {
                     String labelValue = sample.labelValues.get(j);
-                    if (labelValue != null) {
+                    if (labelValue != null && labelValue.indexOf('"') > -1) {
                         labelValue = labelValue.replace("\"", "\\\"");
                     }
                     if (j > 0) {

pulsar-broker-common/src/main/java/org/apache/pulsar/broker/stats/prometheus/PrometheusMetricsServlet.java

@@ -18,85 +18,153 @@
  */
 package org.apache.pulsar.broker.stats.prometheus;
 
-import static org.apache.bookkeeper.util.SafeRunnable.safeRun;
 import io.netty.util.concurrent.DefaultThreadFactory;
 import java.io.EOFException;
 import java.io.IOException;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
 import javax.servlet.AsyncContext;
+import javax.servlet.AsyncEvent;
+import javax.servlet.AsyncListener;
 import javax.servlet.ServletException;
-import javax.servlet.ServletOutputStream;
 import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 public class PrometheusMetricsServlet extends HttpServlet {
-
     private static final long serialVersionUID = 1L;
-    private static final int HTTP_STATUS_OK_200 = 200;
-    private static final int HTTP_STATUS_INTERNAL_SERVER_ERROR_500 = 500;
-
-    private final long metricsServletTimeoutMs;
-    private final String cluster;
+    static final int HTTP_STATUS_OK_200 = 200;
+    static final int HTTP_STATUS_INTERNAL_SERVER_ERROR_500 = 500;
+    protected final long metricsServletTimeoutMs;
+    protected final String cluster;
     protected List<PrometheusRawMetricsProvider> metricsProviders;
 
-    private ExecutorService executor = null;
+    protected ExecutorService executor = null;
+    protected final int executorMaxThreads;
 
     public PrometheusMetricsServlet(long metricsServletTimeoutMs, String cluster) {
+        this(metricsServletTimeoutMs, cluster, 1);
+    }
+
+    public PrometheusMetricsServlet(long metricsServletTimeoutMs, String cluster, int executorMaxThreads) {
         this.metricsServletTimeoutMs = metricsServletTimeoutMs;
         this.cluster = cluster;
+        this.executorMaxThreads = executorMaxThreads;
     }
 
     @Override
     public void init() throws ServletException {
-        executor = Executors.newSingleThreadScheduledExecutor(new DefaultThreadFactory("prometheus-stats"));
+        if (executorMaxThreads > 0) {
+            executor =
+                    Executors.newScheduledThreadPool(executorMaxThreads, new DefaultThreadFactory("prometheus-stats"));
+        }
     }
 
     @Override
     protected void doGet(HttpServletRequest request, HttpServletResponse response) {
         AsyncContext context = request.startAsync();
-        context.setTimeout(metricsServletTimeoutMs);
-        executor.execute(safeRun(() -> {
-            long start = System.currentTimeMillis();
-            HttpServletResponse res = (HttpServletResponse) context.getResponse();
-            try {
-                res.setStatus(HTTP_STATUS_OK_200);
-                res.setContentType("text/plain;charset=utf-8");
-                generateMetrics(cluster, res.getOutputStream());
-            } catch (Exception e) {
-                long end = System.currentTimeMillis();
-                long time = end - start;
-                if (e instanceof EOFException) {
-                    // NO STACKTRACE
-                    log.error("Failed to send metrics, "
-                            + "likely the client or this server closed "
-                            + "the connection due to a timeout ({} ms elapsed): {}", time, e + "");
-                } else {
-                    log.error("Failed to generate prometheus stats, {} ms elapsed", time, e);
+        // set hard timeout to 2 * timeout
+        if (metricsServletTimeoutMs > 0) {
+            context.setTimeout(metricsServletTimeoutMs * 2);
+        }
+        long startNanos = System.nanoTime();
+        AtomicBoolean taskStarted = new AtomicBoolean(false);
+        Future<?> future = executor.submit(() -> {
+            taskStarted.set(true);
+            long elapsedNanos = System.nanoTime() - startNanos;
+            // check if the request has been timed out, implement a soft timeout
+            // so that response writing can continue to up to 2 * timeout
+            if (metricsServletTimeoutMs > 0 && elapsedNanos > TimeUnit.MILLISECONDS.toNanos(metricsServletTimeoutMs)) {
+                log.warn("Prometheus metrics request was too long in queue ({}ms). Skipping sending metrics.",
+                        TimeUnit.NANOSECONDS.toMillis(elapsedNanos));
+                if (!response.isCommitted()) {
+                    response.setStatus(HTTP_STATUS_INTERNAL_SERVER_ERROR_500);
                 }
-                res.setStatus(HTTP_STATUS_INTERNAL_SERVER_ERROR_500);
-            } finally {
-                long end = System.currentTimeMillis();
-                long time = end - start;
-                try {
-                    context.complete();
-                } catch (IllegalStateException e) {
-                    // this happens when metricsServletTimeoutMs expires
-                    // java.lang.IllegalStateException: AsyncContext completed and/or Request lifecycle recycled
-                    log.error("Failed to generate prometheus stats, "
-                            + "this is likely due to metricsServletTimeoutMs: {} ms elapsed: {}", time, e + "");
+                context.complete();
+                return;
+            }
+            handleAsyncMetricsRequest(context);
+        });
+        context.addListener(new AsyncListener() {
+            @Override
+            public void onComplete(AsyncEvent asyncEvent) throws IOException {
+                if (!taskStarted.get()) {
+                    future.cancel(false);
+                }
+            }
+
+            @Override
+            public void onTimeout(AsyncEvent asyncEvent) throws IOException {
+                if (!taskStarted.get()) {
+                    future.cancel(false);
+                }
+                log.warn("Prometheus metrics request timed out");
+                HttpServletResponse res = (HttpServletResponse) context.getResponse();
+                if (!res.isCommitted()) {
+                    res.setStatus(HTTP_STATUS_INTERNAL_SERVER_ERROR_500);
+                }
+                context.complete();
+            }
+
+            @Override
+            public void onError(AsyncEvent asyncEvent) throws IOException {
+                if (!taskStarted.get()) {
+                    future.cancel(false);
                 }
             }
-        }));
+
+            @Override
+            public void onStartAsync(AsyncEvent asyncEvent) throws IOException {
+
+            }
+        });
+
+    }
+
+    private void handleAsyncMetricsRequest(AsyncContext context) {
+        long start = System.currentTimeMillis();
+        HttpServletResponse res = (HttpServletResponse) context.getResponse();
+        try {
+            generateMetricsSynchronously(res);
+        } catch (Exception e) {
+            long end = System.currentTimeMillis();
+            long time = end - start;
+            if (e instanceof EOFException) {
+                // NO STACKTRACE
+                log.error("Failed to send metrics, "
+                        + "likely the client or this server closed "
+                        + "the connection due to a timeout ({} ms elapsed): {}", time, e + "");
+            } else {
+                log.error("Failed to generate prometheus stats, {} ms elapsed", time, e);
+            }
+            if (!res.isCommitted()) {
+                res.setStatus(HTTP_STATUS_INTERNAL_SERVER_ERROR_500);
+            }
+        } finally {
+            long end = System.currentTimeMillis();
+            long time = end - start;
+            try {
+                context.complete();
+            } catch (IllegalStateException e) {
+                // this happens when metricsServletTimeoutMs expires
+                // java.lang.IllegalStateException: AsyncContext completed and/or Request lifecycle recycled
+                log.error("Failed to generate prometheus stats, "
+                        + "this is likely due to metricsServletTimeoutMs: {} ms elapsed: {}", time, e + "");
+            }
+        }
     }
 
-    protected void generateMetrics(String cluster, ServletOutputStream outputStream) throws IOException {
-        PrometheusMetricsGeneratorUtils.generate(cluster, outputStream, metricsProviders);
+    private void generateMetricsSynchronously(HttpServletResponse res) throws IOException {
+        res.setStatus(HTTP_STATUS_OK_200);
+        res.setContentType("text/plain;charset=utf-8");
+        PrometheusMetricsGeneratorUtils.generate(cluster, res.getOutputStream(), metricsProviders);
     }
 
     @Override

pulsar-broker-common/src/main/java/org/apache/pulsar/broker/web/GzipHandlerUtil.java

@@ -0,0 +1,48 @@
+/**
+ * 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.pulsar.broker.web;
+
+import java.util.List;
+import org.eclipse.jetty.server.Handler;
+import org.eclipse.jetty.server.handler.gzip.GzipHandler;
+
+public class GzipHandlerUtil {
+    public static Handler wrapWithGzipHandler(Handler innerHandler, List<String> gzipCompressionExcludedPaths) {
+        Handler wrappedHandler;
+        if (isGzipCompressionCompletelyDisabled(gzipCompressionExcludedPaths)) {
+            // no need to add GZIP handler if it's disabled by setting the excluded path to "^.*" or "^.*$"
+            wrappedHandler = innerHandler;
+        } else {
+            // add GZIP handler which is active when the request contains "Accept-Encoding: gzip" header
+            GzipHandler gzipHandler = new GzipHandler();
+            gzipHandler.setHandler(innerHandler);
+            if (gzipCompressionExcludedPaths != null && gzipCompressionExcludedPaths.size() > 0) {
+                gzipHandler.setExcludedPaths(gzipCompressionExcludedPaths.toArray(new String[0]));
+            }
+            wrappedHandler = gzipHandler;
+        }
+        return wrappedHandler;
+    }
+
+    public static boolean isGzipCompressionCompletelyDisabled(List<String> gzipCompressionExcludedPaths) {
+        return gzipCompressionExcludedPaths != null && gzipCompressionExcludedPaths.size() == 1
+                && (gzipCompressionExcludedPaths.get(0).equals("^.*")
+                || gzipCompressionExcludedPaths.get(0).equals("^.*$"));
+    }
+}