This is an automated email from the ASF dual-hosted git repository.
ZanderXu pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/hadoop.git
The following commit(s) were added to refs/heads/trunk by this push:
new b178b0d654e HDFS-17901. Fix getEligibleChildren not handling
DatanodeInfo in excludedNodes (#8386)
b178b0d654e is described below
commit b178b0d654ea3930e26785b28b793606adc58103
Author: CapMoon <[email protected]>
AuthorDate: Fri May 8 17:40:46 2026 +0800
HDFS-17901. Fix getEligibleChildren not handling DatanodeInfo in
excludedNodes (#8386)
---
.../org/apache/hadoop/net/NetworkTopology.java | 2 +-
.../apache/hadoop/hdfs/net/DFSNetworkTopology.java | 8 ++++
.../hadoop/hdfs/net/TestDFSNetworkTopology.java | 49 ++++++++++++++++++++++
3 files changed, 58 insertions(+), 1 deletion(-)
diff --git
a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetworkTopology.java
b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetworkTopology.java
index 0eeb592e9a7..e6220e03bb6 100644
---
a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetworkTopology.java
+++
b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetworkTopology.java
@@ -51,7 +51,7 @@ public class NetworkTopology {
LoggerFactory.getLogger(NetworkTopology.class);
private static final char PATH_SEPARATOR = '/';
- private static final String PATH_SEPARATOR_STR = "/";
+ protected static final String PATH_SEPARATOR_STR = "/";
private static final String ROOT = "/";
private static final AtomicReference<Random> RANDOM_REF =
new AtomicReference<>();
diff --git
a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/net/DFSNetworkTopology.java
b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/net/DFSNetworkTopology.java
index 66288a749ba..7f9eb0fa973 100644
---
a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/net/DFSNetworkTopology.java
+++
b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/net/DFSNetworkTopology.java
@@ -385,6 +385,14 @@ private ArrayList<DFSTopologyNodeImpl> getEligibleChildren(
} else if (excludedNode instanceof DFSTopologyNodeImpl) {
storageCount -= ((DFSTopologyNodeImpl) excludedNode)
.getSubtreeStorageCount(type);
+ } else if (excludedNode instanceof DatanodeInfo) {
+ String nodeLocation = excludedNode.getNetworkLocation()
+ + NetworkTopology.PATH_SEPARATOR_STR +
excludedNode.getName();
+ DatanodeDescriptor dn = (DatanodeDescriptor)
getNode(nodeLocation);
+ if (dn == null) {
+ continue;
+ }
+ storageCount -= dn.hasStorageType(type) ? 1 : 0;
}
}
}
diff --git
a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/net/TestDFSNetworkTopology.java
b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/net/TestDFSNetworkTopology.java
index 5bcb6ba46ce..d0402c91695 100644
---
a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/net/TestDFSNetworkTopology.java
+++
b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/net/TestDFSNetworkTopology.java
@@ -22,6 +22,7 @@
import org.apache.hadoop.fs.StorageType;
import org.apache.hadoop.hdfs.DFSTestUtil;
import org.apache.hadoop.hdfs.protocol.DatanodeID;
+import org.apache.hadoop.hdfs.protocol.DatanodeInfo;
import org.apache.hadoop.hdfs.protocol.DatanodeInfo.DatanodeInfoBuilder;
import org.apache.hadoop.hdfs.server.blockmanagement.DatanodeDescriptor;
import org.apache.hadoop.hdfs.server.blockmanagement.DatanodeStorageInfo;
@@ -37,6 +38,7 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.Set;
+import java.util.UUID;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -419,6 +421,53 @@ public void
testChooseRandomWithStorageTypeWithExcludedforNullCheck()
assertNotNull(node);
}
+ /**
+ * Test that chooseRandomWithStorageType correctly handles DatanodeInfo
+ * (not DatanodeDescriptor) in the excludedNodes set.
+ * getEligibleChildren did not subtract storageCount for DatanodeInfo
+ * excluded nodes, which could cause the excluded node to be returned
+ * when the random selection falls on the wrong branch.
+ *
+ * Topology:
+ * /d1/r1: nodeA (ARCHIVE) -- the only ARCHIVE under /d1
+ * /d2/r2: nodeB (ARCHIVE) -- the only ARCHIVE under /d2
+ *
+ * Exclude nodeA as a DatanodeInfo with a different UUID, so
+ * HashSet.contains() at rack level won't match by equals().
+ * Without the fix, getEligibleChildren gives /d1 a non-zero weight,
+ * and if selected, nodeA is returned despite being excluded.
+ */
+ @Test
+ public void testChooseRandomWithDatanodeInfoExcluded() {
+ DFSNetworkTopology dfsCluster =
+ DFSNetworkTopology.getInstance(new Configuration());
+ final String[] racks = {"/d1/r1", "/d2/r2"};
+ final String[] hosts = {"hostA", "hostB"};
+ final StorageType[] types = {StorageType.ARCHIVE, StorageType.ARCHIVE};
+ final DatanodeStorageInfo[] storages =
+ DFSTestUtil.createDatanodeStorageInfos(2, racks, hosts, types);
+ DatanodeDescriptor[] dns = DFSTestUtil.toDatanodeDescriptor(storages);
+ dfsCluster.add(dns[0]);
+ dfsCluster.add(dns[1]);
+
+ DatanodeInfo excludedInfo = new DatanodeInfoBuilder()
+ .setNodeID(new DatanodeID(UUID.randomUUID().toString(), dns[0]))
+ .setNetworkLocation(dns[0].getNetworkLocation())
+ .build();
+
+ HashSet<Node> excluded = new HashSet<>();
+ excluded.add(excludedInfo);
+
+ for (int i = 0; i < 100; i++) {
+ Node n = dfsCluster.chooseRandomWithStorageType(
+ "/", null, excluded, StorageType.ARCHIVE);
+ assertNotNull(n, "Should always find a valid ARCHIVE node");
+ DatanodeDescriptor dd = (DatanodeDescriptor) n;
+ assertEquals("hostB", dd.getHostName(),
+ "Should only return hostB since hostA is excluded");
+ }
+ }
+
/**
* This test tests the wrapper method. The wrapper method only takes one
scope
* where if it starts with a ~, it is an excluded scope, and searching always
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
