sokui commented on code in PR #3186:
URL: https://github.com/apache/ozone/pull/3186#discussion_r902159679
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java:
##########
@@ -362,33 +362,33 @@ public RegisteredCommand register(
.build();
}
+ InetAddress dnAddress = Server.getRemoteIp();
+ if (dnAddress != null) {
+ // Mostly called inside an RPC, update ip
+ datanodeDetails.setIpAddress(dnAddress.getHostAddress());
+ }
+
+ String dnsName;
+ String networkLocation;
+ datanodeDetails.setNetworkName(datanodeDetails.getUuidString());
+ if (useHostname) {
+ dnsName = datanodeDetails.getHostName();
+ } else {
+ dnsName = datanodeDetails.getIpAddress();
+ }
+ networkLocation = nodeResolve(dnsName);
+ if (networkLocation != null) {
+ datanodeDetails.setNetworkLocation(networkLocation);
+ }
+
if (!isNodeRegistered(datanodeDetails)) {
- InetAddress dnAddress = Server.getRemoteIp();
- if (dnAddress != null) {
- // Mostly called inside an RPC, update ip and peer hostname
- datanodeDetails.setHostName(dnAddress.getHostName());
Review Comment:
Hi @GeorgeJahad ,
To not change the old code path, I added the if condition: when
`useHostname` is true, we will not setHostname, but when it is false (old
code), we keep the old logic which set the hostname. There are two things I
want to explain:
1. When datanode fist register with scm, the `datanodeDetails` already
contains hostName. Here the code we are talking about is just to reset the
`datanodeDetails.hostName`. So the code you listed above won't return null if
we remove this line of code
`datanodeDetails.setHostName(dnAddress.getHostName());`.
2. Why it doesn't work when we reset `datanodeDetails.hostName` when
`useHostname` is true? this is because in k8s, when datanode first registered
with scm, `dnAddress.getHostName()` may return IP instead of hostName (maybe
because of k8s DNS lookup service delay, I am not exactly sure). this will
result in the IP instead of hostName is used in datanode Ratis communication
for Pipelines. When datanode gets restarted with different IP, then the Ratis
communication with old IP throws the HostNotFoundException. But if we remove
this line, then we are sure that the `datanodeDetails.hostName` always
contains the hostname instead of the IP. So it won't have the Ratis
communication problem.
This is the whole story. That's why now I keep the old code path same, but
if `useHostname` is true, we won't do
`datanodeDetails.setHostName(dnAddress.getHostName());` in the register
process. Please let me know if it makes sense to you.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
