HBase
  1. HBase
  2. HBASE-5327

Print a message when an invalid hbase.rootdir is passed

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.90.5
    • Fix Version/s: 0.90.6, 0.92.1, 0.94.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      As seen on the mailing list: http://comments.gmane.org/gmane.comp.java.hadoop.hbase.user/24124

      If hbase.rootdir doesn't specify a folder on hdfs we crash while opening a path to .oldlogs:

      2012-02-02 23:07:26,292 FATAL org.apache.hadoop.hbase.master.HMaster: Unhandled exception. Starting shutdown.
      java.lang.IllegalArgumentException: java.net.URISyntaxException: Relative path in absolute URI: hdfs://sv4r11s38:9100.oldlogs
              at org.apache.hadoop.fs.Path.initialize(Path.java:148)
              at org.apache.hadoop.fs.Path.<init>(Path.java:71)
              at org.apache.hadoop.fs.Path.<init>(Path.java:50)
              at org.apache.hadoop.hbase.master.MasterFileSystem.<init>(MasterFileSystem.java:112)
              at org.apache.hadoop.hbase.master.HMaster.finishInitialization(HMaster.java:448)
              at org.apache.hadoop.hbase.master.HMaster.run(HMaster.java:326)
              at java.lang.Thread.run(Thread.java:662)
      Caused by: java.net.URISyntaxException: Relative path in absolute URI: hdfs://sv4r11s38:9100.oldlogs
              at java.net.URI.checkPath(URI.java:1787)
              at java.net.URI.<init>(URI.java:735)
              at org.apache.hadoop.fs.Path.initialize(Path.java:145)
              ... 6 more
      

      It could also crash anywhere else, this just happens to be the first place we use hbase.rootdir. We need to verify that it's an actual folder.

      1. hbase-5327_v2.txt
        4 kB
        Jimmy Xiang
      2. hbase-5327.txt
        2 kB
        Jimmy Xiang

        Activity

        Hide
        Hudson added a comment -

        Integrated in HBase-0.92-security #91 (See https://builds.apache.org/job/HBase-0.92-security/91/)
        HBASE-5327 Print a message when an invalid hbase.rootdir is passed (Jimmy Xiang) (Revision 1243648)

        Result = FAILURE
        tedyu :
        Files :

        • /hbase/branches/0.92/CHANGES.txt
        • /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java
        Show
        Hudson added a comment - Integrated in HBase-0.92-security #91 (See https://builds.apache.org/job/HBase-0.92-security/91/ ) HBASE-5327 Print a message when an invalid hbase.rootdir is passed (Jimmy Xiang) (Revision 1243648) Result = FAILURE tedyu : Files : /hbase/branches/0.92/CHANGES.txt /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK-security #110 (See https://builds.apache.org/job/HBase-TRUNK-security/110/)
        HBASE-5327 Print a message when an invalid hbase.rootdir is passed (Jimmy Xiang) (Revision 1243650)

        Result = SUCCESS
        tedyu :
        Files :

        • /hbase/trunk/CHANGES.txt
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java
        Show
        Hudson added a comment - Integrated in HBase-TRUNK-security #110 (See https://builds.apache.org/job/HBase-TRUNK-security/110/ ) HBASE-5327 Print a message when an invalid hbase.rootdir is passed (Jimmy Xiang) (Revision 1243650) Result = SUCCESS tedyu : Files : /hbase/trunk/CHANGES.txt /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java
        Hide
        Hudson added a comment -

        Integrated in HBase-0.92 #278 (See https://builds.apache.org/job/HBase-0.92/278/)
        HBASE-5327 Print a message when an invalid hbase.rootdir is passed (Jimmy Xiang) (Revision 1243648)

        Result = SUCCESS
        tedyu :
        Files :

        • /hbase/branches/0.92/CHANGES.txt
        • /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java
        Show
        Hudson added a comment - Integrated in HBase-0.92 #278 (See https://builds.apache.org/job/HBase-0.92/278/ ) HBASE-5327 Print a message when an invalid hbase.rootdir is passed (Jimmy Xiang) (Revision 1243648) Result = SUCCESS tedyu : Files : /hbase/branches/0.92/CHANGES.txt /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK #2660 (See https://builds.apache.org/job/HBase-TRUNK/2660/)
        HBASE-5327 Print a message when an invalid hbase.rootdir is passed (Jimmy Xiang) (Revision 1243650)

        Result = FAILURE
        tedyu :
        Files :

        • /hbase/trunk/CHANGES.txt
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java
        Show
        Hudson added a comment - Integrated in HBase-TRUNK #2660 (See https://builds.apache.org/job/HBase-TRUNK/2660/ ) HBASE-5327 Print a message when an invalid hbase.rootdir is passed (Jimmy Xiang) (Revision 1243650) Result = FAILURE tedyu : Files : /hbase/trunk/CHANGES.txt /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java
        Hide
        Ted Yu added a comment -

        Integrated to 0.90, 0.92 and TRUNK.

        Thanks for the patch, Jimmy.

        Thanks for the review Stack and Jon.

        Show
        Ted Yu added a comment - Integrated to 0.90, 0.92 and TRUNK. Thanks for the patch, Jimmy. Thanks for the review Stack and Jon.
        Hide
        Jonathan Hsieh added a comment -

        @Jimmy. Thanks for looking into this. I'm +1 on v2, plan on committing tomorrow unless I hear otherwise.

        Show
        Jonathan Hsieh added a comment - @Jimmy. Thanks for looking into this. I'm +1 on v2, plan on committing tomorrow unless I hear otherwise.
        Hide
        Jimmy Xiang added a comment -

        I looked into it. For new Path(path), the path doesn't have to be a complete and valid path. It could be a relative path so it can't be validated.
        new Path(parent, child) takes two paths to form a new one (String is converted to Path implicitly). If parent = "hdfs://localhost:999"
        and child = "/test", the new path will be hdfs://localhost:999/test and it is valid and all are happy. However is child = "test",
        in combining there to a URI, the result is hdfs://localhost:999test which is invalid. That's the reason for URISyntaxException.

        v2 patch doesn't look good, but I am ok with it.

        Show
        Jimmy Xiang added a comment - I looked into it. For new Path(path), the path doesn't have to be a complete and valid path. It could be a relative path so it can't be validated. new Path(parent, child) takes two paths to form a new one (String is converted to Path implicitly). If parent = "hdfs://localhost:999" and child = "/test", the new path will be hdfs://localhost:999/test and it is valid and all are happy. However is child = "test", in combining there to a URI, the result is hdfs://localhost:999test which is invalid. That's the reason for URISyntaxException. v2 patch doesn't look good, but I am ok with it.
        Hide
        Jonathan Hsieh added a comment -

        Jimmy and I were chatting about the funny control flow and found it strange that new Path(invalidRootDir) didn't through IllegalArgumentException, while {{ new Path (new Path(invalidRootDir), ".olddirs")}} threw an IllegalArguentException. (I was expecting the mkdirs to IAE, not exists).

        This explains why the slightly odd control flow is necessary and makes me much happier with v2 patch.

        Maybe someone with more HDFS background can chime in about why this seeming inconsistency exists?

        Show
        Jonathan Hsieh added a comment - Jimmy and I were chatting about the funny control flow and found it strange that new Path(invalidRootDir) didn't through IllegalArgumentException, while {{ new Path (new Path(invalidRootDir), ".olddirs")}} threw an IllegalArguentException. (I was expecting the mkdirs to IAE, not exists). This explains why the slightly odd control flow is necessary and makes me much happier with v2 patch. Maybe someone with more HDFS background can chime in about why this seeming inconsistency exists?
        Hide
        Jonathan Hsieh added a comment -

        My comment was really just asking for more info in the first version's IAE string.

        Show
        Jonathan Hsieh added a comment - My comment was really just asking for more info in the first version's IAE string.
        Hide
        Jimmy Xiang added a comment -

        I prefer the first version actually. If the root dir is invalid, HDFS will throw an IAE. That's how we know a path an invalid HDFS path.

        Show
        Jimmy Xiang added a comment - I prefer the first version actually. If the root dir is invalid, HDFS will throw an IAE. That's how we know a path an invalid HDFS path.
        Hide
        Jonathan Hsieh added a comment -

        I'm not a fan of using the IAE for control flow, but the lgtm.

        Show
        Jonathan Hsieh added a comment - I'm not a fan of using the IAE for control flow, but the lgtm.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12513982/hbase-5327_v2.txt
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. 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 javadoc. The javadoc tool appears to have generated -136 warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        -1 findbugs. The patch appears to introduce 156 new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed these unit tests:
        org.apache.hadoop.hbase.replication.TestReplication
        org.apache.hadoop.hbase.io.hfile.TestHFileBlock
        org.apache.hadoop.hbase.mapreduce.TestImportTsv
        org.apache.hadoop.hbase.mapred.TestTableMapReduce
        org.apache.hadoop.hbase.mapreduce.TestHFileOutputFormat

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/929//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/929//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/929//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12513982/hbase-5327_v2.txt against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. 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 javadoc. The javadoc tool appears to have generated -136 warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 156 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: org.apache.hadoop.hbase.replication.TestReplication org.apache.hadoop.hbase.io.hfile.TestHFileBlock org.apache.hadoop.hbase.mapreduce.TestImportTsv org.apache.hadoop.hbase.mapred.TestTableMapReduce org.apache.hadoop.hbase.mapreduce.TestHFileOutputFormat Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/929//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/929//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/929//console This message is automatically generated.
        Hide
        stack added a comment -

        +1

        Show
        stack added a comment - +1
        Hide
        Jimmy Xiang added a comment -

        This patch fix two issues:
        (1) check the root dir to make sure it is valid before generating the old log dir. So that it can give a meaningful error message.
        (2) make sure the root dir is a dir instead of a file. If it is a file, the master will hang and try to create the version file forever.

        @Jon, I added some actionable log message.

        Show
        Jimmy Xiang added a comment - This patch fix two issues: (1) check the root dir to make sure it is valid before generating the old log dir. So that it can give a meaningful error message. (2) make sure the root dir is a dir instead of a file. If it is a file, the master will hang and try to create the version file forever. @Jon, I added some actionable log message.
        Hide
        Jonathan Hsieh added a comment -

        Can the comment in the exception be more specific and actionable?

        Let's tell the user that the root dir is not a valid dir and tell the name of the hbase-site.xml property to update.

        Show
        Jonathan Hsieh added a comment - Can the comment in the exception be more specific and actionable? Let's tell the user that the root dir is not a valid dir and tell the name of the hbase-site.xml property to update.
        Hide
        stack added a comment -

        The difference is that we get a nicer message on master crash out? Its about the bad rootdir passed rather than some complaint about a URI parse?

        If so, good enough for me. +1.

        Show
        stack added a comment - The difference is that we get a nicer message on master crash out? Its about the bad rootdir passed rather than some complaint about a URI parse? If so, good enough for me. +1.
        Hide
        Ted Yu added a comment -

        Patch looks good to me.

        Show
        Ted Yu added a comment - Patch looks good to me.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12513890/hbase-5327.txt
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. 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 javadoc. The javadoc tool appears to have generated -136 warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        -1 findbugs. The patch appears to introduce 156 new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in .

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/926//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/926//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/926//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12513890/hbase-5327.txt against trunk revision . +1 @author. The patch does not contain any @author tags. -1 tests included. 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 javadoc. The javadoc tool appears to have generated -136 warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 156 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/926//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/926//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/926//console This message is automatically generated.
        Hide
        Jimmy Xiang added a comment -

        I tested the patch and it can detect the error mentioned in the description and abort the master with error message saying it is not a valid hdfs file path.

        Show
        Jimmy Xiang added a comment - I tested the patch and it can detect the error mentioned in the description and abort the master with error message saying it is not a valid hdfs file path.

          People

          • Assignee:
            Jimmy Xiang
            Reporter:
            Jean-Daniel Cryans
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development