[jira] Commented: (HADOOP-5016) FSNamesystem should have an InvalidateBlockMap class to manage blocks scheduled to remove

["Konstantin Shvachko (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/HADOOP-5016?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12665693#action_12665693 ] 

Konstantin Shvachko commented on HADOOP-5016:
---------------------------------------------

I usually am in favor of refactoring. But in this case I don't see the point. {{recentInvalidateSets}} in current code is a map of blocks, which were scheduled for deletion on particular data-nodes. The scheduling is performed by {{ReplicationMonitor}}, which schedules both replications and deletions. So imo {{recentInvalidateSets}} should rather belong to {{ReplicationMonitor}} than to a separate class.
I noticed that synchronization around {{computeInvalidateWork}} has changed. In original code global lock is acquired only for one node, while in the new code all "invalidate work" is performed under the global lock.
I think it is especially bad idea to mix optimizations with refacotring. The code has moved and I cannot really find the optimization snippets.
I would rather postpone this refactoring until we have a clear design of the new {{BlockManager}} as proposed in HADOOP-5015 than doing *gradual changes*.
Do you think it would be reasonable to restrict the scope of this issue only to the 2 optimizations you mentioned?
Minor thing, but I have seen it in other patches too. I thought that we use Sun's code convention, which says that JavaDoc comments should look like this
{code}
/**
 * Description
 * 
 * @return type value
 */
{code}
rather than
{code}
/** Description
 * ......
 */
{code}

> FSNamesystem should have an InvalidateBlockMap class to manage blocks scheduled to remove
> -----------------------------------------------------------------------------------------
>
>                 Key: HADOOP-5016
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5016
>             Project: Hadoop Core
>          Issue Type: Improvement
>          Components: dfs
>            Reporter: Hairong Kuang
>            Assignee: Hairong Kuang
>             Fix For: 0.21.0
>
>         Attachments: invalidateBlocksMap.patch
>
>
> This jira intends to move the code that handles recentInvalideSet to a separate class InvalidateBlockMap.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.