hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Suresh Srinivas (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-1725) Cleanup FSImage construction
Date Tue, 22 Mar 2011 18:36:06 GMT

    [ https://issues.apache.org/jira/browse/HDFS-1725?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13009768#comment-13009768

Suresh Srinivas commented on HDFS-1725:

I would prefer if this jira just made constructor cleanup. I am concerned about other changes
you have made.

# FSImage.java
#* @see for FSImage(Configuration conf) and private FSImage(Configuration conf, FSNamesystem
ns) point to the constructor with wrong signature.
#* Make members storage, conf final
#* The log {{NameNode.LOG.info("set FSImage.restoreFailedStorage");}} is removed. Is this
not necessary?
#* The old set of constructors and new are not equivalent. For example: FSImage(Configuration)
constructor in the old code never set editLog. Similarly only one conustros called setCheckpointDirectories(),
now all constructors do. Is this fine?
#* Where is the functionality of {{FSImage(URI imageDir)}} imeplemented? Corresponding change
in CreateEditsLog.java could have been avoided. I think this is a valid use case and a constructor
and it should nto be removed.
# BackupImage.java - remove unused imports of URI and Collection.
#* Why are you calling attemptRestoreRemovedStorage() in #reset() method? 
# FSDirectory.java 
#* #loadFSImage has IOException as param. 
#* remove unused imports URI and Collection.
# NameNode.java - why did you remove in #format method, adding editDirsToFormat to dirsToFormat?
The list of directories passed into FSImage in this method are now changed to the list of
directories in conf?
# minor: NNStorage.java - remove empty space change after #setStorageDirectories()
# SecondaryNameNode.java - in #startCheckpoint() - why did you remove unlockAll() call? Also
recoverCreate() has attemptRestoreRemovedStorage() and unlockAll() call?
# UpgradeUtilities.java - NNStorage need not have any image or edit dirs. The main goal of
this is to write versionFile which is done using StorageDirectory near the end of the for
# TestSaveNamespace.java - why is close() method call to unlock storage not required any more?
Also remove unused imports List and Collection.

> Cleanup FSImage construction
> ----------------------------
>                 Key: HDFS-1725
>                 URL: https://issues.apache.org/jira/browse/HDFS-1725
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>            Reporter: Ivan Kelly
>            Assignee: Ivan Kelly
>             Fix For: 0.23.0
>         Attachments: HDFS-1725.diff, HDFS-1725.diff, HDFS-1725.diff
> FSImage construction is messy. Sometimes the storagedirectories in use are set straight
away, sometimes they are not. This makes it hard for anything under FSImage (i.e. FSEditLog)
to make assumptions about what it can use. Therefore, this patch makes FSImage set the storage
directories in use during construction, and never allows them to change. If you want to change
storagedirectories you create a new image.
> Also, all the construction code should be the same with the only difference being the
parameters passed. When not passed, these should get sensible defaults.

This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

View raw message