Ethanlm commented on a change in pull request #3317:
URL: https://github.com/apache/storm/pull/3317#discussion_r468129169
##########
File path: storm-client/src/jvm/org/apache/storm/messaging/netty/Context.java
##########
@@ -67,7 +75,8 @@ public synchronized IConnection bind(String stormId, int
port, IConnectionCallba
@Override
public IConnection connect(String stormId, String host, int port,
AtomicBoolean[] remoteBpStatus) {
return new Client(topoConf, remoteBpStatus, workerEventLoopGroup,
Review comment:
There could be name collisions.
Every time where there is a new connection needs to be established, a new
`Client` is created and the metrics will be registered with the name for
example `send-connection-reconnects-remoteHost1-remotePort1`.
Imaging the remote worker is then rescheduled to another host, say,
host2-port2, then the new metric will be created as
`send-connection-reconnects-remoteHost2-remotePort2` while the old one
`send-connection-reconnects-remoteHost1-remotePort1` still remain in the
metricRegistry.
Then rescheduling happens again, that remote worker is rescheduled back to
host1-port1, a new Client is being created and it tries to register
`send-connection-reconnects-remoteHost1-remotePort1` metric. This is when name
collision happens. Will it cause `IllegalArgumentException` ?
----------------------------------------------------------------
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
