New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FLINK-4087] [metrics] Improved JMX port handling #2145
Conversation
this.reporter = new JMXReporter(); | ||
this.reporter.open(reporterConfig); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is also the catch clause of the else branch where we create a JMXReporter
, but we never call the open
method there. I think we should add a proper open
call there as well.
Good work @zentol. I mostly had general questions for my understanding. Before merging, I think it would be good to add a JMXReporter test which checks that we can start multiple JMXReporter on the same machine and that we can query them. |
-refactor JMXReporter start into separate method -removed all jmx-related JVM arguments -use NetUtils.getPortRangeFromString -refactored JMXServer to be instantiated once
@tillrohrmann I've addressed your comments. |
Excellent @zentol :-) +1 for merging. |
merging |
This PR modifies the JMX port handling.
Instead of setting a specific configured port on JVM start-up we use an ephemeral port for the JVM's JMXServer.
Inside the JVM we then spin-up another JMXServer which users can connect to. This JMXServer picks a port from a configured port range; iterating from start to end until it finds an open one.