zentol commented on a change in pull request #13163:
URL: https://github.com/apache/flink/pull/13163#discussion_r476645574
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/management/JMXService.java
##########
@@ -85,6 +86,9 @@ private static JMXServer
startJMXServerWithPortRanges(Iterator<Integer> ports) {
while (ports.hasNext() && successfullyStartedServer == null) {
JMXServer server = new JMXServer();
int port = ports.next();
+ if (port == 0) { // try poke with a random port when
port is set to zero
Review comment:
> it would be better to do this conversion in the realm of the JMXServer
Agreed.
> Do we know that the startup period will be super long if we don't do this?
It probably isn't long. In the port-probing version, you are essentially
entering a race condition between all processes on this node. I would consider
it unlikely you will have more than a hundred of TaskExecutors on a given node,
and only at that point should it be relevant in the first place.
I only suggested the special iterator to workaround the worst case scenario,
something like the first 1000 ports already being allocated, where the current
approach would likely fair miserably.
But there clear downsides to having this be applied in general to all users
of the NetUtils; it is _very_ convenient to define a port-range and be pretty
much guaranteed that all processes start at the front, and form a continuous
port sequence if present on a single node.
----------------------------------------------------------------
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:
us...@infra.apache.org
