Apache9 commented on code in PR #5688:
URL: https://github.com/apache/hbase/pull/5688#discussion_r1501771164
##########
hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/NettyRpcConnection.java:
##########
@@ -252,35 +268,99 @@ public void operationComplete(Future<Boolean> future)
throws Exception {
p.remove(NettyHBaseRpcConnectionHeaderHandler.class);
// don't send connection header,
NettyHBaseRpcConnectionHeaderHandler
// sent it already
- established(ch);
+ saslEstablished(ch, serverPrincipal);
} else {
final Throwable error = future.cause();
scheduleRelogin(error);
- failInit(ch, toIOE(error));
+ saslFailInit(ch, serverPrincipal, toIOE(error));
}
}
});
} else {
// send the connection header to server
ch.write(connectionHeaderWithLength.retainedDuplicate());
- established(ch);
+ saslEstablished(ch, serverPrincipal);
}
} else {
final Throwable error = future.cause();
scheduleRelogin(error);
- failInit(ch, toIOE(error));
+ saslFailInit(ch, serverPrincipal, toIOE(error));
}
}
});
}
- private void getConnectionRegistry(Channel ch) throws IOException {
- established(ch);
- NettyFutureUtils.safeWriteAndFlush(ch,
- Unpooled.directBuffer(6).writeBytes(RpcClient.REGISTRY_PREAMBLE_HEADER));
+ private void getConnectionRegistry(Channel ch, Call connectionRegistryCall)
throws IOException {
+ assert eventLoop.inEventLoop();
+ PreambleCallHandler.setup(ch.pipeline(), rpcClient.readTO, this,
+ RpcClient.REGISTRY_PREAMBLE_HEADER, connectionRegistryCall);
}
- private void connect() throws UnknownHostException {
+ private void onSecurityPreambleError(Channel ch, Set<String>
serverPrincipals,
+ IOException error) {
+ assert eventLoop.inEventLoop();
+ LOG.debug("Error when trying to do a security preamble call to {}",
remoteId.address, error);
+ if (ConnectionUtils.isUnexpectedPreambleHeaderException(error)) {
+ // this means we are connecting to an old server which does not support
the security
+ // preamble call, so we should fallback to randomly select a principal
to use
+ // TODO: find a way to reconnect without failing all the pending calls,
for now, when we
+ // reach here, shutdown should have already been scheduled
+ return;
Review Comment:
I've sent a discuss email to the dev list, there is no response so I just
choose the easiest way, just do not implement the fallback logic. In our
current architecture it really not easy to implement the logic here, as said in
the comment, when we reach here, the connection should have already been closed
because server will close the connection immediately when preamble header
mismatches. And at client side, all the calls are associated with a connection,
so closing a connection means failing these calls.
https://lists.apache.org/thread/81p7f5f5lrrqh1v51mvct43spx8l0rhc
--
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.
To unsubscribe, e-mail: issues-unsubscr...@hbase.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
