[ https://issues.apache.org/jira/browse/HDFS-15655?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17243784#comment-17243784 ]
Ayush Saxena commented on HDFS-15655: ------------------------------------- Thanx [~hadoop_yangyun] for the patch. Looks good overall. Some comments : {code:java} + public static final String DFS_NAMENODE_HOT_BLOCK_TIME_INTERVAL_KEY + = "dfs.namenode.hot.block.time.interval"; {code} * Shouldn't the config be DFS_BALANCER* similar to {{DFS_BALANCER_GETBLOCKS_MIN_BLOCK_SIZE_KEY}} which too is is used as part of the same method, this config too seems quite balancer specific {code:java} INodeFile iFile = ((INode)getBlockCollection(blockInfo)).asFile(); {code} * Any reasons why you aren't typecasting directly to INodeFile? {code:java} private boolean isHotBlock(BlockInfo blockInfo, long time) { 1641 INodeFile iFile = ((INode)getBlockCollection(blockInfo)).asFile(); 1642 if(iFile == null) { 1643 return false; {code} * Get into this block only when the time is greater than 0, Add a if check as the first line of the method, if the value is set to 0, no need of these conversions, a value of 0 should set this feature off, and shouldn't impact in any way. {code:java} - <name>dfs.datanode.same-disk-tiering.enabled</name> - <value>false</value> + <name>dfs.namenode.hot.block.time.interval"</name> + <value>0</value> {code} * Why have you removed {{dfs.datanode.same-disk-tiering.enabled}}, Just add a new conf without bothering anything else, and if possible please add support for time units as well for this conf. Couldn't check the test in detail, but : {code:java} + Thread.sleep(hotInterval); {code} * This sleep can cause problems later, can we get away of this sleep and try some mocking, or if that too isn't the case, then something like {{GenericTestUtils.waitFor}} should be better than having an explicit sleep. {code:java} 1705 for(int i = 0; i < pending.size() && totalSize < size; i++) { 1706 curBlock = pending.get(i); {code} * Can you drop a comment here, explaining what it does, Just like if the total size isn't achieved, trying to add the excluded cold blocks or something like that. * Give a check to the Balancer doc as well, Add the appropriate details there as well. > Add option to make balancer prefer to get cold blocks > ----------------------------------------------------- > > Key: HDFS-15655 > URL: https://issues.apache.org/jira/browse/HDFS-15655 > Project: Hadoop HDFS > Issue Type: Improvement > Components: balancer & mover > Reporter: Yang Yun > Assignee: Yang Yun > Priority: Minor > Attachments: HDFS-15655.001.patch, HDFS-15655.002.patch, > HDFS-15655.003.patch, HDFS-15655.004.patch > > > We met two issues when using balancer. > # Moving hot files may cause failing of dfsclient reading. > # Some blocks of temporary files are moved and they are deleted soon. > Add a config key " dfs.namenode.hot.block.time.interval", the balancer > prefer to get the blocks which are belong to the cold files created before > this time period. > Also add a option "-hotBlockTimeInterval" to the command line of balancer for > inputting. -- This message was sent by Atlassian Jira (v8.3.4#803005) --------------------------------------------------------------------- To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org