Synchronise modifications of metricsServlet and pendingMetricsProviders

apache · Mar 15, 2022 · d32291e · d32291e
1 parent c21738d
commit d32291e
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 28 deletions.
diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
@@ -413,7 +413,9 @@ public CompletableFuture<Void> closeAsync() {
                 }
             }
 
-            metricsServlet = null;
+            synchronized (this) {
+                metricsServlet = null;
+            }
 
             if (this.webSocketService != null) {
                 this.webSocketService.close();
@@ -697,14 +699,17 @@ public void start() throws PulsarServerException {
             this.brokerAdditionalServlets = AdditionalServlets.load(config);
 
             this.webService = new WebService(this);
-            this.metricsServlet = new PulsarPrometheusMetricsServlet(
-                    this, config.isExposeTopicLevelMetricsInPrometheus(),
-                    config.isExposeConsumerLevelMetricsInPrometheus(),
-                    config.isExposeProducerLevelMetricsInPrometheus(),
-                    config.isSplitTopicAndPartitionLabelInPrometheus());
-            if (pendingMetricsProviders != null) {
-                pendingMetricsProviders.forEach(provider -> metricsServlet.addRawMetricsProvider(provider));
-                this.pendingMetricsProviders = null;
+
+            synchronized (this) {
+                this.metricsServlet = new PulsarPrometheusMetricsServlet(
+                        this, config.isExposeTopicLevelMetricsInPrometheus(),
+                        config.isExposeConsumerLevelMetricsInPrometheus(),
+                        config.isExposeProducerLevelMetricsInPrometheus(),
+                        config.isSplitTopicAndPartitionLabelInPrometheus());
+                if (pendingMetricsProviders != null) {
+                    pendingMetricsProviders.forEach(provider -> metricsServlet.addRawMetricsProvider(provider));
+                    this.pendingMetricsProviders = null;
+                }
             }
 
             this.addWebServerHandlers(webService, metricsServlet, this.config);
@@ -1516,13 +1521,15 @@ public ResourceUsageTransportManager getResourceUsageTransportManager() {
     }
 
     public void addPrometheusRawMetricsProvider(PrometheusRawMetricsProvider metricsProvider) {
-        if (metricsServlet == null) {
-            if (pendingMetricsProviders == null) {
-                pendingMetricsProviders = new LinkedList<>();
+        synchronized (this) {
+            if (metricsServlet == null) {
+                if (pendingMetricsProviders == null) {
+                    pendingMetricsProviders = new LinkedList<>();
+                }
+                pendingMetricsProviders.add(metricsProvider);
+            } else {
+                this.metricsServlet.addRawMetricsProvider(metricsProvider);
             }
-            pendingMetricsProviders.add(metricsProvider);
-        } else {
-            this.metricsServlet.addRawMetricsProvider(metricsProvider);
         }
     }
 

diff --git a/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyService.java b/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyService.java
@@ -256,10 +256,12 @@ public void start() throws Exception {
             this.serviceUrlTls = null;
         }
 
-        this.metricsServlet = new PrometheusMetricsServlet(-1L, proxyConfig.getClusterName());
-        if (pendingMetricsProviders != null) {
-            pendingMetricsProviders.forEach(provider -> metricsServlet.addRawMetricsProvider(provider));
-            this.pendingMetricsProviders = null;
+        synchronized (this) {
+            this.metricsServlet = new PrometheusMetricsServlet(-1L, proxyConfig.getClusterName());
+            if (pendingMetricsProviders != null) {
+                pendingMetricsProviders.forEach(provider -> metricsServlet.addRawMetricsProvider(provider));
+                this.pendingMetricsProviders = null;
+            }
         }
 
         // Initialize the message protocol handlers.
@@ -347,7 +349,9 @@ public void close() throws IOException {
             proxyAdditionalServlets = null;
         }
 
-        metricsServlet = null;
+        synchronized (this) {
+            metricsServlet = null;
+        }
 
         if (localMetadataStore != null) {
             try {
@@ -429,13 +433,15 @@ public Authentication getProxyClientAuthenticationPlugin() {
     }
 
     public void addPrometheusRawMetricsProvider(PrometheusRawMetricsProvider metricsProvider) {
-        if (metricsServlet == null) {
-            if (pendingMetricsProviders == null) {
-                pendingMetricsProviders = new LinkedList<>();
+        synchronized (this) {
+            if (metricsServlet == null) {
+                if (pendingMetricsProviders == null) {
+                    pendingMetricsProviders = new LinkedList<>();
+                }
+                pendingMetricsProviders.add(metricsProvider);
+            } else {
+                this.metricsServlet.addRawMetricsProvider(metricsProvider);
             }
-            pendingMetricsProviders.add(metricsProvider);
-        } else {
-            this.metricsServlet.addRawMetricsProvider(metricsProvider);
         }
     }
 

diff --git a/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyServiceStarter.java b/pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyServiceStarter.java
@@ -239,8 +239,10 @@ public static void addWebServerHandlers(WebServer server,
                                      ProxyConfiguration config,
                                      ProxyService service,
                                      BrokerDiscoveryProvider discoveryProvider) throws Exception {
-        server.addServlet("/metrics", new ServletHolder(service.getMetricsServlet()),
-                Collections.emptyList(), config.isAuthenticateMetricsEndpoint());
+        if (service != null && service.getMetricsServlet() != null) {
+            server.addServlet("/metrics", new ServletHolder(service.getMetricsServlet()),
+                    Collections.emptyList(), config.isAuthenticateMetricsEndpoint());
+        }
         server.addRestResources("/", VipStatus.class.getPackage().getName(),
                 VipStatus.ATTRIBUTE_STATUS_FILE_PATH, config.getStatusFilePath());
         server.addRestResources("/proxy-stats", ProxyStats.class.getPackage().getName(),