[
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 loop.
# 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
