Uploaded image for project: 'Hadoop HDFS'
  1. Hadoop HDFS
  2. HDFS-9112

Improve error message for Haadmin when multiple name service IDs are configured

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.7.1
    • Fix Version/s: 2.8.0, 3.0.0-alpha1
    • Component/s: tools
    • Labels:
      None
    • Target Version/s:
    • Hadoop Flags:
      Reviewed

      Description

      In HDFS-6376 we supported a feature for distcp that allows multiple NameService IDs to be specified so that we can copy from two HA enabled clusters.

      That confuses haadmin command since we have a check in DFSUtil#getNamenodeServiceAddr which fails if it finds more than 1 name in that property.

      1. HDFS-9112.001.patch
        7 kB
        Anu Engineer
      2. HDFS-9112.002.patch
        1 kB
        Anu Engineer
      3. HDFS-9112.003.patch
        2 kB
        Anu Engineer
      4. HDFS-9112.004.patch
        2 kB
        Anu Engineer

        Activity

        Hide
        atm Aaron T. Myers added a comment -

        Pinging Daniel Templeton, since I know he was just looking at this code as well.

        Show
        atm Aaron T. Myers added a comment - Pinging Daniel Templeton , since I know he was just looking at this code as well.
        Hide
        anu Anu Engineer added a comment -

        Aaron T. Myers Thanks for letting me know. Daniel Templeton I would appreciate if you can take a look at this patch.

        This patch fixes getNamenodeServiceAddr by looking at dfs.internal.nameservices and choosing the right name if we have more than one name entry in dfs.nameservices.

        Along with Unit tests, manually verified that haadmin command is now able to locate nameserver URI if we have the setup described in HDFS-6376

        Show
        anu Anu Engineer added a comment - Aaron T. Myers Thanks for letting me know. Daniel Templeton I would appreciate if you can take a look at this patch. This patch fixes getNamenodeServiceAddr by looking at dfs.internal.nameservices and choosing the right name if we have more than one name entry in dfs.nameservices. Along with Unit tests, manually verified that haadmin command is now able to locate nameserver URI if we have the setup described in HDFS-6376
        Hide
        jingzhao Jing Zhao added a comment -

        We had a discussion in HDFS-6376 about this and Dave Marion's point is it's better to require admin to specify the name service id using "-ns" option in haadmin commands in such a complex configuration scenario (please see his comment here).

        Show
        jingzhao Jing Zhao added a comment - We had a discussion in HDFS-6376 about this and Dave Marion 's point is it's better to require admin to specify the name service id using "-ns" option in haadmin commands in such a complex configuration scenario (please see his comment here ).
        Hide
        anu Anu Engineer added a comment -

        Jing Zhao Thanks for the pointer to the Dave Marion 's comments. I see that we had assumed that it is better to let users specify -ns option if they have this kind of HA setup. However it looks like both us and cloudera ran into this issue in the field hence I think we need to have a little more clarity with error messages, with the current code the error message is very cryptic.

        hdfs haadmin -getServiceState nn2
        Illegal argument: Unable to determine the nameservice id.
        

        This gives no clue to the user that they are expected to specify -ns option. Also from the comments that you pointed me to I am not able to decipher why it is better to specify "-ns" by the user, when we have that information in the config files. Since I don't have much context on HDFS-6376, I would appreciate if you can provide some rationale (From cursory comment reading it looks to me that Dave originally had exclude settings which created some issues, but Haohui Mai modified them to internal nameservices. If so using internal name services hopefully should not cause a failure.)

        if you like I can modify this patch to print out an error message which asks user to add -ns option explicitly, instead of reading the name services name from config, that would be a trivial change. Please let me know if you think I should do that or if this change looks good enough.

        Show
        anu Anu Engineer added a comment - Jing Zhao Thanks for the pointer to the Dave Marion 's comments. I see that we had assumed that it is better to let users specify -ns option if they have this kind of HA setup. However it looks like both us and cloudera ran into this issue in the field hence I think we need to have a little more clarity with error messages, with the current code the error message is very cryptic. hdfs haadmin -getServiceState nn2 Illegal argument: Unable to determine the nameservice id. This gives no clue to the user that they are expected to specify -ns option. Also from the comments that you pointed me to I am not able to decipher why it is better to specify "-ns" by the user, when we have that information in the config files. Since I don't have much context on HDFS-6376 , I would appreciate if you can provide some rationale (From cursory comment reading it looks to me that Dave originally had exclude settings which created some issues, but Haohui Mai modified them to internal nameservices. If so using internal name services hopefully should not cause a failure.) if you like I can modify this patch to print out an error message which asks user to add -ns option explicitly, instead of reading the name services name from config, that would be a trivial change. Please let me know if you think I should do that or if this change looks good enough.
        Hide
        jingzhao Jing Zhao added a comment -

        Thanks for the clarification, Anu Engineer! I think for admin or other clients it's not necessary for them to clearly distinguish internal/external name services. The internal/external ns makes sense maybe only to DataNodes. Thus I'm currently leaning towards requiring admins to explicitly specify the name service using "-ns" option. But I completely agree with you that we should improve the error message.

        Show
        jingzhao Jing Zhao added a comment - Thanks for the clarification, Anu Engineer ! I think for admin or other clients it's not necessary for them to clearly distinguish internal/external name services. The internal/external ns makes sense maybe only to DataNodes. Thus I'm currently leaning towards requiring admins to explicitly specify the name service using "-ns" option. But I completely agree with you that we should improve the error message.
        Hide
        hadoopqa Hadoop QA added a comment -



        -1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 21m 55s Pre-patch trunk compilation is healthy.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 tests included 0m 0s The patch appears to include 1 new or modified test files.
        +1 javac 8m 8s There were no new javac warning messages.
        +1 javadoc 10m 24s There were no new javadoc warning messages.
        +1 release audit 0m 27s The applied patch does not increase the total number of release audit warnings.
        +1 checkstyle 3m 50s There were no new checkstyle issues.
        +1 whitespace 0m 0s The patch has no lines that end in whitespace.
        +1 install 1m 48s mvn install still works.
        +1 eclipse:eclipse 0m 34s The patch built with eclipse:eclipse.
        +1 findbugs 6m 36s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
        +1 common tests 24m 29s Tests passed in hadoop-common.
        -1 hdfs tests 197m 36s Tests failed in hadoop-hdfs.
        +1 hdfs tests 0m 45s Tests passed in hadoop-hdfs-client.
            276m 37s  



        Reason Tests
        Failed unit tests hadoop.hdfs.server.namenode.TestFileTruncate
          hadoop.hdfs.TestRollingUpgrade



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12761473/HDFS-9112.001.patch
        Optional Tests javadoc javac unit findbugs checkstyle
        git revision trunk / b00392d
        hadoop-common test log https://builds.apache.org/job/PreCommit-HDFS-Build/12573/artifact/patchprocess/testrun_hadoop-common.txt
        hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/12573/artifact/patchprocess/testrun_hadoop-hdfs.txt
        hadoop-hdfs-client test log https://builds.apache.org/job/PreCommit-HDFS-Build/12573/artifact/patchprocess/testrun_hadoop-hdfs-client.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/12573/testReport/
        Java 1.7.0_55
        uname Linux asf908.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/12573/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 21m 55s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. +1 tests included 0m 0s The patch appears to include 1 new or modified test files. +1 javac 8m 8s There were no new javac warning messages. +1 javadoc 10m 24s There were no new javadoc warning messages. +1 release audit 0m 27s The applied patch does not increase the total number of release audit warnings. +1 checkstyle 3m 50s There were no new checkstyle issues. +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 48s mvn install still works. +1 eclipse:eclipse 0m 34s The patch built with eclipse:eclipse. +1 findbugs 6m 36s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 common tests 24m 29s Tests passed in hadoop-common. -1 hdfs tests 197m 36s Tests failed in hadoop-hdfs. +1 hdfs tests 0m 45s Tests passed in hadoop-hdfs-client.     276m 37s   Reason Tests Failed unit tests hadoop.hdfs.server.namenode.TestFileTruncate   hadoop.hdfs.TestRollingUpgrade Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12761473/HDFS-9112.001.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / b00392d hadoop-common test log https://builds.apache.org/job/PreCommit-HDFS-Build/12573/artifact/patchprocess/testrun_hadoop-common.txt hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/12573/artifact/patchprocess/testrun_hadoop-hdfs.txt hadoop-hdfs-client test log https://builds.apache.org/job/PreCommit-HDFS-Build/12573/artifact/patchprocess/testrun_hadoop-hdfs-client.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/12573/testReport/ Java 1.7.0_55 uname Linux asf908.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HDFS-Build/12573/console This message was automatically generated.
        Hide
        anu Anu Engineer added a comment -

        Based on Jing Zhao comments, this change makes the error message more explicit. It tells the user to pass -ns if needed.

        As for the test failures for the patch 1, that does not seem related to the patch

        Show
        anu Anu Engineer added a comment - Based on Jing Zhao comments, this change makes the error message more explicit. It tells the user to pass -ns if needed. As for the test failures for the patch 1, that does not seem related to the patch
        Hide
        hadoopqa Hadoop QA added a comment -



        -1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 22m 40s Pre-patch trunk compilation is healthy.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 tests included 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
        +1 javac 9m 56s There were no new javac warning messages.
        +1 javadoc 10m 8s There were no new javadoc warning messages.
        +1 release audit 0m 24s The applied patch does not increase the total number of release audit warnings.
        +1 checkstyle 1m 37s There were no new checkstyle issues.
        +1 whitespace 0m 0s The patch has no lines that end in whitespace.
        +1 install 1m 37s mvn install still works.
        +1 eclipse:eclipse 0m 36s The patch built with eclipse:eclipse.
        +1 findbugs 2m 26s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
        +1 native 3m 9s Pre-build of native portion
        -1 hdfs tests 163m 19s Tests failed in hadoop-hdfs.
            215m 56s  



        Reason Tests
        Failed unit tests hadoop.hdfs.server.balancer.TestBalancerWithMultipleNameNodes



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12761529/HDFS-9112.002.patch
        Optional Tests javadoc javac unit findbugs checkstyle
        git revision trunk / b00392d
        hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/12579/artifact/patchprocess/testrun_hadoop-hdfs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/12579/testReport/
        Java 1.7.0_55
        uname Linux asf901.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/12579/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 22m 40s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. -1 tests included 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac 9m 56s There were no new javac warning messages. +1 javadoc 10m 8s There were no new javadoc warning messages. +1 release audit 0m 24s The applied patch does not increase the total number of release audit warnings. +1 checkstyle 1m 37s There were no new checkstyle issues. +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 37s mvn install still works. +1 eclipse:eclipse 0m 36s The patch built with eclipse:eclipse. +1 findbugs 2m 26s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 native 3m 9s Pre-build of native portion -1 hdfs tests 163m 19s Tests failed in hadoop-hdfs.     215m 56s   Reason Tests Failed unit tests hadoop.hdfs.server.balancer.TestBalancerWithMultipleNameNodes Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12761529/HDFS-9112.002.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / b00392d hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/12579/artifact/patchprocess/testrun_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/12579/testReport/ Java 1.7.0_55 uname Linux asf901.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HDFS-Build/12579/console This message was automatically generated.
        Hide
        anu Anu Engineer added a comment -

        test failure is not related to the patch

        Show
        anu Anu Engineer added a comment - test failure is not related to the patch
        Hide
        templedf Daniel Templeton added a comment -

        Thanks, Anu, I haven't tested the patch yet, but from reading the changes, here are my initial comments. (I'm playing with formatting options for the code macro. Hopefully this comes through as intended.)

        DFSUtil.java
            // HDFS-6376 Added the ability to support multiple NameService IDs.
            // if we have multiple nameService IDs we need to read
            // dfs.internal.nameservices and pick that name as the internal Name
            // Service Name.
        

        Love that you're explaining what you're doing, but let's leave the JIRA ID out of it and don't explain it in terms of what changed. The comment should just explain the current state of things.

        DFSUtil.java
            return internalNSIds.toArray(new String[1])[0];
        

        I think

        internalNSIds.iterator().next()

        is the more standard way to do that.

        HdfsClientConfigKeys.java
          String DFS_INTERNAL_NAMESERVICES_KEY = "dfs.internal.nameservices";
        

        I realize that you're just following the pattern set with DFS_NAMESERVICES_KEY, but I really don't like the idea of duplicating key names between DFSConfigKeys and HdfsClientConfigKeys. If there some reason not to use DFSConfigKeys.DFS_INTERNAL_NAMESERVICES_KEY?

        Show
        templedf Daniel Templeton added a comment - Thanks, Anu, I haven't tested the patch yet, but from reading the changes, here are my initial comments. (I'm playing with formatting options for the code macro. Hopefully this comes through as intended.) DFSUtil.java // HDFS-6376 Added the ability to support multiple NameService IDs. // if we have multiple nameService IDs we need to read // dfs.internal.nameservices and pick that name as the internal Name // Service Name. Love that you're explaining what you're doing, but let's leave the JIRA ID out of it and don't explain it in terms of what changed. The comment should just explain the current state of things. DFSUtil.java return internalNSIds.toArray( new String [1])[0]; I think internalNSIds.iterator().next() is the more standard way to do that. HdfsClientConfigKeys.java String DFS_INTERNAL_NAMESERVICES_KEY = "dfs.internal.nameservices" ; I realize that you're just following the pattern set with DFS_NAMESERVICES_KEY, but I really don't like the idea of duplicating key names between DFSConfigKeys and HdfsClientConfigKeys. If there some reason not to use DFSConfigKeys.DFS_INTERNAL_NAMESERVICES_KEY?
        Hide
        anu Anu Engineer added a comment -

        Daniel Templeton Thank you very much for your code review and comments.

        Before we begin, I want to point out that Jing Zhao commented that we should not fix this issue at all, but just add a warning for the user that they should use "-ns" option in the error message.This is based on the discussion in HDFS-6376. So I have a second patch which does exactly that. So I am guessing that second patch will get applied by Jing Zhao.

        Meanwhile I will post one more patch that modifies the first code path in case you would like to use it.

        Love that you're explaining what you're doing, but let's leave the JIRA ID out of it and don't explain it in terms of what changed. The comment should just explain the current state of things.

        Agreed, let me remove the redundant JIRA number and fix that comment.

        internalNSIds.iterator().next()

        It might be, however I was just following the pattern in the nearby code. It is easier to read code if we have same patterns in a file.

        I really don't like the idea of duplicating key names between DFSConfigKeys and HdfsClientConfigKeys

        Not really sure about this, but I am guessing this might be due to the fact that we have an on-going refactoring work being done which separates HDFS client into its own jar.

        Show
        anu Anu Engineer added a comment - Daniel Templeton Thank you very much for your code review and comments. Before we begin, I want to point out that Jing Zhao commented that we should not fix this issue at all, but just add a warning for the user that they should use "-ns" option in the error message.This is based on the discussion in HDFS-6376 . So I have a second patch which does exactly that. So I am guessing that second patch will get applied by Jing Zhao . Meanwhile I will post one more patch that modifies the first code path in case you would like to use it. Love that you're explaining what you're doing, but let's leave the JIRA ID out of it and don't explain it in terms of what changed. The comment should just explain the current state of things. Agreed, let me remove the redundant JIRA number and fix that comment. internalNSIds.iterator().next() It might be, however I was just following the pattern in the nearby code. It is easier to read code if we have same patterns in a file. I really don't like the idea of duplicating key names between DFSConfigKeys and HdfsClientConfigKeys Not really sure about this, but I am guessing this might be due to the fact that we have an on-going refactoring work being done which separates HDFS client into its own jar.
        Hide
        templedf Daniel Templeton added a comment -

        [~aengineer], don't sweat it. I saw the second patch, but missed that it was a replacement and not intended as additive.

        In the second patch, "Name Service" should be "Name Services".

        Show
        templedf Daniel Templeton added a comment - [~aengineer] , don't sweat it. I saw the second patch, but missed that it was a replacement and not intended as additive. In the second patch, "Name Service" should be "Name Services".
        Hide
        jingzhao Jing Zhao added a comment -

        Thanks for the new patch, Anu Engineer! And thanks for the review, Daniel Templeton!

        Besides the comment from Daniel Templeton, maybe we can print out the current value of DFS_NAMESERVICES in the error message? Other than this the patch looks good to me.

        Show
        jingzhao Jing Zhao added a comment - Thanks for the new patch, Anu Engineer ! And thanks for the review, Daniel Templeton ! Besides the comment from Daniel Templeton , maybe we can print out the current value of DFS_NAMESERVICES in the error message? Other than this the patch looks good to me.
        Hide
        anu Anu Engineer added a comment -

        Thanks for the reviews Jing Zhao and Daniel Templeton
        I have addressed feedback from both of you in the latest patch.

        Show
        anu Anu Engineer added a comment - Thanks for the reviews Jing Zhao and Daniel Templeton I have addressed feedback from both of you in the latest patch.
        Hide
        jingzhao Jing Zhao added a comment -

        Thanks for updating the patch, Anu Engineer! The latest patch looks good to me. +1. I will commit the patch later if Daniel Templeton does not have further comments.

        Show
        jingzhao Jing Zhao added a comment - Thanks for updating the patch, Anu Engineer ! The latest patch looks good to me. +1. I will commit the patch later if Daniel Templeton does not have further comments.
        Hide
        liuml07 Mingliang Liu added a comment -

        Just for the record.

        I realize that you're just following the pattern set with DFS_NAMESERVICES_KEY, but I really don't like the idea of duplicating key names between DFSConfigKeys and HdfsClientConfigKeys.

        Daniel Templeton I agree that we need to make the config keys consistent and, if possible, singleton. As you pointed out, Anu Engineer is just following the pattern set with DFS_NAMESERVICES key. In fact, the DFS_NAMESERVICES is duplicated in DFSConfigKeys and HdfsClientConfigKeys. Since it is mainly used in server side, it's better to keep it stay in DFSConfigKeys as well along with other related keys like DFS_NAMESERVICE_ID and DFS_INTERNAL_NAMESERVICES_KEY.

        One improvement is that, we can copy of the value from HdfsClientConfigKeys instead of duplicating the string value.

        DFSConfigKeys.java
        - public static final String DFS_NAMESERVICES = "dfs.nameservices";
        + public static final String DFS_NAMESERVICES = HdfsClientConfigKeys.DFS_NAMESERVICES;
        

        If this makes sense, I can file a jira addressing this.

        Show
        liuml07 Mingliang Liu added a comment - Just for the record. I realize that you're just following the pattern set with DFS_NAMESERVICES_KEY, but I really don't like the idea of duplicating key names between DFSConfigKeys and HdfsClientConfigKeys. Daniel Templeton I agree that we need to make the config keys consistent and, if possible, singleton. As you pointed out, Anu Engineer is just following the pattern set with DFS_NAMESERVICES key. In fact, the DFS_NAMESERVICES is duplicated in DFSConfigKeys and HdfsClientConfigKeys . Since it is mainly used in server side, it's better to keep it stay in DFSConfigKeys as well along with other related keys like DFS_NAMESERVICE_ID and DFS_INTERNAL_NAMESERVICES_KEY . One improvement is that, we can copy of the value from HdfsClientConfigKeys instead of duplicating the string value. DFSConfigKeys.java - public static final String DFS_NAMESERVICES = "dfs.nameservices" ; + public static final String DFS_NAMESERVICES = HdfsClientConfigKeys.DFS_NAMESERVICES; If this makes sense, I can file a jira addressing this.
        Hide
        hadoopqa Hadoop QA added a comment -



        -1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 20m 24s Pre-patch trunk compilation is healthy.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 tests included 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
        +1 javac 9m 4s There were no new javac warning messages.
        +1 javadoc 11m 23s There were no new javadoc warning messages.
        +1 release audit 0m 26s The applied patch does not increase the total number of release audit warnings.
        +1 checkstyle 1m 38s There were no new checkstyle issues.
        +1 whitespace 0m 0s The patch has no lines that end in whitespace.
        +1 install 1m 42s mvn install still works.
        +1 eclipse:eclipse 0m 40s The patch built with eclipse:eclipse.
        -1 findbugs 2m 30s Post-patch findbugs hadoop-hdfs-project/hadoop-hdfs compilation is broken.
        +1 findbugs 2m 30s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
        +1 native 0m 27s Pre-build of native portion
        -1 hdfs tests 199m 12s Tests failed in hadoop-hdfs.
            247m 30s  



        Reason Tests
        Failed unit tests hadoop.hdfs.TestLeaseRecovery2
          hadoop.hdfs.server.namenode.TestFSNamesystem
          hadoop.fs.TestHdfsNativeCodeLoader
        Timed out tests org.apache.hadoop.hdfs.TestDFSStorageStateRecovery



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12762213/HDFS-9112.003.patch
        Optional Tests javadoc javac unit findbugs checkstyle
        git revision trunk / d1b9b85
        hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/12669/artifact/patchprocess/testrun_hadoop-hdfs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/12669/testReport/
        Java 1.7.0_55
        uname Linux asf909.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/12669/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 20m 24s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. -1 tests included 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac 9m 4s There were no new javac warning messages. +1 javadoc 11m 23s There were no new javadoc warning messages. +1 release audit 0m 26s The applied patch does not increase the total number of release audit warnings. +1 checkstyle 1m 38s There were no new checkstyle issues. +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 42s mvn install still works. +1 eclipse:eclipse 0m 40s The patch built with eclipse:eclipse. -1 findbugs 2m 30s Post-patch findbugs hadoop-hdfs-project/hadoop-hdfs compilation is broken. +1 findbugs 2m 30s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 native 0m 27s Pre-build of native portion -1 hdfs tests 199m 12s Tests failed in hadoop-hdfs.     247m 30s   Reason Tests Failed unit tests hadoop.hdfs.TestLeaseRecovery2   hadoop.hdfs.server.namenode.TestFSNamesystem   hadoop.fs.TestHdfsNativeCodeLoader Timed out tests org.apache.hadoop.hdfs.TestDFSStorageStateRecovery Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12762213/HDFS-9112.003.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / d1b9b85 hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/12669/artifact/patchprocess/testrun_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/12669/testReport/ Java 1.7.0_55 uname Linux asf909.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HDFS-Build/12669/console This message was automatically generated.
        Hide
        templedf Daniel Templeton added a comment -

        Thanks, Anu Engineer. Good error message! If you don't mind, I'd like to suggest some minor changes:

        "Unable to determine the nameservice id. This is an HA configuration with multiple Name services configured. " + DFS_NAMESERVICES + " value is set to " + Arrays.toString(dfsNames) + ". Please re-run with -ns option."

        I would prefer:

        "Unable to determine the name service ID. This is an HA configuration with multiple name services configured. " + DFS_NAMESERVICES + " is set to " + Arrays.toString(dfsNames) + ". Please re-run with the -ns option."
        

        Sorry to play language police.

        I would also rather see some use of newlines to make the code a little more readable. I like to see a newline on either side of a code block, so, in this case, before each of the if statements and after the closing braces. I'll +1 one the patch (after the language changes) without newlines, but I think it would be swell to add some.

        Show
        templedf Daniel Templeton added a comment - Thanks, Anu Engineer . Good error message! If you don't mind, I'd like to suggest some minor changes: "Unable to determine the nameservice id. This is an HA configuration with multiple Name services configured. " + DFS_NAMESERVICES + " value is set to " + Arrays.toString(dfsNames) + ". Please re-run with -ns option." I would prefer: "Unable to determine the name service ID. This is an HA configuration with multiple name services configured. " + DFS_NAMESERVICES + " is set to " + Arrays.toString(dfsNames) + ". Please re-run with the -ns option." Sorry to play language police. I would also rather see some use of newlines to make the code a little more readable. I like to see a newline on either side of a code block, so, in this case, before each of the if statements and after the closing braces. I'll +1 one the patch (after the language changes) without newlines, but I think it would be swell to add some.
        Hide
        anu Anu Engineer added a comment -

        Daniel Templeton Thanks for your comments, I have taken care of both issues pointed out by you.

        Show
        anu Anu Engineer added a comment - Daniel Templeton Thanks for your comments, I have taken care of both issues pointed out by you.
        Hide
        templedf Daniel Templeton added a comment -

        Thanks, Anu Engineer. +1 (non-bindng)

        (That last newline isn't really needed. Newlines between braces don't help much.)

        Show
        templedf Daniel Templeton added a comment - Thanks, Anu Engineer . +1 (non-bindng) (That last newline isn't really needed. Newlines between braces don't help much.)
        Hide
        jingzhao Jing Zhao added a comment -

        I've committed the patch to trunk and branch-2. Thanks Anu Engineer for the contribution! Also thanks for the review Daniel Templeton!

        Show
        jingzhao Jing Zhao added a comment - I've committed the patch to trunk and branch-2. Thanks Anu Engineer for the contribution! Also thanks for the review Daniel Templeton !
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-trunk-Commit #8519 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8519/)
        HDFS-9112. Improve error message for Haadmin when multiple name service IDs are configured. Contributed by Anu Engineer. (jing9: rev 83e99d06d0e5a71888aab33e9ae47460e9f1231f)

        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/NNHAServiceTarget.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #8519 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8519/ ) HDFS-9112 . Improve error message for Haadmin when multiple name service IDs are configured. Contributed by Anu Engineer. (jing9: rev 83e99d06d0e5a71888aab33e9ae47460e9f1231f) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/NNHAServiceTarget.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #447 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/447/)
        HDFS-9112. Improve error message for Haadmin when multiple name service IDs are configured. Contributed by Anu Engineer. (jing9: rev 83e99d06d0e5a71888aab33e9ae47460e9f1231f)

        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/NNHAServiceTarget.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #447 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/447/ ) HDFS-9112 . Improve error message for Haadmin when multiple name service IDs are configured. Contributed by Anu Engineer. (jing9: rev 83e99d06d0e5a71888aab33e9ae47460e9f1231f) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/NNHAServiceTarget.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #441 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/441/)
        HDFS-9112. Improve error message for Haadmin when multiple name service IDs are configured. Contributed by Anu Engineer. (jing9: rev 83e99d06d0e5a71888aab33e9ae47460e9f1231f)

        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/NNHAServiceTarget.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #441 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/441/ ) HDFS-9112 . Improve error message for Haadmin when multiple name service IDs are configured. Contributed by Anu Engineer. (jing9: rev 83e99d06d0e5a71888aab33e9ae47460e9f1231f) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/NNHAServiceTarget.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Yarn-trunk #1180 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/1180/)
        HDFS-9112. Improve error message for Haadmin when multiple name service IDs are configured. Contributed by Anu Engineer. (jing9: rev 83e99d06d0e5a71888aab33e9ae47460e9f1231f)

        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/NNHAServiceTarget.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk #1180 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/1180/ ) HDFS-9112 . Improve error message for Haadmin when multiple name service IDs are configured. Contributed by Anu Engineer. (jing9: rev 83e99d06d0e5a71888aab33e9ae47460e9f1231f) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/NNHAServiceTarget.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Mapreduce-trunk #2385 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2385/)
        HDFS-9112. Improve error message for Haadmin when multiple name service IDs are configured. Contributed by Anu Engineer. (jing9: rev 83e99d06d0e5a71888aab33e9ae47460e9f1231f)

        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/NNHAServiceTarget.java
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #2385 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2385/ ) HDFS-9112 . Improve error message for Haadmin when multiple name service IDs are configured. Contributed by Anu Engineer. (jing9: rev 83e99d06d0e5a71888aab33e9ae47460e9f1231f) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/NNHAServiceTarget.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Hdfs-trunk #2358 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2358/)
        HDFS-9112. Improve error message for Haadmin when multiple name service IDs are configured. Contributed by Anu Engineer. (jing9: rev 83e99d06d0e5a71888aab33e9ae47460e9f1231f)

        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/NNHAServiceTarget.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #2358 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2358/ ) HDFS-9112 . Improve error message for Haadmin when multiple name service IDs are configured. Contributed by Anu Engineer. (jing9: rev 83e99d06d0e5a71888aab33e9ae47460e9f1231f) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/NNHAServiceTarget.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #418 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/418/)
        HDFS-9112. Improve error message for Haadmin when multiple name service IDs are configured. Contributed by Anu Engineer. (jing9: rev 83e99d06d0e5a71888aab33e9ae47460e9f1231f)

        • hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/NNHAServiceTarget.java
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #418 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/418/ ) HDFS-9112 . Improve error message for Haadmin when multiple name service IDs are configured. Contributed by Anu Engineer. (jing9: rev 83e99d06d0e5a71888aab33e9ae47460e9f1231f) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/NNHAServiceTarget.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt

          People

          • Assignee:
            anu Anu Engineer
            Reporter:
            anu Anu Engineer
          • Votes:
            0 Vote for this issue
            Watchers:
            11 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development