smengcl commented on code in PR #8852:
URL: https://github.com/apache/hadoop/pull/8852#discussion_r4343233925
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java:
##########
@@ -2264,7 +2264,11 @@ private List<DatanodeDescriptor>
getDnDescriptorsFromIpAddr(List<String> nodes)
if (StringUtils.isBlank(node) || !node.contains(IP_PORT_SEPARATOR)) {
continue;
}
- String ipAddr = node.split(IP_PORT_SEPARATOR)[0];
+ // Port is after the last ':'; IPv6 hosts may be bracketed.
+ String ipAddr = node.substring(0, node.lastIndexOf(IP_PORT_SEPARATOR));
+ if (ipAddr.startsWith("[") && ipAddr.endsWith("]")) {
+ ipAddr = ipAddr.substring(1, ipAddr.length() - 1);
+ }
Review Comment:
Slow peer keys always come from `DatanodeID#getInfoAddr()` or
`getInfoSecureAddr()`, which always append the port, so a bare IPv6 literal
never reaches this method. If one did, the extracted host would not match any
registered node and the key would be skipped, which is the same outcome as for
any other malformed key (and the same as before this change). Leaving the guard
as is to keep the patch narrow.
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestDatanodeManager.java:
##########
@@ -195,6 +197,42 @@ public void testHost2NodeMapCorrectAfterReregister()
assertNotNull(dm.getDatanodeByHostName(hostnameNew), "should be a node
with new hostname");
}
+ /**
+ * Slow peer reports keyed by IPv4, IPv6 and bracketed IPv6 info addresses
+ * must all resolve to their registered datanodes.
+ */
+ @Test
+ public void testSlowPeersResolvedForIpv4AndIpv6() throws IOException {
+ FSNamesystem fsn = Mockito.mock(FSNamesystem.class);
+ Mockito.when(fsn.hasWriteLock()).thenReturn(true);
+ Mockito.when(fsn.hasWriteLock(RwLockMode.BM)).thenReturn(true);
+ Configuration conf = new Configuration();
+ conf.setBoolean(DFSConfigKeys.DFS_DATANODE_PEER_STATS_ENABLED_KEY, true);
+ DatanodeManager dm = mockDatanodeManager(fsn, conf);
+ try {
+ String[] ips = {"1.2.3.4", "2001:db8::1", "2001:db8::2", "5.6.7.8"};
+ for (int i = 0; i < ips.length; i++) {
+ dm.registerDatanode(new DatanodeRegistration(
+ new DatanodeID(ips[i], "", "uuid" + i, 9866, 9864, 0, 9867), null,
null, "version"));
+ }
+ DatanodeDescriptor ipv4 = dm.getDatanodeByHost(ips[0]);
+ DatanodeDescriptor ipv6 = dm.getDatanodeByHost(ips[1]);
+ DatanodeDescriptor ipv6Bracketed = dm.getDatanodeByHost(ips[2]);
+
+ SlowPeerTracker tracker = dm.getSlowPeerTracker();
+ OutlierMetrics metrics = new OutlierMetrics(0.0, 0.0, 0.0, 5.0);
+ tracker.addReport(ipv4.getInfoAddr(), ips[3], metrics);
+ tracker.addReport(ipv6.getInfoAddr(), ips[3], metrics);
Review Comment:
Good point. On trunk `getInfoAddr()` is unbracketed, but HDFS-18010 will
bracket it, and then both cases would be bracketed. The test now spells out the
three keys (`1.2.3.4:9864`, `2001:db8::1:9864`, `[2001:db8::2]:9864`) so both
IPv6 forms stay covered.
##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/blockmanagement/TestDatanodeManager.java:
##########
@@ -195,6 +197,42 @@ public void testHost2NodeMapCorrectAfterReregister()
assertNotNull(dm.getDatanodeByHostName(hostnameNew), "should be a node
with new hostname");
}
+ /**
+ * Slow peer reports keyed by IPv4, IPv6 and bracketed IPv6 info addresses
+ * must all resolve to their registered datanodes.
+ */
+ @Test
+ public void testSlowPeersResolvedForIpv4AndIpv6() throws IOException {
+ FSNamesystem fsn = Mockito.mock(FSNamesystem.class);
+ Mockito.when(fsn.hasWriteLock()).thenReturn(true);
+ Mockito.when(fsn.hasWriteLock(RwLockMode.BM)).thenReturn(true);
+ Configuration conf = new Configuration();
+ conf.setBoolean(DFSConfigKeys.DFS_DATANODE_PEER_STATS_ENABLED_KEY, true);
+ DatanodeManager dm = mockDatanodeManager(fsn, conf);
+ try {
+ String[] ips = {"1.2.3.4", "2001:db8::1", "2001:db8::2", "5.6.7.8"};
+ for (int i = 0; i < ips.length; i++) {
+ dm.registerDatanode(new DatanodeRegistration(
+ new DatanodeID(ips[i], "", "uuid" + i, 9866, 9864, 0, 9867), null,
null, "version"));
+ }
+ DatanodeDescriptor ipv4 = dm.getDatanodeByHost(ips[0]);
+ DatanodeDescriptor ipv6 = dm.getDatanodeByHost(ips[1]);
+ DatanodeDescriptor ipv6Bracketed = dm.getDatanodeByHost(ips[2]);
+
+ SlowPeerTracker tracker = dm.getSlowPeerTracker();
+ OutlierMetrics metrics = new OutlierMetrics(0.0, 0.0, 0.0, 5.0);
+ tracker.addReport(ipv4.getInfoAddr(), ips[3], metrics);
+ tracker.addReport(ipv6.getInfoAddr(), ips[3], metrics);
+ tracker.addReport(NetUtils.getHostPortString(ips[2],
ipv6Bracketed.getInfoPort()), ips[3],
+ metrics);
+
+ assertThat(dm.getAllSlowDataNodes()).containsExactlyInAnyOrder(ipv4,
ipv6, ipv6Bracketed);
Review Comment:
Renamed to `dnIpv4`, `dnIpv6PlainKey` and `dnIpv6BracketedKey` so the names
describe the key form each node is reported with.
--
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]
