You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-dev@hadoop.apache.org by "dhruba borthakur (JIRA)" <ji...@apache.org> on 2007/04/19 23:05:15 UTC
[jira] Created: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
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
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.
[jira] Updated: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
Posted by "dhruba borthakur (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1272?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
dhruba borthakur updated HADOOP-1272:
-------------------------------------
Attachment: (was: innerclasses3.patch)
> 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: innerclasses4.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.
[jira] Commented: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12491362 ]
Doug Cutting commented on HADOOP-1272:
--------------------------------------
bq. Once I am done with this splitting, then I will open another JIRA issue to reorder files into packages.
Why not open that issue now, linking it to this?
> 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: innerclasses2.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.
[jira] Updated: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
Posted by "dhruba borthakur (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1272?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
dhruba borthakur updated HADOOP-1272:
-------------------------------------
Attachment: (was: innerclasses.patch)
> 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
>
> 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.
[jira] Commented: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
Posted by "Andrzej Bialecki (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12490286 ]
Andrzej Bialecki commented on HADOOP-1272:
-------------------------------------------
Some renaming will be necessary, though, to avoid meaningless and conflicting names - e.g. SequenceFile.Reader -> SequenceFileReader, MapFile.Reader -> MapFileReader. Other inner classes with unique names could be extracted as is. There are some cases of doubly nested classes, e.g. FSNamesystem.ReplicationTargetChooser.NotEnoughReplicasException, SequenceFile.Sorter.MergeQueue, etc ... In such cases I propose to extract only the first level, to avoid exploding the parent package namespace with classes that are unusable outside their current parent class.
For that matter, perhaps in more complex cases, such as SequenceFile / MapFile / ArrayFile and associated sorters, or the family of Writable classes, it would be better to create new sub-packages? This is how it's done with compression-related classes.
> 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: innerclasses.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.
[jira] Commented: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
Posted by "Tom White (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12491181 ]
Tom White commented on HADOOP-1272:
-----------------------------------
bq. Arguably HDFS should move into a set of packages under org.apache.hadoop.dfs
Or under org.apache.hadoop.fs.dfs, like the S3 code.
> 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: innerclasses2.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.
[jira] Updated: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
Posted by "dhruba borthakur (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1272?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
dhruba borthakur updated HADOOP-1272:
-------------------------------------
Attachment: innerclasses7.patch
Ok, removed SafeModeInfo from these changes.
Konstantin: can you pl review this one? Thanks.
> 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: innerclasses4.patch, innerclasses5.patch, innerclasses6.patch, innerclasses7.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.
[jira] Updated: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
Posted by "Tom White (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1272?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Tom White updated HADOOP-1272:
------------------------------
Resolution: Fixed
Fix Version/s: 0.13.0
Status: Resolved (was: Patch Available)
I've just committed this. Thanks Dhruba!
> 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
> Fix For: 0.13.0
>
> Attachments: innerclasses4.patch, innerclasses5.patch, innerclasses6.patch, innerclasses7.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.
[jira] Updated: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
Posted by "dhruba borthakur (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1272?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
dhruba borthakur updated HADOOP-1272:
-------------------------------------
Attachment: innerclasses6.patch
Removed whitespaces from FileUnderConstruction.java.
> 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: innerclasses4.patch, innerclasses5.patch, innerclasses6.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.
[jira] Commented: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12491358 ]
Doug Cutting commented on HADOOP-1272:
--------------------------------------
bq. Or under org.apache.hadoop.fs.dfs, like the S3 code.
Oops. That's what I meant. Thanks for clarifying!
> 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: innerclasses2.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.
[jira] Updated: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
Posted by "dhruba borthakur (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1272?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
dhruba borthakur updated HADOOP-1272:
-------------------------------------
Attachment: innerclasses5.patch
The same patch as the earlier one with most checkstyle warnings fixed.
> 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: innerclasses4.patch, innerclasses5.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.
[jira] Updated: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
Posted by "dhruba borthakur (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1272?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
dhruba borthakur updated HADOOP-1272:
-------------------------------------
Status: Patch Available (was: Open)
> 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: innerclasses4.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.
[jira] Updated: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
Posted by "dhruba borthakur (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1272?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
dhruba borthakur updated HADOOP-1272:
-------------------------------------
Attachment: (was: innerclasses2.patch)
> 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
>
> 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.
[jira] Commented: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
Posted by "Konstantin Shvachko (JIRA)" <ji...@apache.org>.
[ 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.
[jira] Updated: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
Posted by "dhruba borthakur (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1272?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
dhruba borthakur updated HADOOP-1272:
-------------------------------------
Attachment: innerclasses.patch
Extracts SafeModeInfo, UnderReplciatedBlocks and FileUnderConstruction into separate files.
> 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: innerclasses.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.
[jira] Commented: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12490470 ]
Doug Cutting commented on HADOOP-1272:
--------------------------------------
> org.apache.hadoop.dfs.servlets
Generally packages should be based on what the code does, not how it does it. So I wouldn't think that the servlets should be in a sub-package. Rather, the namenode, datanode and client might be in separate packages.
> In general we should not multiply inner classes but rather create separate ones.
Inner classes are fine so long as they don't get too big.
> 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: innerclasses.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.
[jira] Commented: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
Posted by "dhruba borthakur (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12491071 ]
dhruba borthakur commented on HADOOP-1272:
------------------------------------------
Thanks for all the comments. In the first pass, I plan on breaking up FSNamesystem into the classes that Konstantin has suggested. I agree that, this at first sight, might appear as if we are moving clutter from inside FSNamesystem into the file-names in the dfs package. I would still like to split out the Innerclasses from FSnamesystem into their own files. This helps me in implementing file-grain lock per data structure (hadoop-1269) in a much cleaner way. Once I am done with this splitting, then I will open another JIRA issue to reorder files into packages.
Doug: please let me know if this sounds reasonable.
> 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: innerclasses.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.
[jira] Commented: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
Posted by "Konstantin Shvachko (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12491456 ]
Konstantin Shvachko commented on HADOOP-1272:
---------------------------------------------
- removeNeededReplications
should take 2 more parameters rather than recalculating them
does not need to be synchronized
- updateNeededReplications
should take new and old replications rather than deltas
should not recalculate replication if it is already known
- I don't think there is a need for removeNeededReplications and updateNeededReplications
One can just use neededReplications.remove and neededReplications.update
the way it used to be.
- UnderReplicatedBlocks:
3 import warnings
- SafeModeInfo
2 import warnings
I really didn't like how the code got more complicated, because you need to startSafeModeMonitor()
in 3 places rather than one, and stateChangeLog() is printing only in one place instead of 3.
I'd just keep SafeModeInfo as an inner class.
- FileUnderConstruction
1 import warning
- Import warnings all new files.
- Why GetImageServlet and FsckServlet are not separated?
I thought nobody argued about keeping them inner as long as they are in the same package.
> 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: innerclasses2.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.
[jira] Commented: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
Posted by "Konstantin Shvachko (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12492822 ]
Konstantin Shvachko commented on HADOOP-1272:
---------------------------------------------
+1 Reviewed the patch. Looks good.
> 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: innerclasses4.patch, innerclasses5.patch, innerclasses6.patch, innerclasses7.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.
[jira] Updated: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
Posted by "dhruba borthakur (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1272?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
dhruba borthakur updated HADOOP-1272:
-------------------------------------
Attachment: innerclasses4.patch
Hi Konstantin, thanks for the feedback. I agree that moving the SafeModeInfo out of the FSNamesystem file makes the code layout has become a little more involved that earlier. But the assumed simplicity that was earlier present was because of global variables (global with respect to FSNamesystem). I think this patch actually ensures that methods in SafeModeInfo cannot access private members of FSNamesystem directly. This should improve code maintainability and cleanliness.
The fact the SafeModeInfo cannot access private members of FSNamesystem means that a log message that was earlier present in one place is now duplicated at two or three places. I have removed the assert because SafeModeInfo cannot be accessing private variables in FSNamesystem.
Thanks for catching the problems with javadoc. I have fixed them in this version of this patch.
I would like to make this "patch available" because that might make other people review this patch.
> 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: innerclasses4.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.
[jira] Commented: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
Posted by "Konstantin Shvachko (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12490467 ]
Konstantin Shvachko commented on HADOOP-1272:
---------------------------------------------
I would start extracting servlet classes first. In general Servlets should be in a different package or sub-package, e.g.
org.apache.hadoop.dfs.servlets
Then there are some straightforward static inner classes, like Host2NodesMap.
Then BlocksMap and FileUnderConstruction, which are practically static although not declared as such.
It is easy to extract ReplicationTargetChooser, just move clusterMap inside the chooser and
provide api for modification and the access.
In general we should not multiply inner classes but rather create separate ones.
Should we add that into code review guidelines?
> 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: innerclasses.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.
[jira] Commented: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12491014 ]
Doug Cutting commented on HADOOP-1272:
--------------------------------------
This seems like we're cluttering the package in order to de-clutter the
file, not a great tradeoff. I'd rather see this as a part of a package
re-structuring. Arguably HDFS should move into a set of packages under
org.apache.hadoop.dfs, and these would go into the namenode sub-package.
If folks agree with that as the long-term plan, then:
- How would we name these after such a restructuring? That's probably
what we should name them now. The above names are probably okay in that
context, as package-private classes in the namenode package.
- Is there any reason not to restructure HDFS's packages now? I can't
think of any. There shouldn't be any user code that calls HDFS code
directly, should there? So I don't think back-compatibility is an issue.
Doug
> 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: innerclasses.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.
[jira] Updated: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
Posted by "dhruba borthakur (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1272?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
dhruba borthakur updated HADOOP-1272:
-------------------------------------
Attachment: innerclasses3.patch
This incorporates most of Konstantin's comments.
1. I kept the wrapper method updateNeededReplications but removed removeNeededReplications and addNeededReplications.
2. Removed all import warnings.
3. I kept a seperate class for SafeModeInfo because I think this is the right way to go in the long run. It will help us in getting fine-grain locking easy to understand.
4. Separated out the servlet classes into its own file(s).
> 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.
[jira] Commented: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
Posted by "Konstantin Shvachko (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12490485 ]
Konstantin Shvachko commented on HADOOP-1272:
---------------------------------------------
Here is the list of classes that can be easily factored out from FSNamesystem (no new packages) :
# FsckServlet
# GetImageServlet
# Host2NodesMap
# BlocksMap
# FileUnderConstruction
# ReplicationTargetChooser
Is there a consensus on this?
Do we need to change any of the names?
> Rather, the namenode, datanode and client might be in separate packages.
We are talking for a long time about it. Should we do it now?
> 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: innerclasses.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.
[jira] Commented: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
Posted by "Konstantin Shvachko (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12492790 ]
Konstantin Shvachko commented on HADOOP-1272:
---------------------------------------------
Looks like we agreed with Dhruba that whether SafeModeInfo is separated from FSNamesystem
or not it should be reorganized in order to support fine-grained locking he is working on now.
I am still not happy about how it is separated in this patch because it reduces functionality and introduces code replication.
I propose to leave it as it is in the FSNamesystem and open a new Jira issue for separating the SafeModeInfo.
I volunteer to do the separation.
It is important to proceed with all other inner classes.
> 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: innerclasses4.patch, innerclasses5.patch, innerclasses6.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.
[jira] Commented: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
Posted by "Tom White (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12490261 ]
Tom White commented on HADOOP-1272:
-----------------------------------
This will also make the code clearer by breaking up the huge class into smaller chunks (FSNamesystem is over 4000 lines long!). The top ten:
find src/java/org/apache/hadoop -name *.java | xargs wc -l | sort -nr | head
68614 total
4277 src/java/org/apache/hadoop/dfs/FSNamesystem.java
2682 src/java/org/apache/hadoop/io/SequenceFile.java
1960 src/java/org/apache/hadoop/mapred/TaskTracker.java
1717 src/java/org/apache/hadoop/mapred/JobTracker.java
1440 src/java/org/apache/hadoop/dfs/DFSClient.java
1309 src/java/org/apache/hadoop/dfs/DataNode.java
1208 src/java/org/apache/hadoop/mapred/ReduceTask.java
1095 src/java/org/apache/hadoop/fs/FsShell.java
964 src/java/org/apache/hadoop/mapred/JobInProgress.java
> 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: innerclasses.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.
[jira] Commented: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
Posted by "Konstantin Shvachko (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12492151 ]
Konstantin Shvachko commented on HADOOP-1272:
---------------------------------------------
So what is going to set
safeMode = null;
in checkMode() after this.leave() ?
> 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: innerclasses4.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.
[jira] Updated: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
Posted by "dhruba borthakur (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1272?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
dhruba borthakur updated HADOOP-1272:
-------------------------------------
Attachment: innerclasses2.patch
Since this patch moves huge amounts of code from one file to another, the earlier patch needed lots of merging so that it is trunk-compatible.
> 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: innerclasses2.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.
[jira] Commented: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
Posted by "dhruba borthakur (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12492160 ]
dhruba borthakur commented on HADOOP-1272:
------------------------------------------
In response to Konstantin's comments: The checkMode() call does not need to reset safeMode. When the SafeModeMonitor exists, it sets safeMode = null.
> 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: innerclasses4.patch, innerclasses5.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.
[jira] Commented: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12492862 ]
Hadoop QA commented on HADOOP-1272:
-----------------------------------
Integrated in Hadoop-Nightly #75 (See http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Nightly/75/)
> 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
> Fix For: 0.13.0
>
> Attachments: innerclasses4.patch, innerclasses5.patch, innerclasses6.patch, innerclasses7.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.
[jira] Commented: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
Posted by "Tom White (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12492279 ]
Tom White commented on HADOOP-1272:
-----------------------------------
Is there consensus on this patch now? Konstantin, are you happy with the changes related to SafeModeInfo or does this need further discussion before being committed?
Also, I notice there are still some whitespace issues in FileUnderConstruction.
> 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: innerclasses4.patch, innerclasses5.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.
[jira] Commented: (HADOOP-1272) Extract InnerClasses from
FSNamesystem into separate classes
Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-1272?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12491541 ]
Hadoop QA commented on HADOOP-1272:
-----------------------------------
+1
http://issues.apache.org/jira/secure/attachment/12356205/innerclasses4.patch applied and successfully tested against trunk revision r532083.
Test results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/75/testReport/
Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/75/console
> 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: innerclasses4.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.