Re: [PR] [fix][broker] Optimize /metrics, fix unbounded request queue issue and fix race conditions in metricsBufferResponse mode [pulsar]

["asafm (via GitHub)" <gi...@apache.org>]

asafm commented on code in PR #22494:
URL: https://github.com/apache/pulsar/pull/22494#discussion_r1623253845


##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/stats/prometheus/PrometheusMetricsServlet.java:
##########
@@ -25,77 +25,146 @@
 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(() -> {
-            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

Review Comment:
   Seeing this makes me happy we're moving to a consolidated maintained exporters in OTel :)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org