[ https://issues.apache.org/jira/browse/HDFS-11482?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15902875#comment-15902875 ]
Yiqun Lin commented on HDFS-11482: ---------------------------------- Thanks [~vagarychen] for the explanation and thanks for updating the patch. The latest patch looks almost good now. Only three minor comments from me: * Would you use {{RANDOM}} to replace {{r}}? {code} public class DFSNetworkTopology extends NetworkTopology { + + private static final Random r = new Random(); {code} * This position of the following comment seems not correct in the latest patch, it should be updated like following: {code} + } else { // the children are inner nodes + ArrayList<DFSTopologyNodeImpl> candidates = + getEligibleChildren(root, excludeRoot, type); + if (candidates.size() == 0) { + return null; + } {code} * Can you please add one more test case with the scope that start with {{"~"}}? This new test is used to test the method {{public Node chooseRandomWithStorageType(final String scope, final Collection<Node> excludedNodes, StorageType type)}}. Now I see this test is missing. {quote} I think we can reduce iteration e.g in getSubtreeStorageCount by maintaining more aggregate state at... {quote} BTW, also +1 for this idea. > Add storage type demand to into DFSNetworkTopology#chooseRandom > --------------------------------------------------------------- > > Key: HDFS-11482 > URL: https://issues.apache.org/jira/browse/HDFS-11482 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: namenode > Reporter: Chen Liang > Assignee: Chen Liang > Attachments: HDFS-11482.001.patch, HDFS-11482.002.patch, > HDFS-11482.003.patch, HDFS-11482.004.patch, HDFS-11482.005.patch, > HDFS-11482.006.patch > > > HDFS-11450 adds storage type info into network topology, with on this info we > may change chooseRandom to take storage type requirement as parameter, only > checking subtrees required storage type available. This way we avoid blindly > picking up nodes that are not applicable. -- This message was sent by Atlassian JIRA (v6.3.15#6346) --------------------------------------------------------------------- To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org