walterddr commented on a change in pull request #11586: [FLINK-5552][runtime]
make JMXServer static per JVM
URL: https://github.com/apache/flink/pull/11586#discussion_r401916046
##########
File path:
flink-metrics/flink-metrics-jmx/src/main/java/org/apache/flink/metrics/jmx/JMXReporter.java
##########
@@ -96,29 +87,7 @@ public String filterCharacters(String input) {
this.registeredMetrics = new HashMap<>();
if (portsConfig != null) {
- Iterator<Integer> ports =
NetUtils.getPortRangeFromString(portsConfig);
-
- JMXServer successfullyStartedServer = null;
- while (ports.hasNext() && successfullyStartedServer ==
null) {
- JMXServer server = new JMXServer();
- int port = ports.next();
- try {
- server.start(port);
- LOG.info("Started JMX server on port "
+ port + ".");
- successfullyStartedServer = server;
- } catch (IOException ioe) { //assume port
conflict
- LOG.debug("Could not start JMX server
on port " + port + ".", ioe);
- try {
- server.stop();
- } catch (Exception e) {
- LOG.debug("Could not stop JMX
server.", e);
- }
- }
- }
- if (successfullyStartedServer == null) {
- throw new RuntimeException("Could not start JMX
server on any configured port. Ports: " + portsConfig);
- }
- this.jmxServer = successfullyStartedServer;
+ this.jmxServer = JMXServer.getInstance(portsConfig);
Review comment:
I think so too. good catch.
After the change, the JMXServer static instance will be first created at
ClusterEntrypoint & TaskManagerRunner. we should only invoke the close() method
there.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
