hadoop-common-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Konstantin Shvachko (JIRA)" <j...@apache.org>
Subject [jira] Commented: (HADOOP-1272) Extract InnerClasses from FSNamesystem into separate classes
Date Wed, 25 Apr 2007 00:44:15 GMT

    [ https://issues.apache.org/jira/browse/HADOOP-1272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12491498
] 

Konstantin Shvachko commented on HADOOP-1272:
---------------------------------------------

I looked more at the SafeModeInfo separation project.

# Separation is achieved by introducing code replication, which is rather dangerous,
since one can forget to include the replicated part in one of the places it should be.
Especial in case of SafeMode where miscalculation of blocks or setting a member value
can lead to loosing data blocks.
Example:
Before your patch SafeModeInfo.leave() would set safeMode = null;
Now you cannot do the same inside leave() so you should set it to null whenever leave()
is called in 3 places, namely in
leaveSafeMode(), checkMode(), and in SafeModeMonitor.run()
You did it in 2 places out of 3.
Same thing with reporting of Network topology and UnderReplicatedBlocks
NameNode.stateChangeLog.info(.......
Your patch does not report those when you leave safe mode manually.

# You completely removed the assert, which checks whether the block counting isConsistent()
because it uses FSNamesystem members.
This is the only way to check whether the block counting is right, so now our tests will not
catch it.
I've seen it actually working and catching wrong doings :)

# SafeMode is not on the critical path for fine grain locking imo.
It does not have big maps or long methods, and it works mainly during cluster startup.

# JavaDoc @links and @see tags need to be updated.

I think we should keep SafeModeInfo as an inner class for the time being.
I am not saying it does not make sense to separate it, just that it is not as straightforward
as other classes.

Everything else looks good to me.

> Extract InnerClasses from FSNamesystem into separate classes
> ------------------------------------------------------------
>
>                 Key: HADOOP-1272
>                 URL: https://issues.apache.org/jira/browse/HADOOP-1272
>             Project: Hadoop
>          Issue Type: Bug
>          Components: dfs
>            Reporter: dhruba borthakur
>         Assigned To: dhruba borthakur
>         Attachments: innerclasses3.patch
>
>
> This will make the code cleaner. Also, it leads itself to a cleaner and easily understandable
finer-grain locking model.

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


Mime
View raw message