[ https://issues.apache.org/jira/browse/HDFS-2011?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13043892#comment-13043892
]
Todd Lipcon commented on HDFS-2011:
-----------------------------------
A few style nits on HDFS-2011.4.patch:
- please try to keep lines under 80 columns wide where possible. If it spills over to 85 or
90 here and there, not a huge deal, but 100+ columns should be avoided
- why catch SecurityException? that looks very much out of place, and given it's an unchecked
exception, you don't need to catch it at all
- the assertTrue around mkdir() in testSetCheckpoingTimeInStorageHandlesIOException should
probably check "exists() || mkdir()". Or call deleteFully on it at the top of the test
- you construct that same file path several times in the same test. Please just make it once
as a constant
- in the error messages, better to do something like: "Couldn't remove directory " + TEST_STORAGE_DIR.getAbsoluteFile().
That way the developer can easily track down the full path
- alignment is off in NNStorage.java change
- the comment referring to "edit and edits.new" in ELFOS is out of place - that class shouldn't
know about details of how it's used. Instead it should read something like "// if already
closed, just return"
- TestCheckpoint inherits from TestCase, so you don't need to import org.junit.Assert
> Removal and restoration of storage directories on checkpointing failure doesn't work
properly
> ---------------------------------------------------------------------------------------------
>
> Key: HDFS-2011
> URL: https://issues.apache.org/jira/browse/HDFS-2011
> Project: Hadoop HDFS
> Issue Type: Bug
> Components: name-node
> Affects Versions: 0.23.0
> Reporter: Ravi Prakash
> Assignee: Ravi Prakash
> Attachments: HDFS-2011.3.patch, HDFS-2011.4.patch, HDFS-2011.patch, HDFS-2011.patch,
HDFS-2011.patch
>
>
> Removal and restoration of storage directories on checkpointing failure doesn't work
properly. Sometimes it throws a NullPointerException and sometimes it doesn't take off a failed
storage directory
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
|