PrashantBhanage commented on code in PR #13696:
URL: https://github.com/apache/cloudstack/pull/13696#discussion_r3649951306
##########
plugins/integrations/prometheus/src/main/java/org/apache/cloudstack/metrics/PrometheusExporterServerImpl.java:
##########
@@ -108,6 +113,10 @@ public boolean stop() {
httpServer.stop(0);
logger.debug("Stopped Prometheus exporter http server");
}
+ if (httpExecutor != null) {
+ httpExecutor.shutdownNow();
+ logger.debug("Shut down Prometheus exporter http executor");
+ }
Review Comment:
Both fair points:
- `setExecutor(null)`: not functionally needed since `start()` always
creates a fresh `HttpServer` instance (the old one, executor included,
just gets discarded/GC'd — and JDK's `HttpServer` can't be restarted
after `stop()` anyway). But happy to add it defensively, no downside.
- `FixedThreadPool(2)`: kept deliberately small since the TTL guard
means most scrapes now skip `updateMetrics()` entirely — 2 threads is
just enough to stop one slow request blocking others, without
over-engineering. Can make it configurable if you'd prefer.
Will push a follow-up commit nulling out the executor per your suggestion.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
