hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Todd Lipcon (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-3094) add -nonInteractive and -force option to namenode -format command
Date Sun, 01 Apr 2012 02:28:28 GMT

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

Todd Lipcon commented on HDFS-3094:
-----------------------------------

Almost everything below is style/formatting nits. In general, try to match the style of the
surrounding code. Otherwise looks good, thanks for adding this nice improvement!

----

{code}
+        for(i=i+1 ; i < argsLen ; i++) {
{code}
space after "for", no space before ";"s, spaces in "i = i + 1"

----
{code}
+              // as no cluster id specified, return null
{code}
should read "*if* no cluster id specified"

----
{code}
+            // make sure the user did not send -force or -noninteractive as an
+            // option after -clusterid or send no id after -clusterid
{code}
replace "send" with "specify" (nothing's being sent here): "make sure the user did not specify
-force or -nonInteractive as an option after -clusterId, or forget to specify an ID after
-clusterId". Or maybe just "Make sure an id is specified, and not another flag."

This comment inside the block seems redundant with the one above:
{code}
+              // return null if the user sent something like
+              // clusterid -force or clusterid -nonInteractive
{code}
I think you can remove it.

However, it seems like you should log a warning here with their mistake, something like:
{code}
LOG.fatal("Must specify a valid cluster ID after the -clusterId flag")
{code}


----
{code}
+            if (clusterId.isEmpty()
+                || clusterId.equalsIgnoreCase(StartupOption.FORCE.getName())
+                || clusterId.equalsIgnoreCase(StartupOption.NONINTERACTIVE
+                    .getName())) {
{code}
formatting: ||s should go on the line before. ie:
{code}
            if (clusterId.isEmpty() ||
                clusterId.equalsIgnoreCase(StartupOption.FORCE.getName()) ||
                clusterId.equalsIgnoreCase(
                    StartupOption.NONINTERACTIVE.getName())) {

----
{code}
-        boolean aborted = format(conf, false);
+      boolean aborted = format(conf, startOpt.getForce(),
{code}
bad indentation on this line

----
{code}
+  protected static class ExitException extends SecurityException {
{code}
Why is this protected instead of private?

Please move the inner classes to the bottom of the file

----
In the tests, please change the '//' style comments before each test case to be javadoc style

- in all the test cases, I think you need to add the line:
{code}
fail("createNameNode() did not call System.exit()")
{code}
inside the {{try}} clause. Otherwise if the code just returned without exiting, we wouldn't
catch the bug.

----
{code}
+    final Configuration config = new Configuration();
+    config.set(DFS_NAMENODE_NAME_DIR_KEY, hdfsDir.getPath());
{code}
This code shows up in almost all the cases. Can it be done in the setup method?
                
> add -nonInteractive and -force option to namenode -format command
> -----------------------------------------------------------------
>
>                 Key: HDFS-3094
>                 URL: https://issues.apache.org/jira/browse/HDFS-3094
>             Project: Hadoop HDFS
>          Issue Type: Improvement
>    Affects Versions: 0.24.0, 1.0.2
>            Reporter: Arpit Gupta
>            Assignee: Arpit Gupta
>         Attachments: HDFS-3094.branch-1.0.patch, HDFS-3094.branch-1.0.patch, HDFS-3094.branch-1.0.patch,
HDFS-3094.branch-1.0.patch, HDFS-3094.branch-1.0.patch, HDFS-3094.branch-1.0.patch, HDFS-3094.patch,
HDFS-3094.patch, HDFS-3094.patch, HDFS-3094.patch
>
>
> Currently the bin/hadoop namenode -format prompts the user for a Y/N to setup the directories
in the local file system.
> -force : namenode formats the directories without prompting
> -nonInterActive : namenode format will return with an exit code of 1 if the dir exists.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Mime
View raw message