[FLINK-32811][runtime] Add port range support for "taskmanager.data.bind-port" #23183
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ferenc-csaky
force-pushed
the
FLINK-32811
branch
from
60a9177
to
0bc7eb5
Compare
mbalassi
approved these changes
Aug 26, 2023
There was a problem hiding this comment.
I run this locally, it makes sense to add this improvement. It would be great if we could make it explicit in the logs (maybe adding an extra log line) that the following is for the data bind port:
2023-08-26 21:30:45,211 INFO org.apache.flink.runtime.io.network.netty.NettyConfig [] - NettyConfig [server address: localhost/127.0.0.1, server port range: 55000-55100, ssl enabled: false, memory segment size (bytes): 32768, transport type: AUTO, number of server threads: 1 (manual), number of client threads: 1 (manual), server connect backlog: 0 (use Netty's default), client connect timeout (sec): 120, send/receive buffer size (bytes): 0 (use Netty's default)]
2023-08-26 21:30:45,246 INFO org.apache.flink.runtime.io.network.NettyShuffleServiceFactory [] - Created a new FileChannelManager for storing result partitions of BLOCKING shuffles. Used directories:
/var/folders/yh/t9bt8gwj4zsd949jnr55vx980000gn/T/flink-netty-shuffle-9c342614-dc5f-467f-9a9d-1736246bdc62
2023-08-26 21:30:45,280 INFO org.apache.flink.runtime.io.network.buffer.NetworkBufferPool [] - Allocated 128 MB for network buffer pool (number of memory segments: 4096, bytes per segment: 32768).
2023-08-26 21:30:45,289 INFO org.apache.flink.runtime.io.network.NettyShuffleEnvironment [] - Starting the network environment and its components.
2023-08-26 21:30:45,309 INFO org.apache.flink.runtime.io.network.netty.NettyClient [] - Transport type 'auto': using NIO.
2023-08-26 21:30:45,310 INFO org.apache.flink.runtime.io.network.netty.NettyClient [] - Successful initialization (took 20 ms).
2023-08-26 21:30:45,312 INFO org.apache.flink.runtime.io.network.netty.NettyServer [] - Transport type 'auto': using NIO.
2023-08-26 21:30:45,341 INFO org.apache.flink.runtime.io.network.netty.NettyServer [] - Successful initialization (took 30 ms). Listening on SocketAddress /127.0.0.1:55000.
| bootstrap.localAddress(config.getServerAddress(), port); | ||
| try { | ||
| bindFuture = bootstrap.bind().syncUninterruptibly(); | ||
| } catch (Exception e) { |
There was a problem hiding this comment.
Can we be more specific than just Exception here?
There was a problem hiding this comment.
Probably yes, but Netty wraps and throws any kind of exception via FailedChannelFuture, which is not specified really well, so not without a lot more digging.
Furthermore I copied this "pattern" from the codebase, so I considered it good enough:
I'm not sure where you think it would be appropriate. I think Netty specific classes are too general to add that info explicitly, what can be done easily is to highlight the listening port explicitly when the connection is successfully established (last line is the newly added log): The other option would be to log it before the process IMO, but since the whole thing is started by the WDYT? |
ferenc-csaky
force-pushed
the
FLINK-32811
branch
from
0bc7eb5
to
7865b97
Compare
|
Thanks, this makes sense imho. Unless there are any objections I will merge this tomorrow. |
ferenc-csaky
force-pushed
the
FLINK-32811
branch
from
7865b97
to
c11c203
Compare
gaborgsomogyi
approved these changes
Aug 28, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is the purpose of the change
Adds port range support for
taskmanager.data.bind-port, which can be helpful in a restrictive network setup.Brief change log
taskmanager.data.bind-portopion type fromIntegertoString.PortRangeutility to be able to carry the port range string and its generated iterator together.NettyConfigto handle aPortRangeinstead of aportas the port of the server.NettyServerto be able to initialize itself from aPortRange.Verifying this change
This change added tests and can be verified as follows:
NettyServerFromPortRangeTestto test Netty server init from a port range.taskmanager.data.bind-portto a port range (e.g. 55000-55100) whiletaskmanager.data.portis set to0, then TM should bind to the first available port from the given range both locally and externally.Does this pull request potentially affect one of the following parts:
@Public(Evolving): noDocumentation