azagrebin commented on a change in pull request #10161: [FLINK-13986][runtime]
Clean up legacy code for FLIP-49.
URL: https://github.com/apache/flink/pull/10161#discussion_r347771505
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/taskmanager/NettyShuffleEnvironmentConfiguration.java
##########
@@ -449,35 +455,55 @@ private static int getDataport(Configuration
configuration) {
* Calculates the number of network buffers based on configuration and
jvm heap size.
*
* @param configuration configuration object
- * @param maxJvmHeapMemory the maximum JVM heap size (in bytes)
+ * @param shuffleMemorySize the size of memory reserved for shuffle
environment
+ * @param pageSize size of memory segment
* @return the number of network buffers
*/
@SuppressWarnings("deprecation")
- private static int calculateNumberOfNetworkBuffers(Configuration
configuration, long maxJvmHeapMemory) {
- final int numberOfNetworkBuffers;
- if (!hasNewNetworkConfig(configuration)) {
- // fallback: number of network buffers
- numberOfNetworkBuffers =
configuration.getInteger(NettyShuffleEnvironmentOptions.NETWORK_NUM_BUFFERS);
+ private static int calculateNumberOfNetworkBuffers(
+ Configuration configuration,
+ MemorySize shuffleMemorySize,
+ int pageSize,
+ int numberOfNettyArenas) {
- checkOldNetworkConfig(numberOfNetworkBuffers);
- } else {
- if
(configuration.contains(NettyShuffleEnvironmentOptions.NETWORK_NUM_BUFFERS)) {
- LOG.info("Ignoring old (but still present)
network buffer configuration via {}.",
-
NettyShuffleEnvironmentOptions.NETWORK_NUM_BUFFERS.key());
- }
+ final int numberOfNetworkBuffers =
calculateNumberOfNetworkBuffers(shuffleMemorySize.getBytes(), pageSize,
numberOfNettyArenas);
Review comment:
nit: I would rather have one `calculateNumberOfNetworkBuffers`:
```
logIfIgnoringOldConfigs(configuration);
long nettyArenasSizeBytes = calcAndCheckNettyArenasSize(...);
long numberOfNetworkBuffersLong = ....;
if (numberOfNetworkBuffersLong > Integer.MAX_VALUE) {....}
return (int) numberOfNetworkBuffersLong;
```
This way we would address concerns of the respective level of abstraction.
----------------------------------------------------------------
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
