[ https://issues.apache.org/jira/browse/HDFS-10897?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15524153#comment-15524153 ]
Xiaoyu Yao commented on HDFS-10897: ----------------------------------- Thanks [~anu] for working on this and post the patch. I took a look at the patch and it looks good to me overall. Below are my early feedback on the code. I will post my comments on unit tests later. 1. NodeManager.java NIT: I see we have registerDataeNode/removeDataNode and getNodeCount/getAllNodes(). Can we have a consistent name, e.g., all with/wo <Data>. 2. SCMNodeManager.java typo in comments: stale informatio. Can we use the wrapper from HadoopExecutors? {code} executorService = Executors.newScheduledThreadPool(1, 131 new ThreadFactoryBuilder().setDaemon(true) 132 .setNameFormat("SCM Heartbeat Processing Thread - %d").build()); {code} updateHeartbeat() Can you add comments to the synchronization assumption from the caller if any similar to registerDatanode()? removeDatanode() : empty overwrite implementation. Either add a TODO: or remove it if it is not needed. getNodes() : declare "throws IllegalArgumentException" > Ozone: SCM: Add NodeManager > --------------------------- > > Key: HDFS-10897 > URL: https://issues.apache.org/jira/browse/HDFS-10897 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: ozone > Affects Versions: HDFS-7240 > Reporter: Anu Engineer > Assignee: Anu Engineer > Attachments: HDFS-10897-HDFS-7240.001.patch, > HDFS-10897-HDFS-7240.002.patch > > > Add a nodeManager class that will be used by Storage Controller Manager > eventually. -- This message was sent by Atlassian JIRA (v6.3.4#6332) --------------------------------------------------------------------- To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org