Hadoop Common
  1. Hadoop Common
  2. HADOOP-9041

FileSystem initialization can go into infinite loop

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.0.2-alpha
    • Fix Version/s: 3.0.0, 2.0.3-alpha
    • Component/s: fs
    • Labels:
      None

      Description

      More information is there: https://jira.springsource.org/browse/SHDP-111

      Referenced source code from example is: https://github.com/SpringSource/spring-hadoop/blob/master/src/main/java/org/springframework/data/hadoop/configuration/ConfigurationFactoryBean.java

      from isolating that cause it looks like if you register: org.apache.hadoop.fs.FsUrlStreamHandlerFactory before calling FileSystem.loadFileSystems() then it goes into infinite loop.

      1. fstest.groovy
        0.6 kB
        Radim Kolar
      2. HADOOP-9041.patch
        0.9 kB
        Yanbo Liang
      3. fsinit-unit.txt
        2 kB
        Radim Kolar
      4. HADOOP-9041.patch
        3 kB
        Yanbo Liang
      5. fsinit2.txt
        3 kB
        Radim Kolar
      6. TestFileSystem.java
        0.7 kB
        Yanbo Liang
      7. fsinit3.txt
        3 kB
        Radim Kolar
      8. fsinit4.txt
        3 kB
        Radim Kolar
      9. fsinit5.txt
        3 kB
        Radim Kolar
      10. fsinit6.txt
        3 kB
        Radim Kolar

        Activity

        Hide
        Radim Kolar added a comment -

        Testcase

        Show
        Radim Kolar added a comment - Testcase
        Hide
        Yanbo Liang added a comment -

        The infinite loop caused by FileSystem.loadFileSystems(). If the users register org.apache.hadoop.fs.FsUrlStreamHandlerFactory as the current URLStreamHandlerFactory, then calling FsUrlStreamHandlerFactory.createURLStreamHandler() will lead to invoke FileSystem.loadFileSystems(). FileSystem.loadFileSystems() will cause to invoke FsUrlStreamHandlerFactory.createURLStreamHandler() when resolving a URL. Then the infinite loop occurs.

        Just moving the function of FileSystem.loadFileSystems() into a static code block. It will initialize as the construction of Class and only once.

        Looking forward for any comments.

        Show
        Yanbo Liang added a comment - The infinite loop caused by FileSystem.loadFileSystems(). If the users register org.apache.hadoop.fs.FsUrlStreamHandlerFactory as the current URLStreamHandlerFactory, then calling FsUrlStreamHandlerFactory.createURLStreamHandler() will lead to invoke FileSystem.loadFileSystems(). FileSystem.loadFileSystems() will cause to invoke FsUrlStreamHandlerFactory.createURLStreamHandler() when resolving a URL. Then the infinite loop occurs. Just moving the function of FileSystem.loadFileSystems() into a static code block. It will initialize as the construction of Class and only once. Looking forward for any comments.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12553639/HADOOP-9041.patch
        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 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any 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 in hadoop-common-project/hadoop-common:

        org.apache.hadoop.fs.viewfs.TestViewFsURIs
        org.apache.hadoop.fs.viewfs.TestChRootedFs
        org.apache.hadoop.fs.TestLocalFsFCStatistics
        org.apache.hadoop.fs.viewfs.TestViewFsWithAuthorityLocalFs
        org.apache.hadoop.fs.TestS3_LocalFileContextURI
        org.apache.hadoop.fs.TestFileContextResolveAfs
        org.apache.hadoop.fs.TestLocal_S3FileContextURI
        org.apache.hadoop.fs.TestLocalFSFileContextSymlink
        org.apache.hadoop.fs.viewfs.TestViewFsLocalFs
        org.apache.hadoop.fs.TestFileContextDeleteOnExit

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1749//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1749//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/12553639/HADOOP-9041.patch 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 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any 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 in hadoop-common-project/hadoop-common: org.apache.hadoop.fs.viewfs.TestViewFsURIs org.apache.hadoop.fs.viewfs.TestChRootedFs org.apache.hadoop.fs.TestLocalFsFCStatistics org.apache.hadoop.fs.viewfs.TestViewFsWithAuthorityLocalFs org.apache.hadoop.fs.TestS3_LocalFileContextURI org.apache.hadoop.fs.TestFileContextResolveAfs org.apache.hadoop.fs.TestLocal_S3FileContextURI org.apache.hadoop.fs.TestLocalFSFileContextSymlink org.apache.hadoop.fs.viewfs.TestViewFsLocalFs org.apache.hadoop.fs.TestFileContextDeleteOnExit +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1749//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1749//console This message is automatically generated.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12553761/HADOOP-9041.patch
        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 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any 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 hadoop-common-project/hadoop-common.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1760//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1760//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/12553761/HADOOP-9041.patch 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 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any 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 hadoop-common-project/hadoop-common. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1760//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1760//console This message is automatically generated.
        Hide
        Alejandro Abdelnur added a comment -

        Yanbo Liang, the change in the patch looks like it would not change the current behavior. Still I'm failing to see why a call to how FileSystem.loadFileSystems() gets to invoke FsUrlStreamHandlerFactory. AFAIK, this could happen only if the default constructor of the filesystem tries to create a URL with a FS scheme. Which FileSystem impl you've found it is doing that?

        Show
        Alejandro Abdelnur added a comment - Yanbo Liang, the change in the patch looks like it would not change the current behavior. Still I'm failing to see why a call to how FileSystem.loadFileSystems() gets to invoke FsUrlStreamHandlerFactory. AFAIK, this could happen only if the default constructor of the filesystem tries to create a URL with a FS scheme. Which FileSystem impl you've found it is doing that?
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12553900/HADOOP-9041.patch
        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 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any 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 hadoop-common-project/hadoop-common.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1768//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1768//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/12553900/HADOOP-9041.patch 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 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any 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 hadoop-common-project/hadoop-common. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1768//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1768//console This message is automatically generated.
        Hide
        Yanbo Liang added a comment -

        Hi Alejandro,
        My previous comment may be not very clear. The detail calling stack is described as follow:
        If users register org.apache.hadoop.fs.FsUrlStreamHandlerFactory as the current URLStreamHandlerFactory before calling FileSystem.getFileSystem()->FileSystem.loadFileSystems() will lead infinite loop.
        1) org.apache.hadoop.fs.FsUrlStreamHandlerFactory has been registered as the current URLStreamHandlerFactory.
        2) users call FileSystem.getFileSystem()->ClassFileSystem.loadFileSystems().
        3) Because before 2) users have never called FileSystem.loadFileSystems(), so it will execute the code of fuction FileSystem.loadFileSystems().
        4) In FileSystem.loadFileSystems(), it uses ServiceLoader to load providers of FileSystem such as hdfs, kfs, s3 and etc.
        5) When execute ServiceLoader, it need to read the providers of FileSystem from resource directory such as jar file on local disk. The ServiceLoader will recognize the jar file as URL.
        6) ServiceLoader create URL object and open stream to this URL.
        7) The URL need to find handler for a specific protocol such as "file:///" then it will call URL.getURLStreamHandler() and indirectly call FsUrlStreamHandlerFactory.createURLStreamHandler().
        8) At the function of FsUrlStreamHandlerFactory.createURLStreamHandler(), it need to recognize different file system schemes or protocols according to the providers of FileSystem (If the jar file is on local disk, it need to know the implementaion of LocalFileSystem). But at this time the providers of FileSystem had not loaded in memory, it will call FileSystem.getFileSystem("file",conf)->FileSystem.loadFileSystems(). We jump to step 2) and drop into infinite loop.

        Because the URL is closely relevent with concrete FileSystem implementations, we need to load FileSystem implemetations before any URL related operations. I mean to call FileSystem.getFileSystemClass("file",conf) in the construction of class FsUrlStreamHandlerFactory to solve this problem, because FsUrlStreamHandlerFactory need ensure to know the FileSystem implementation of scheme "file:///" at least and then it can work regularly.

        The patch had been attached. Looking forward to your comments.

        Show
        Yanbo Liang added a comment - Hi Alejandro, My previous comment may be not very clear. The detail calling stack is described as follow: If users register org.apache.hadoop.fs.FsUrlStreamHandlerFactory as the current URLStreamHandlerFactory before calling FileSystem.getFileSystem()->FileSystem.loadFileSystems() will lead infinite loop. 1) org.apache.hadoop.fs.FsUrlStreamHandlerFactory has been registered as the current URLStreamHandlerFactory. 2) users call FileSystem.getFileSystem()->ClassFileSystem.loadFileSystems(). 3) Because before 2) users have never called FileSystem.loadFileSystems(), so it will execute the code of fuction FileSystem.loadFileSystems(). 4) In FileSystem.loadFileSystems(), it uses ServiceLoader to load providers of FileSystem such as hdfs, kfs, s3 and etc. 5) When execute ServiceLoader, it need to read the providers of FileSystem from resource directory such as jar file on local disk. The ServiceLoader will recognize the jar file as URL. 6) ServiceLoader create URL object and open stream to this URL. 7) The URL need to find handler for a specific protocol such as "file:///" then it will call URL.getURLStreamHandler() and indirectly call FsUrlStreamHandlerFactory.createURLStreamHandler(). 8) At the function of FsUrlStreamHandlerFactory.createURLStreamHandler(), it need to recognize different file system schemes or protocols according to the providers of FileSystem (If the jar file is on local disk, it need to know the implementaion of LocalFileSystem). But at this time the providers of FileSystem had not loaded in memory, it will call FileSystem.getFileSystem("file",conf)->FileSystem.loadFileSystems(). We jump to step 2) and drop into infinite loop. Because the URL is closely relevent with concrete FileSystem implementations, we need to load FileSystem implemetations before any URL related operations. I mean to call FileSystem.getFileSystemClass("file",conf) in the construction of class FsUrlStreamHandlerFactory to solve this problem, because FsUrlStreamHandlerFactory need ensure to know the FileSystem implementation of scheme "file:///" at least and then it can work regularly. The patch had been attached. Looking forward to your comments.
        Hide
        Radim Kolar added a comment -

        I really need this to be committed soon.

        Show
        Radim Kolar added a comment - I really need this to be committed soon.
        Hide
        Tom White added a comment -

        Can you convert the Groovy test into a JUnit test so we have a regression test?

        Show
        Tom White added a comment - Can you convert the Groovy test into a JUnit test so we have a regression test?
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12554193/fsinit-unit.txt
        against trunk revision .

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

        +1 tests included. The patch appears to include 1 new or modified test files.

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

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any 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 hadoop-common-project/hadoop-common.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1770//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1770//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/12554193/fsinit-unit.txt against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any 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 hadoop-common-project/hadoop-common. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1770//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1770//console This message is automatically generated.
        Hide
        Alejandro Abdelnur added a comment -

        Yanbo Liang, thanks for the detailed explanation, yes it makes sense. As a side question, do you know why this has not been observed before? Also, as Tom said the testcase should be junit based.

        Show
        Alejandro Abdelnur added a comment - Yanbo Liang, thanks for the detailed explanation, yes it makes sense. As a side question, do you know why this has not been observed before? Also, as Tom said the testcase should be junit based.
        Hide
        Yanbo Liang added a comment -

        @Alejandro,
        There may be many reasons this has not been observed before:
        1, There are no test case cover this situation until now. In other words, no test case registering FsUrlStreamHandlerFactory as the current URLStreamHandlerFactory before calling FileSystem.getFileSystem()->FileSystem.loadFileSystems().
        2, The case is environment dependent. @Radim had attach the junit testcase, but the trunk still run success over this test case before we fix this bug. I also encounter this situation and run this test case base on trunk code on different environment, but with different result.

        1. simple java application, the test case failure as expected.
        2. maven+junit, the test case success(this is not expected).
        3. eclipse+junit, the test case failure as expected.

        I still work on tracing the different result in different environment.
        Thanks to @Radim for attaching the junit based test case.

        Show
        Yanbo Liang added a comment - @Alejandro, There may be many reasons this has not been observed before: 1, There are no test case cover this situation until now. In other words, no test case registering FsUrlStreamHandlerFactory as the current URLStreamHandlerFactory before calling FileSystem.getFileSystem()->FileSystem.loadFileSystems(). 2, The case is environment dependent. @Radim had attach the junit testcase, but the trunk still run success over this test case before we fix this bug. I also encounter this situation and run this test case base on trunk code on different environment, but with different result. simple java application, the test case failure as expected. maven+junit, the test case success(this is not expected). eclipse+junit, the test case failure as expected. I still work on tracing the different result in different environment. Thanks to @Radim for attaching the junit based test case.
        Hide
        Radim Kolar added a comment -

        in maven + junit it probably didnt fails because JVM is reused and its already initialized.

        Show
        Radim Kolar added a comment - in maven + junit it probably didnt fails because JVM is reused and its already initialized.
        Hide
        Yanbo Liang added a comment -

        @Radim, in maven+junit test case may not fail is not caused by reduplicative initialization. Because if reduplicative initialization occur, it will throw Error.
        As my opinion, it caused by the maven+junit framework has load the implementation of FileSystem before the test case running, so it will call ServiceLoader ahead of registering FsUrlStreamHandlerFactory all the time.
        So the test case can not reproduce in maven+junit.

        Show
        Yanbo Liang added a comment - @Radim, in maven+junit test case may not fail is not caused by reduplicative initialization. Because if reduplicative initialization occur, it will throw Error. As my opinion, it caused by the maven+junit framework has load the implementation of FileSystem before the test case running, so it will call ServiceLoader ahead of registering FsUrlStreamHandlerFactory all the time. So the test case can not reproduce in maven+junit.
        Hide
        Yanbo Liang added a comment -

        As my tracing the process of running test case in maven+junit environment, I have found that in this environment the ServiceLoader will directly load service providers from the file "target/class/META-INF/services/org.apache.hadoop.fs.FileSystem" rather than load from the jar package. Because much resource would be loaded from this directory "target/class/", so the classloader will reuse this URL and then reuse the URL handler for the scheme of "file". ServiceLoader will call classloader to load resource from the file in this directory, then the loading will not use the the handler we just register. Due to not use the handler we just register in FsUrlStreamHandlerFactory, the reusing handler will work well and not fall into the infinite loop.
        So this bug may be difficult to reproduce in junit environment. But in regular development, it will be dangerous and hidden deep. Until here I think my patch can resolve this problem and looking forward any comments.

        Show
        Yanbo Liang added a comment - As my tracing the process of running test case in maven+junit environment, I have found that in this environment the ServiceLoader will directly load service providers from the file "target/class/META-INF/services/org.apache.hadoop.fs.FileSystem" rather than load from the jar package. Because much resource would be loaded from this directory "target/class/", so the classloader will reuse this URL and then reuse the URL handler for the scheme of "file". ServiceLoader will call classloader to load resource from the file in this directory, then the loading will not use the the handler we just register. Due to not use the handler we just register in FsUrlStreamHandlerFactory, the reusing handler will work well and not fall into the infinite loop. So this bug may be difficult to reproduce in junit environment. But in regular development, it will be dangerous and hidden deep. Until here I think my patch can resolve this problem and looking forward any comments.
        Hide
        Radim Kolar added a comment -

        patch works fine

        Show
        Radim Kolar added a comment - patch works fine
        Hide
        Andy Isaacson added a comment -

        Please post a roll-up patch containing both the code change and the unit test. Also, TestFileSystemInicialization should be spelled TestFileSystemInitialization.

        Show
        Andy Isaacson added a comment - Please post a roll-up patch containing both the code change and the unit test. Also, TestFileSystemInicialization should be spelled TestFileSystemInitialization .
        Hide
        Yanbo Liang added a comment -

        roll-up patch including both the code change and the unit test.

        Show
        Yanbo Liang added a comment - roll-up patch including both the code change and the unit test.
        Hide
        Radim Kolar added a comment -

        @RunWith(PowerMockRunner.class) makes it fail as expected

        Show
        Radim Kolar added a comment - @RunWith(PowerMockRunner.class) makes it fail as expected
        Hide
        Radim Kolar added a comment -

        actually since powermock is not going in, commit it as it is.

        Show
        Radim Kolar added a comment - actually since powermock is not going in, commit it as it is.
        Hide
        Andy Isaacson added a comment -

        So this bug may be difficult to reproduce in junit environment. But in regular development, it will be dangerous and hidden deep. Until here I think my patch can resolve this problem and looking forward any comments.

        Yanbo Liang could you post instructions for how to reproduce the infinite loop failure using a simple java application? I've tried to reproduce it using Eclipse+junit (using JDK 1.6.0_24 on Linux) and have not managed to cause the failure. If you could upload or paste a standalone testcase that fails, plus instructions for how to run it (on whatever platform) that will help with manual testing.

        Show
        Andy Isaacson added a comment - So this bug may be difficult to reproduce in junit environment. But in regular development, it will be dangerous and hidden deep. Until here I think my patch can resolve this problem and looking forward any comments. Yanbo Liang could you post instructions for how to reproduce the infinite loop failure using a simple java application? I've tried to reproduce it using Eclipse+junit (using JDK 1.6.0_24 on Linux) and have not managed to cause the failure. If you could upload or paste a standalone testcase that fails, plus instructions for how to run it (on whatever platform) that will help with manual testing.
        Hide
        Radim Kolar added a comment -

        just run fstest.groovy it pulls library JARs, no need to setup anything.

        Show
        Radim Kolar added a comment - just run fstest.groovy it pulls library JARs, no need to setup anything.
        Hide
        Yanbo Liang added a comment -

        Andy Isaacson I just attached TestFileSystem.java, it can reproduce the infinite loop failure using the simple java application. You should add corresponding libraries to classpath and run this main class, then the infinite loop occurred.

        Show
        Yanbo Liang added a comment - Andy Isaacson I just attached TestFileSystem.java, it can reproduce the infinite loop failure using the simple java application. You should add corresponding libraries to classpath and run this main class, then the infinite loop occurred.
        Hide
        Andy Isaacson added a comment -

        Thanks, that helps. I was able to reproduce the infinite recursion using trunk on Linux with
        javac -classpath `hadoop classpath` TestFileSystem.java
        java -classpath `pwd`:`hadoop classpath` TestFileSystem

        and the stack trace

        ...
                at java.net.URL.getURLStreamHandler(URL.java:1107)
                at java.net.URL.<init>(URL.java:572)
                at java.net.URL.<init>(URL.java:464)
                at java.net.URL.<init>(URL.java:413)
                at java.net.JarURLConnection.parseSpecs(JarURLConnection.java:161)
                at java.net.JarURLConnection.<init>(JarURLConnection.java:144)
                at sun.net.www.protocol.jar.JarURLConnection.<init>(JarURLConnection.java:63)
                at sun.net.www.protocol.jar.Handler.openConnection(Handler.java:24)
                at java.net.URL.openConnection(URL.java:945)
                at java.net.URL.openStream(URL.java:1010)
                at java.util.ServiceLoader.parse(ServiceLoader.java:279)
                at java.util.ServiceLoader.access$200(ServiceLoader.java:164)
                at java.util.ServiceLoader$LazyIterator.hasNext(ServiceLoader.java:332)
                at java.util.ServiceLoader$1.hasNext(ServiceLoader.java:415)
                at org.apache.hadoop.fs.FileSystem.loadFileSystems(FileSystem.java:2232)
                at org.apache.hadoop.fs.FileSystem.getFileSystemClass(FileSystem.java:2243)
                at org.apache.hadoop.fs.FsUrlStreamHandlerFactory.createURLStreamHandler(FsUrlStreamHandlerFactory.java:67)
                at java.net.URL.getURLStreamHandler(URL.java:1107)
        ...
        
        Show
        Andy Isaacson added a comment - Thanks, that helps. I was able to reproduce the infinite recursion using trunk on Linux with javac -classpath `hadoop classpath` TestFileSystem.java java -classpath `pwd`:`hadoop classpath` TestFileSystem and the stack trace ... at java.net.URL.getURLStreamHandler(URL.java:1107) at java.net.URL.<init>(URL.java:572) at java.net.URL.<init>(URL.java:464) at java.net.URL.<init>(URL.java:413) at java.net.JarURLConnection.parseSpecs(JarURLConnection.java:161) at java.net.JarURLConnection.<init>(JarURLConnection.java:144) at sun.net.www.protocol.jar.JarURLConnection.<init>(JarURLConnection.java:63) at sun.net.www.protocol.jar.Handler.openConnection(Handler.java:24) at java.net.URL.openConnection(URL.java:945) at java.net.URL.openStream(URL.java:1010) at java.util.ServiceLoader.parse(ServiceLoader.java:279) at java.util.ServiceLoader.access$200(ServiceLoader.java:164) at java.util.ServiceLoader$LazyIterator.hasNext(ServiceLoader.java:332) at java.util.ServiceLoader$1.hasNext(ServiceLoader.java:415) at org.apache.hadoop.fs.FileSystem.loadFileSystems(FileSystem.java:2232) at org.apache.hadoop.fs.FileSystem.getFileSystemClass(FileSystem.java:2243) at org.apache.hadoop.fs.FsUrlStreamHandlerFactory.createURLStreamHandler(FsUrlStreamHandlerFactory.java:67) at java.net.URL.getURLStreamHandler(URL.java:1107) ...
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12560342/TestFileSystem.java
        against trunk revision .

        -1 patch. The patch command could not apply the patch.

        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1855//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/12560342/TestFileSystem.java against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1855//console This message is automatically generated.
        Hide
        Andy Isaacson added a comment -
        +    FileSystem.getFileSystemClass("file",conf);
        

        missing a space after the ",".

        +public class TestFileSystemInitialization {
        +
        +	/**
        

        Indentation is wonky, we do not use tabs and we use 2 space indents.

        +	 * Check if FileSystem can be properly initialized if URLStreamHandlerFactory
        +	 * is registered.
        +	 */
        

        Since this is such an obscure failure condition, please mention the JIRA in the comment so that someone who is curious does not have to dig in the VCS history to figure out what's going on.

        +		try {
        +			FileSystem.getFileSystemClass("file:", conf);
        +		}
        +		catch (IOException ok) {}
        +	}
        

        Please add asserts as appropriate to ensure that we follow the expected code path. For example, if we expect IOException, add assertFalse(true); after calling getFileSystemClass, with comments explaining the expected behavior. See TestLocalDirAllocator.java for an example.

        Show
        Andy Isaacson added a comment - + FileSystem.getFileSystemClass( "file" ,conf); missing a space after the ",". + public class TestFileSystemInitialization { + + /** Indentation is wonky, we do not use tabs and we use 2 space indents. + * Check if FileSystem can be properly initialized if URLStreamHandlerFactory + * is registered. + */ Since this is such an obscure failure condition, please mention the JIRA in the comment so that someone who is curious does not have to dig in the VCS history to figure out what's going on. + try { + FileSystem.getFileSystemClass( "file:" , conf); + } + catch (IOException ok) {} + } Please add asserts as appropriate to ensure that we follow the expected code path. For example, if we expect IOException , add assertFalse(true); after calling getFileSystemClass , with comments explaining the expected behavior. See TestLocalDirAllocator.java for an example.
        Hide
        Radim Kolar added a comment -

        patch reformatted

        Show
        Radim Kolar added a comment - patch reformatted
        Hide
        Hadoop QA added a comment -

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

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

        +1 tests included. The patch appears to include 1 new or modified test files.

        -1 javac. The patch appears to cause the build to fail.

        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1874//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/12561053/fsinit3.txt against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. -1 javac . The patch appears to cause the build to fail. Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1874//console This message is automatically generated.
        Hide
        Radim Kolar added a comment -

        added comments, fixed unit test, and do not throw IOException from initializer

        Show
        Radim Kolar added a comment - added comments, fixed unit test, and do not throw IOException from initializer
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12561133/fsinit4.txt
        against trunk revision .

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

        +1 tests included. The patch appears to include 1 new or modified test files.

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

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any 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 hadoop-common-project/hadoop-common.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1882//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1882//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/12561133/fsinit4.txt against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any 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 hadoop-common-project/hadoop-common. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1882//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1882//console This message is automatically generated.
        Hide
        Luke Lu added a comment -

        and do not throw IOException from initializer

        Why? I can't think of a case where you want to swallow the exception. At least log an error?

        Show
        Luke Lu added a comment - and do not throw IOException from initializer Why? I can't think of a case where you want to swallow the exception. At least log an error?
        Hide
        Radim Kolar added a comment -

        If there is an problem in FileSystem.getFileSystemClass("file", conf); then you will catch that exception later in correct code path, where it is called, expected and processed. Here it is just initializer.

        You might be interested in different filesystem "hdfs" and thus throwing an exception because "file" failed load increases complexity of initializer exception handling in caller.

        Show
        Radim Kolar added a comment - If there is an problem in FileSystem.getFileSystemClass("file", conf); then you will catch that exception later in correct code path, where it is called, expected and processed. Here it is just initializer. You might be interested in different filesystem "hdfs" and thus throwing an exception because "file" failed load increases complexity of initializer exception handling in caller.
        Hide
        Luke Lu added a comment -

        Something is seriously wrong if "file" handler cannot be loaded. I'd say swallowing the exception in this case really doesn't help debugging either.

        Show
        Luke Lu added a comment - Something is seriously wrong if "file" handler cannot be loaded. I'd say swallowing the exception in this case really doesn't help debugging either.
        Hide
        Radim Kolar added a comment -

        You missed my point. Try to read it again.

        Show
        Radim Kolar added a comment - You missed my point. Try to read it again.
        Hide
        Luke Lu added a comment -

        You missed MY point. I don't agree that "throwing an exception because "file" failed to load increase complexity of initializer exception handling in caller". On the contrary, things might fail mysteriously much later in unrelated code path. i.e. "hdfs" could load just fine because you swallowed the exception. I prefer it to fail early when things go wrong. When you say "you will catch that exception later", you're assuming the exception is deterministic. It could be that the first exception make the system get into a wrong/undefined state and you may never catch that exception later.

        I suggest that you simply convert the exception into a run time exception.

        Show
        Luke Lu added a comment - You missed MY point. I don't agree that "throwing an exception because "file" failed to load increase complexity of initializer exception handling in caller". On the contrary, things might fail mysteriously much later in unrelated code path. i.e. "hdfs" could load just fine because you swallowed the exception. I prefer it to fail early when things go wrong. When you say "you will catch that exception later", you're assuming the exception is deterministic. It could be that the first exception make the system get into a wrong/undefined state and you may never catch that exception later. I suggest that you simply convert the exception into a run time exception.
        Hide
        Radim Kolar added a comment -

        my point is do not increase conditional complexity of program. Runtime exception is fine.

        Show
        Radim Kolar added a comment - my point is do not increase conditional complexity of program. Runtime exception is fine.
        Hide
        Hadoop QA added a comment -

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

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

        +1 tests included. The patch appears to include 1 new or modified test files.

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

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any 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 in hadoop-common-project/hadoop-common:

        org.apache.hadoop.ha.TestZKFailoverController

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1887//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1887//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/12561140/fsinit5.txt against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any 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 in hadoop-common-project/hadoop-common: org.apache.hadoop.ha.TestZKFailoverController +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1887//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1887//console This message is automatically generated.
        Hide
        Luke Lu added a comment -

        The logic of fsinit5 lgtm. OTOH, you should fix your ide/editor settings. Hadoop indention is 2 spaces, not 3, or 4. The patch has all 3 kinds of indentations.

        Show
        Luke Lu added a comment - The logic of fsinit5 lgtm. OTOH, you should fix your ide/editor settings. Hadoop indention is 2 spaces, not 3, or 4. The patch has all 3 kinds of indentations.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12561146/fsinit6.txt
        against trunk revision .

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

        +1 tests included. The patch appears to include 1 new or modified test files.

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

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any 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 hadoop-common-project/hadoop-common.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1888//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1888//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/12561146/fsinit6.txt against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any 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 hadoop-common-project/hadoop-common. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/1888//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/1888//console This message is automatically generated.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-trunk-Commit #3128 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3128/)
        HADOOP-9041. FsUrlStreamHandlerFactory could cause an infinite loop in FileSystem initialization. (Yanbo Liang and Radim Kolar via llu) (Revision 1422430)

        Result = SUCCESS
        llu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1422430
        Files :

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FsUrlStreamHandlerFactory.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileSystemInitialization.java
        Show
        Hudson added a comment - Integrated in Hadoop-trunk-Commit #3128 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3128/ ) HADOOP-9041 . FsUrlStreamHandlerFactory could cause an infinite loop in FileSystem initialization. (Yanbo Liang and Radim Kolar via llu) (Revision 1422430) Result = SUCCESS llu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1422430 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FsUrlStreamHandlerFactory.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileSystemInitialization.java
        Hide
        Luke Lu added a comment -

        Committed to trunk and branch-2. Thanks Yanbo and Radim for the patch, Alejandro, Tom and Andy for the reviews.

        Show
        Luke Lu added a comment - Committed to trunk and branch-2. Thanks Yanbo and Radim for the patch, Alejandro, Tom and Andy for the reviews.
        Hide
        Luke Lu added a comment -

        Note branch-1* FsUrlStreamHandlerFactory doesn't have the bug since the first version. The bug is introduced in HADOOP-8321.

        Show
        Luke Lu added a comment - Note branch-1* FsUrlStreamHandlerFactory doesn't have the bug since the first version. The bug is introduced in HADOOP-8321 .
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Yarn-trunk #67 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/67/)
        HADOOP-9041. FsUrlStreamHandlerFactory could cause an infinite loop in FileSystem initialization. (Yanbo Liang and Radim Kolar via llu) (Revision 1422430)

        Result = SUCCESS
        llu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1422430
        Files :

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FsUrlStreamHandlerFactory.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileSystemInitialization.java
        Show
        Hudson added a comment - Integrated in Hadoop-Yarn-trunk #67 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/67/ ) HADOOP-9041 . FsUrlStreamHandlerFactory could cause an infinite loop in FileSystem initialization. (Yanbo Liang and Radim Kolar via llu) (Revision 1422430) Result = SUCCESS llu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1422430 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FsUrlStreamHandlerFactory.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileSystemInitialization.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk #1256 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1256/)
        HADOOP-9041. FsUrlStreamHandlerFactory could cause an infinite loop in FileSystem initialization. (Yanbo Liang and Radim Kolar via llu) (Revision 1422430)

        Result = FAILURE
        llu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1422430
        Files :

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FsUrlStreamHandlerFactory.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileSystemInitialization.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1256 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1256/ ) HADOOP-9041 . FsUrlStreamHandlerFactory could cause an infinite loop in FileSystem initialization. (Yanbo Liang and Radim Kolar via llu) (Revision 1422430) Result = FAILURE llu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1422430 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FsUrlStreamHandlerFactory.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileSystemInitialization.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk #1287 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1287/)
        HADOOP-9041. FsUrlStreamHandlerFactory could cause an infinite loop in FileSystem initialization. (Yanbo Liang and Radim Kolar via llu) (Revision 1422430)

        Result = SUCCESS
        llu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1422430
        Files :

        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FsUrlStreamHandlerFactory.java
        • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileSystemInitialization.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1287 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1287/ ) HADOOP-9041 . FsUrlStreamHandlerFactory could cause an infinite loop in FileSystem initialization. (Yanbo Liang and Radim Kolar via llu) (Revision 1422430) Result = SUCCESS llu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1422430 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FsUrlStreamHandlerFactory.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileSystemInitialization.java

          People

          • Assignee:
            Radim Kolar
            Reporter:
            Radim Kolar
          • Votes:
            0 Vote for this issue
            Watchers:
            11 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development