ptlrs commented on code in PR #7495:
URL: https://github.com/apache/ozone/pull/7495#discussion_r1990762098
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/helpers/ContainerUtils.java:
##########
@@ -174,20 +174,25 @@ public static synchronized DatanodeDetails
readDatanodeDetailsFrom(File path)
if (!path.exists()) {
throw new IOException("Datanode ID file not found.");
}
+ DatanodeDetails datanodeDetails;
try {
- return DatanodeIdYaml.readDatanodeIdFile(path);
+ datanodeDetails = DatanodeIdYaml.readDatanodeIdFile(path);
} catch (IOException e) {
LOG.warn("Error loading DatanodeDetails yaml from {}",
path.getAbsolutePath(), e);
// Try to load as protobuf before giving up
try (FileInputStream in = new FileInputStream(path)) {
- return DatanodeDetails.getFromProtoBuf(
+ datanodeDetails = DatanodeDetails.getFromProtoBuf(
HddsProtos.DatanodeDetailsProto.parseFrom(in));
} catch (IOException io) {
throw new IOException("Failed to parse DatanodeDetails from "
+ path.getAbsolutePath(), io);
}
}
+
+ datanodeDetails.validateDatanodeIpAddress();
Review Comment:
@adoroszlai I agree that `HddsDatanodeService` will overwrite the ip address
in this instance and render the point of validating the IP address moot. Also,
since `HddsDatanodeService` is setting the IP address after reading the file,
there is likely no need of any extra validation logic there as that IP address
is expected to be valid.
I am however not convinced that IP validation shouldn't be called when
reading the file.
The question is about expectations from the `readDatanodeDetailsFrom` method
in any different situation. Is it valid to expect a `DatanodeDetails` object
from `readDatanodeDetailsFrom()` where the IP address and hostname do not match?
At the least, can we set the IP address in such a situation to be `null`
rather than an incorrect value? We could update the `DatanodeDetails` class so
that any `get` access to a null IP address would first and only once be
resolved to a valid IP address based on the hostname.
--
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...@ozone.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@ozone.apache.org
For additional commands, e-mail: issues-h...@ozone.apache.org
