[ 
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 &amp; 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

Reply via email to