pnowojski commented on a change in pull request #11541:
URL: https://github.com/apache/flink/pull/11541#discussion_r413884448
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/io/network/netty/PartitionRequestClientFactory.java
##########
@@ -81,16 +92,12 @@ NettyPartitionRequestClient
createPartitionRequestClient(ConnectionID connection
Object old = clients.putIfAbsent(connectionId,
connectingChannel);
if (old == null) {
Review comment:
I think I agree with @zhijiangW. I don't get the heaviness argument.
Difference between 256 * 256 lock acquisitions vs current ConcurrentHashMap
accesses (which are not for free) wouldn't be probably measurable. (256 * 256
locks could be acquired in under 0.01s on a single machine, and you are
assuming 256 task managers, so we should be fine even with 100 000s
connections).
But we could easily block other users of this class, and calls like
`destroyPartitionRequestClient` or `closeOpenChannelConnections` do not sound
like should be blocked waiting for lock acquisition during
connections/reconnections
edit: by the last comment I meant that it's probably easier to just leave it
as it is, as clean up might prove to be more challenging than it seems.
----------------------------------------------------------------
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]
