[ 
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

Reply via email to