[ https://issues.apache.org/jira/browse/HDFS-11548?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15935131#comment-15935131 ]
Anu Engineer commented on HDFS-11548: ------------------------------------- [~xyao] Thank you for adding this feature. Some of my next patches depend on this feature. Some comments below. * public SCMNodePoolManager(final OzoneConfiguration conf) -- remove double brackets ? {code} if ((scmMetaDataDir == null)) { {code} * TestSCMNodePoolManager::testDefaultNodePool -- we declare nodeCount, but hard code 4 in comparisons. We have this in few places in this test. Can we please use nodeCount ? {code} assertEquals(4, nodesRetrieved.size()); {code} -- Looks like {{Iterators.elementsEqual}} returns a boolean. So we should assertTrue on the return value from those statements. I am getting some failures when I add that assertion. -- Same comments for the other test too. * SCMNodeManager.java : 2 Check Style warnings. -- Unused import - org.apache.hadoop.conf.Configuration. (25:8) -- Empty statement. (177:57) * SCMNodePoolManager.java: Line 127 -- Shouldn't we fail SCM load at this point of time ? {{LOG.error("Loading node pool error " + e);}} * removeNode() : Line 171 {code} if (kData == null) { throw new StorageContainerException("Unable to find the key.", ContainerProtos.Result.NO_SUCH_KEY); } {code} -- I think we should have a new SCM server only exception to propagate this kind of error. The container protos are part of wire protocol that we use to communicate with datanode containers. While the semantics is appropriate here, I am worried that someone looking at logs might get confused that the error came from a datanode. Same for all uses of StorageContainerException in this file. > OZone: SCM: Add node pool management API > ---------------------------------------- > > Key: HDFS-11548 > URL: https://issues.apache.org/jira/browse/HDFS-11548 > Project: Hadoop HDFS > Issue Type: Sub-task > Reporter: Xiaoyu Yao > Assignee: Xiaoyu Yao > Attachments: HDFS-11548-HDFS-7240.001.patch > > > The idea is to group registered nodes into pools of fixed size (say 24 nodes > per pool) so that the container allocation and report can all be handled > independently on a pool basis by SCM. > The initial patch will implement the following Node Pool API that > 1) add node to a node pool > 2) remove a node from a pool > 3) get the pool name that a node belongs to > 4) get all the pool names > 5) get all nodes of a pool > The integration with SCM container allocation can be all nodes in a single > default pool upon registration. We will provide a CLI to manage multiple > pools and support for pool definition file later. -- 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