Apache9 commented on a change in pull request #2669:
URL: https://github.com/apache/hbase/pull/2669#discussion_r530805655
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionUtils.java
##########
@@ -128,32 +129,17 @@ public static void
setServerSideHConnectionRetriesConfig(final Configuration c,
}
/**
- * Return retires + 1. The returned value will be in range [1,
Integer.MAX_VALUE].
+ * Get a unique key for the rpc stub to the given server.
*/
- static int retries2Attempts(int retries) {
- return Math.max(1, retries == Integer.MAX_VALUE ? Integer.MAX_VALUE :
retries + 1);
+ static String getStubKey(String serviceName, ServerName serverName) {
Review comment:
As said before, I wonder what is the problem if we just use host:port
directly here? In the past I think the problem is that we will not resolve
again when connecting, for now, I think the problem has been solved?
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/AbstractRpcClient.java
##########
@@ -390,8 +391,8 @@ private void onCallFinished(Call call, HBaseRpcController
hrc, InetSocketAddress
}
Call callMethod(final Descriptors.MethodDescriptor md, final
HBaseRpcController hrc,
- final Message param, Message returnType, final User ticket, final
InetSocketAddress addr,
- final RpcCallback<Message> callback) {
+ final Message param, Message returnType, final User ticket,
+ final InetSocketAddress inetAddr, final RpcCallback<Message> callback) {
Review comment:
Checked the code, I think we could avoid creating an InetSocketAddress
everytime here, we just need to change more classes to make use of Address
instead of InetSocketAddress, such as ConnetionId, FailedServers, as well as
the RpcClient interface. And the resolving of the actual address could be
delayed to NettyRpcConnection.connect and
BlockingRpcConnection.setupConnection, where we really want to connect to the
remote side. And once the connection has been established, and it has not been
closed because of error or idle for too long, we do not need to involve
InetSocketAddress again. I think it is OK?
----------------------------------------------------------------
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
