slfan1989 commented on code in PR #8957:
URL: https://github.com/apache/hadoop/pull/8957#discussion_r4500033771
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/TableMapping.java:
##########
@@ -124,6 +125,14 @@ private Map<String, String> load() {
return loadMap;
}
+ /**
+ * Rewrite an IP literal to the form {@link NetUtils#normalizeHostName}
gives lookups,
+ * e.g. 2001:db8::10 to 2001:db8:0:0:0:0:0:10. Other keys are kept as
written.
+ */
+ private static String normalizeIpLiteral(String key) {
+ return InetAddresses.isInetAddress(key) ?
InetAddresses.forString(key).getHostAddress() : key;
Review Comment:
Could we handle normalization failures per entry ?
For `fe80::1%hadoop-no-such-interface`, `isInetAddress()` returns `true`,
but `forString()` throws `IllegalArgumentException` because the interface does
not exist on the local host.
That exception reaches the outer `catch` in `load()` and discards the entire
table, so even valid entries fall back to `/default-rack`.
Could we keep the original key when normalization fails and add a regression
test verifying that the other entries still resolve?
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/TableMapping.java:
##########
@@ -124,6 +125,14 @@ private Map<String, String> load() {
return loadMap;
}
+ /**
+ * Rewrite an IP literal to the form {@link NetUtils#normalizeHostName}
gives lookups,
+ * e.g. 2001:db8::10 to 2001:db8:0:0:0:0:0:10. Other keys are kept as
written.
+ */
+ private static String normalizeIpLiteral(String key) {
+ return InetAddresses.isInetAddress(key) ?
InetAddresses.forString(key).getHostAddress() : key;
Review Comment:
Could we handle normalization failures per entry?
For `fe80::1%hadoop-no-such-interface`, `isInetAddress()` returns `true`,
but `forString()` throws `IllegalArgumentException` because the interface does
not exist on the local host.
That exception reaches the outer `catch` in `load()` and discards the entire
table, so even valid entries fall back to `/default-rack`.
Could we keep the original key when normalization fails and add a regression
test verifying that the other entries still resolve?
--
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]
