Uploaded image for project: 'Hadoop Common'
  1. Hadoop Common
  2. HADOOP-12551

Introduce FileNotFoundException for WASB FileSystem API

    Details

    • Type: Bug
    • 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
    • Hadoop Flags:
      Reviewed

      Description

      HADOOP-12533 introduced FileNotFoundException to the read and seek API for WASB. The open and getFileStatus api currently throws FileNotFoundException correctly when the file does not exists when the API is called but does not throw the same exception if there is another thread/process deletes the file during its execution. This Jira fixes that behavior.

      This jira also re-examines other Azure storage store calls to check for BlobNotFoundException in setPermission(), setOwner, rename(), delete(), open(), listStatus() APIs.

      1. HADOOP-12551.004.patch
        47 kB
        Dushyanth
      2. HADOOP-12551.003.patch
        48 kB
        Dushyanth
      3. HADOOP-12551.002.patch
        48 kB
        Dushyanth
      4. HADOOP-12551.001.patch
        49 kB
        Dushyanth

        Activity

        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-trunk-Commit #9079 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9079/)
        HADOOP-12551. Introduce FileNotFoundException for WASB FileSystem API. (cnauroth: rev 0e76f1fceaaaeb66bdb4818f43b9a55fc092bf79)

        • hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestFileSystemOperationExceptionHandling.java
        • hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestFileSystemOperationsExceptionHandlingMultiThreaded.java
        • hadoop-common-project/hadoop-common/CHANGES.txt
        • hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/NativeAzureFileSystem.java
        • hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/ExceptionHandlingTestHelper.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #9079 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9079/ ) HADOOP-12551 . Introduce FileNotFoundException for WASB FileSystem API. (cnauroth: rev 0e76f1fceaaaeb66bdb4818f43b9a55fc092bf79) hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestFileSystemOperationExceptionHandling.java hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/TestFileSystemOperationsExceptionHandlingMultiThreaded.java hadoop-common-project/hadoop-common/CHANGES.txt hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/NativeAzureFileSystem.java hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/ExceptionHandlingTestHelper.java
        Hide
        cnauroth Chris Nauroth added a comment -

        +1 for patch v004. I have committed this to trunk, branch-2 and branch-2.8. Dushyanth, thank you for the patch.

        Show
        cnauroth Chris Nauroth added a comment - +1 for patch v004. I have committed this to trunk, branch-2 and branch-2.8. Dushyanth , thank you for the patch.
        Hide
        hadoopqa Hadoop QA added a comment -
        +1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 0s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
        +1 mvninstall 8m 23s trunk passed
        +1 compile 0m 19s trunk passed with JDK v1.8.0_66
        +1 compile 0m 18s trunk passed with JDK v1.7.0_91
        +1 checkstyle 0m 17s trunk passed
        +1 mvnsite 0m 24s trunk passed
        +1 mvneclipse 0m 13s trunk passed
        +1 findbugs 0m 34s trunk passed
        +1 javadoc 0m 14s trunk passed with JDK v1.8.0_66
        +1 javadoc 0m 15s trunk passed with JDK v1.7.0_91
        +1 mvninstall 0m 17s the patch passed
        +1 compile 0m 13s the patch passed with JDK v1.8.0_66
        +1 javac 0m 13s the patch passed
        +1 compile 0m 15s the patch passed with JDK v1.7.0_91
        +1 javac 0m 15s the patch passed
        +1 checkstyle 0m 9s the patch passed
        +1 mvnsite 0m 19s the patch passed
        +1 mvneclipse 0m 9s the patch passed
        +1 whitespace 0m 0s Patch has no whitespace issues.
        +1 findbugs 0m 38s the patch passed
        +1 javadoc 0m 11s the patch passed with JDK v1.8.0_66
        +1 javadoc 0m 12s the patch passed with JDK v1.7.0_91
        +1 unit 1m 11s hadoop-azure in the patch passed with JDK v1.8.0_66.
        +1 unit 1m 25s hadoop-azure in the patch passed with JDK v1.7.0_91.
        +1 asflicense 0m 20s Patch does not generate ASF License warnings.
        17m 26s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:0ca8df7
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12781427/HADOOP-12551.004.patch
        JIRA Issue HADOOP-12551
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 8054c9dd7c0f 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
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / dec8dfd
        Default Java 1.7.0_91
        Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91
        findbugs v3.0.0
        JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8376/testReport/
        modules C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure
        Max memory used 76MB
        Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org
        Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8376/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. +1 mvninstall 8m 23s trunk passed +1 compile 0m 19s trunk passed with JDK v1.8.0_66 +1 compile 0m 18s trunk passed with JDK v1.7.0_91 +1 checkstyle 0m 17s trunk passed +1 mvnsite 0m 24s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 0m 34s trunk passed +1 javadoc 0m 14s trunk passed with JDK v1.8.0_66 +1 javadoc 0m 15s trunk passed with JDK v1.7.0_91 +1 mvninstall 0m 17s the patch passed +1 compile 0m 13s the patch passed with JDK v1.8.0_66 +1 javac 0m 13s the patch passed +1 compile 0m 15s the patch passed with JDK v1.7.0_91 +1 javac 0m 15s the patch passed +1 checkstyle 0m 9s the patch passed +1 mvnsite 0m 19s the patch passed +1 mvneclipse 0m 9s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 0m 38s the patch passed +1 javadoc 0m 11s the patch passed with JDK v1.8.0_66 +1 javadoc 0m 12s the patch passed with JDK v1.7.0_91 +1 unit 1m 11s hadoop-azure in the patch passed with JDK v1.8.0_66. +1 unit 1m 25s hadoop-azure in the patch passed with JDK v1.7.0_91. +1 asflicense 0m 20s Patch does not generate ASF License warnings. 17m 26s Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12781427/HADOOP-12551.004.patch JIRA Issue HADOOP-12551 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 8054c9dd7c0f 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 Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / dec8dfd Default Java 1.7.0_91 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91 findbugs v3.0.0 JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8376/testReport/ modules C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure Max memory used 76MB Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8376/console This message was automatically generated.
        Hide
        dchickabasapa Dushyanth added a comment -

        Chris Nauroth I apologise, I mis-interpreted the comment. I have changed all the multi-threaded tests to be conistent in v004 of the patch.

        Testing is same as v003

        Show
        dchickabasapa Dushyanth added a comment - Chris Nauroth I apologise, I mis-interpreted the comment. I have changed all the multi-threaded tests to be conistent in v004 of the patch. Testing is same as v003
        Hide
        cnauroth Chris Nauroth added a comment -

        The changes to address the FindBugs warnings look good.

        Regarding the multi-threaded testing suggestion, I see the change in testMultiThreadedPageBlobOpenScenario, but there are other tests still using while (runCount < maxCount). Sorry if this wasn't clear, but I actually wanted to suggest making the change consistently across all of these tests. Could you please revise the patch again? I think that's the last change we'll need.

        Show
        cnauroth Chris Nauroth added a comment - The changes to address the FindBugs warnings look good. Regarding the multi-threaded testing suggestion, I see the change in testMultiThreadedPageBlobOpenScenario , but there are other tests still using while (runCount < maxCount) . Sorry if this wasn't clear, but I actually wanted to suggest making the change consistently across all of these tests. Could you please revise the patch again? I think that's the last change we'll need.
        Hide
        hadoopqa Hadoop QA added a comment -
        +1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 0s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
        +1 mvninstall 8m 22s trunk passed
        +1 compile 0m 15s trunk passed with JDK v1.8.0_66
        +1 compile 0m 16s trunk passed with JDK v1.7.0_91
        +1 checkstyle 0m 10s trunk passed
        +1 mvnsite 0m 22s trunk passed
        +1 mvneclipse 0m 42s trunk passed
        +1 findbugs 0m 31s trunk passed
        +1 javadoc 0m 13s trunk passed with JDK v1.8.0_66
        +1 javadoc 0m 14s trunk passed with JDK v1.7.0_91
        +1 mvninstall 0m 17s the patch passed
        +1 compile 0m 12s the patch passed with JDK v1.8.0_66
        +1 javac 0m 12s the patch passed
        +1 compile 0m 14s the patch passed with JDK v1.7.0_91
        +1 javac 0m 14s the patch passed
        +1 checkstyle 0m 9s the patch passed
        +1 mvnsite 0m 19s the patch passed
        +1 mvneclipse 0m 10s the patch passed
        +1 whitespace 0m 0s Patch has no whitespace issues.
        +1 findbugs 0m 36s the patch passed
        +1 javadoc 0m 10s the patch passed with JDK v1.8.0_66
        +1 javadoc 0m 12s the patch passed with JDK v1.7.0_91
        +1 unit 1m 8s hadoop-azure in the patch passed with JDK v1.8.0_66.
        +1 unit 1m 22s hadoop-azure in the patch passed with JDK v1.7.0_91.
        +1 asflicense 0m 19s Patch does not generate ASF License warnings.
        17m 21s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:0ca8df7
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12781359/HADOOP-12551.003.patch
        JIRA Issue HADOOP-12551
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 2371df030153 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
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 109e528
        Default Java 1.7.0_91
        Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91
        findbugs v3.0.0
        JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8371/testReport/
        modules C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure
        Max memory used 75MB
        Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org
        Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8371/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. +1 mvninstall 8m 22s trunk passed +1 compile 0m 15s trunk passed with JDK v1.8.0_66 +1 compile 0m 16s trunk passed with JDK v1.7.0_91 +1 checkstyle 0m 10s trunk passed +1 mvnsite 0m 22s trunk passed +1 mvneclipse 0m 42s trunk passed +1 findbugs 0m 31s trunk passed +1 javadoc 0m 13s trunk passed with JDK v1.8.0_66 +1 javadoc 0m 14s trunk passed with JDK v1.7.0_91 +1 mvninstall 0m 17s the patch passed +1 compile 0m 12s the patch passed with JDK v1.8.0_66 +1 javac 0m 12s the patch passed +1 compile 0m 14s the patch passed with JDK v1.7.0_91 +1 javac 0m 14s the patch passed +1 checkstyle 0m 9s the patch passed +1 mvnsite 0m 19s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 0m 36s the patch passed +1 javadoc 0m 10s the patch passed with JDK v1.8.0_66 +1 javadoc 0m 12s the patch passed with JDK v1.7.0_91 +1 unit 1m 8s hadoop-azure in the patch passed with JDK v1.8.0_66. +1 unit 1m 22s hadoop-azure in the patch passed with JDK v1.7.0_91. +1 asflicense 0m 19s Patch does not generate ASF License warnings. 17m 21s Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12781359/HADOOP-12551.003.patch JIRA Issue HADOOP-12551 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 2371df030153 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 Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 109e528 Default Java 1.7.0_91 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91 findbugs v3.0.0 JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8371/testReport/ modules C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure Max memory used 75MB Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8371/console This message was automatically generated.
        Hide
        dchickabasapa Dushyanth added a comment -

        Testing done for Patch v003 - The newly added tests were executed along with Contract Live tests for Block and Page blobs.

        Show
        dchickabasapa Dushyanth added a comment - Testing done for Patch v003 - The newly added tests were executed along with Contract Live tests for Block and Page blobs.
        Hide
        dchickabasapa Dushyanth added a comment -

        Chris Nauroth This actually is an issue with the way the try-catch. We should not be swallowing the storage exception in scenarios where its a StorageException and not FileNotFoundException. There were other places where I had used this paradigm, I have fixed those try-catch blocks.

        Show
        dchickabasapa Dushyanth added a comment - Chris Nauroth This actually is an issue with the way the try-catch. We should not be swallowing the storage exception in scenarios where its a StorageException and not FileNotFoundException. There were other places where I had used this paradigm, I have fixed those try-catch blocks.
        Hide
        cnauroth Chris Nauroth added a comment -

        While submitting the patch I built it using checkstyle flag, but didn't find any new style check warnings.

        I forgot that hadoop-azure has an overridden checkstyle.xml. That's something to clean up and align with the rest of the codebase at some point, but not in scope of this JIRA.

        I have commented the code explaining that we would not be hitting the NullReferenceException in the code path that the warnings are raised.

        I think Findbugs correctly spotted a bug, but it's in the exception handling:

              if (renamed) {
               listing = null;
               try {
                 listing = store.list(key, AZURE_LIST_ALL, 1, partialKey);
               } catch (IOException ex) {
                 Throwable innerException = NativeAzureFileSystem.checkForAzureStorageException(ex);
        
                 if (innerException instanceof StorageException) {
                   if (NativeAzureFileSystem.isFileNotFoundException((StorageException) innerException)) {
                     throw new FileNotFoundException(String.format("%s is not found", key));
                   }
                 } else {
                   throw ex;
                 }
               }
              }
        

        If the exception is a StorageException, but not a FileNotFoundException, then the StorageException is swallowed instead of propagated to the caller. Findbugs has identified correctly that there is a potential code path for execution to continue and dereference listing, which would be null. I recommend changing the exception handling to this:

               } catch (IOException ex) {
                 Throwable innerException = NativeAzureFileSystem.checkForAzureStorageException(ex);
        
                 if (innerException != null &&
                     innerException instanceof StorageException &&
                     NativeAzureFileSystem.isFileNotFoundException((StorageException) innerException)) {
                   throw new FileNotFoundException(String.format("%s is not found", key));
                 } else {
                   throw ex;
                 }
               }
        
        Show
        cnauroth Chris Nauroth added a comment - While submitting the patch I built it using checkstyle flag, but didn't find any new style check warnings. I forgot that hadoop-azure has an overridden checkstyle.xml. That's something to clean up and align with the rest of the codebase at some point, but not in scope of this JIRA. I have commented the code explaining that we would not be hitting the NullReferenceException in the code path that the warnings are raised. I think Findbugs correctly spotted a bug, but it's in the exception handling: if (renamed) { listing = null ; try { listing = store.list(key, AZURE_LIST_ALL, 1, partialKey); } catch (IOException ex) { Throwable innerException = NativeAzureFileSystem.checkForAzureStorageException(ex); if (innerException instanceof StorageException) { if (NativeAzureFileSystem.isFileNotFoundException((StorageException) innerException)) { throw new FileNotFoundException( String .format( "%s is not found" , key)); } } else { throw ex; } } } If the exception is a StorageException , but not a FileNotFoundException , then the StorageException is swallowed instead of propagated to the caller. Findbugs has identified correctly that there is a potential code path for execution to continue and dereference listing , which would be null . I recommend changing the exception handling to this: } catch (IOException ex) { Throwable innerException = NativeAzureFileSystem.checkForAzureStorageException(ex); if (innerException != null && innerException instanceof StorageException && NativeAzureFileSystem.isFileNotFoundException((StorageException) innerException)) { throw new FileNotFoundException( String .format( "%s is not found" , key)); } else { throw ex; } }
        Hide
        dchickabasapa Dushyanth added a comment -

        Chris Nauroth I see that there are two findbugs warnings in the QA run. I have commented the code explaining that we would not be hitting the NullReferenceException in the code path that the warnings are raised. I am leaving this as a No-op unless you feel it needs to be fixed.

        Show
        dchickabasapa Dushyanth added a comment - Chris Nauroth I see that there are two findbugs warnings in the QA run. I have commented the code explaining that we would not be hitting the NullReferenceException in the code path that the warnings are raised. I am leaving this as a No-op unless you feel it needs to be fixed.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 0s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
        +1 mvninstall 7m 32s trunk passed
        +1 compile 0m 13s trunk passed with JDK v1.8.0_66
        +1 compile 0m 16s trunk passed with JDK v1.7.0_91
        +1 checkstyle 0m 7s trunk passed
        +1 mvnsite 0m 21s trunk passed
        +1 mvneclipse 0m 12s trunk passed
        +1 findbugs 0m 30s trunk passed
        +1 javadoc 0m 12s trunk passed with JDK v1.8.0_66
        +1 javadoc 0m 14s trunk passed with JDK v1.7.0_91
        +1 mvninstall 0m 14s the patch passed
        +1 compile 0m 12s the patch passed with JDK v1.8.0_66
        +1 javac 0m 12s the patch passed
        +1 compile 0m 13s the patch passed with JDK v1.7.0_91
        +1 javac 0m 13s the patch passed
        +1 checkstyle 0m 8s the patch passed
        +1 mvnsite 0m 19s the patch passed
        +1 mvneclipse 0m 9s the patch passed
        +1 whitespace 0m 0s Patch has no whitespace issues.
        -1 findbugs 0m 36s hadoop-tools/hadoop-azure introduced 2 new FindBugs issues.
        +1 javadoc 0m 9s the patch passed with JDK v1.8.0_66
        +1 javadoc 0m 11s the patch passed with JDK v1.7.0_91
        +1 unit 1m 5s hadoop-azure in the patch passed with JDK v1.8.0_66.
        +1 unit 1m 17s hadoop-azure in the patch passed with JDK v1.7.0_91.
        +1 asflicense 0m 18s Patch does not generate ASF License warnings.
        15m 29s



        Reason Tests
        FindBugs module:hadoop-tools/hadoop-azure
          Possible null pointer dereference of listing in org.apache.hadoop.fs.azure.NativeAzureFileSystem.listStatus(Path) on exception path Dereferenced at NativeAzureFileSystem.java:listing in org.apache.hadoop.fs.azure.NativeAzureFileSystem.listStatus(Path) on exception path Dereferenced at NativeAzureFileSystem.java:[line 1961]
          Null passed for non-null parameter of conditionalRedoFolderRenames(PartialListing) in org.apache.hadoop.fs.azure.NativeAzureFileSystem.listStatus(Path) Method invoked at NativeAzureFileSystem.java:of conditionalRedoFolderRenames(PartialListing) in org.apache.hadoop.fs.azure.NativeAzureFileSystem.listStatus(Path) Method invoked at NativeAzureFileSystem.java:[line 1937]



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:0ca8df7
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12781335/HADOOP-12551.002.patch
        JIRA Issue HADOOP-12551
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 86c7a81119b1 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
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 109e528
        Default Java 1.7.0_91
        Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91
        findbugs v3.0.0
        findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/8370/artifact/patchprocess/new-findbugs-hadoop-tools_hadoop-azure.html
        JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8370/testReport/
        modules C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure
        Max memory used 76MB
        Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org
        Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8370/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. +1 mvninstall 7m 32s trunk passed +1 compile 0m 13s trunk passed with JDK v1.8.0_66 +1 compile 0m 16s trunk passed with JDK v1.7.0_91 +1 checkstyle 0m 7s trunk passed +1 mvnsite 0m 21s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 0m 30s trunk passed +1 javadoc 0m 12s trunk passed with JDK v1.8.0_66 +1 javadoc 0m 14s trunk passed with JDK v1.7.0_91 +1 mvninstall 0m 14s the patch passed +1 compile 0m 12s the patch passed with JDK v1.8.0_66 +1 javac 0m 12s the patch passed +1 compile 0m 13s the patch passed with JDK v1.7.0_91 +1 javac 0m 13s the patch passed +1 checkstyle 0m 8s the patch passed +1 mvnsite 0m 19s the patch passed +1 mvneclipse 0m 9s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. -1 findbugs 0m 36s hadoop-tools/hadoop-azure introduced 2 new FindBugs issues. +1 javadoc 0m 9s the patch passed with JDK v1.8.0_66 +1 javadoc 0m 11s the patch passed with JDK v1.7.0_91 +1 unit 1m 5s hadoop-azure in the patch passed with JDK v1.8.0_66. +1 unit 1m 17s hadoop-azure in the patch passed with JDK v1.7.0_91. +1 asflicense 0m 18s Patch does not generate ASF License warnings. 15m 29s Reason Tests FindBugs module:hadoop-tools/hadoop-azure   Possible null pointer dereference of listing in org.apache.hadoop.fs.azure.NativeAzureFileSystem.listStatus(Path) on exception path Dereferenced at NativeAzureFileSystem.java:listing in org.apache.hadoop.fs.azure.NativeAzureFileSystem.listStatus(Path) on exception path Dereferenced at NativeAzureFileSystem.java: [line 1961]   Null passed for non-null parameter of conditionalRedoFolderRenames(PartialListing) in org.apache.hadoop.fs.azure.NativeAzureFileSystem.listStatus(Path) Method invoked at NativeAzureFileSystem.java:of conditionalRedoFolderRenames(PartialListing) in org.apache.hadoop.fs.azure.NativeAzureFileSystem.listStatus(Path) Method invoked at NativeAzureFileSystem.java: [line 1937] Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12781335/HADOOP-12551.002.patch JIRA Issue HADOOP-12551 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 86c7a81119b1 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 Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 109e528 Default Java 1.7.0_91 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91 findbugs v3.0.0 findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/8370/artifact/patchprocess/new-findbugs-hadoop-tools_hadoop-azure.html JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8370/testReport/ modules C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure Max memory used 76MB Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8370/console This message was automatically generated.
        Hide
        dchickabasapa Dushyanth added a comment -

        Chris Nauroth Thanks a lot for the review. Attaching patch with the Multi-threaded test fix you suggested.

        While submitting the patch I built it using checkstyle flag, but didn't find any new style check warnings. So I am kicking of a QA build, to see if I am missing anything.

        Testing: The patch contains new tests to verify the changes made. Also changes have been tested against FileSystemContractLive tests for the both Block Blobs and Page Blobs.

        Show
        dchickabasapa Dushyanth added a comment - Chris Nauroth Thanks a lot for the review. Attaching patch with the Multi-threaded test fix you suggested. While submitting the patch I built it using checkstyle flag, but didn't find any new style check warnings. So I am kicking of a QA build, to see if I am missing anything. Testing: The patch contains new tests to verify the changes made. Also changes have been tested against FileSystemContractLive tests for the both Block Blobs and Page Blobs.
        Hide
        cnauroth Chris Nauroth added a comment -

        Dushyanth, thank you for the patch. This looks good overall. Just 2 comments:

        1. I think there are going to be some warnings on code style, such as lines that are not less than 80 characters. Clicking Submit Patch for a pre-commit run would reveal the specifics.
        2. The multi-threaded tests do this:
              // Setting the value to 1000. This is to ensure that
              // delete thread would get a chance to run at some point
              // and would be able to delete the test file.
              int maxCount = 1000;
              int runCount = 1;
              while (runCount < maxCount) {
                inputStream = fs.open(testPath);
                inputStream.close();
              }
            }
          

          Potentially a more reliable way to do this is:

              while (t.isAlive()) {
                inputStream = fs.open(testPath);
                inputStream.close();
              }
              inputStream = fs.open(testPath);
              inputStream.close();
          

          ...where t refers to the DeleteThread instance. This would keep trying until the thread runs to completion. The extra open outside the loop is for the edge case that the loop body doesn't get a chance to execute after the thread completes the delete. The extra open would throw the FileNotFoundException in that case.

        Show
        cnauroth Chris Nauroth added a comment - Dushyanth , thank you for the patch. This looks good overall. Just 2 comments: I think there are going to be some warnings on code style, such as lines that are not less than 80 characters. Clicking Submit Patch for a pre-commit run would reveal the specifics. The multi-threaded tests do this: // Setting the value to 1000. This is to ensure that // delete thread would get a chance to run at some point // and would be able to delete the test file. int maxCount = 1000; int runCount = 1; while (runCount < maxCount) { inputStream = fs.open(testPath); inputStream.close(); } } Potentially a more reliable way to do this is: while (t.isAlive()) { inputStream = fs.open(testPath); inputStream.close(); } inputStream = fs.open(testPath); inputStream.close(); ...where t refers to the DeleteThread instance. This would keep trying until the thread runs to completion. The extra open outside the loop is for the edge case that the loop body doesn't get a chance to execute after the thread completes the delete. The extra open would throw the FileNotFoundException in that case.
        Hide
        dchickabasapa Dushyanth added a comment -

        Attaching first iteration for the JIRA.

        The patch contains modification to the handling of calls made to the Azure Storage layer wrapped in a try catch block to handle BlobNotFound exception. The patch handles open(), rename(), delete(), listStatus(), setOwner(), setPermission() APIs.

        Testing: The patch contains new tests to verify the changes made. Also changes have been tested against FileSystemContractLive tests for the both Block Blobs and Page Blobs.

        Show
        dchickabasapa Dushyanth added a comment - Attaching first iteration for the JIRA. The patch contains modification to the handling of calls made to the Azure Storage layer wrapped in a try catch block to handle BlobNotFound exception. The patch handles open(), rename(), delete(), listStatus(), setOwner(), setPermission() APIs. Testing: The patch contains new tests to verify the changes made. Also changes have been tested against FileSystemContractLive tests for the both Block Blobs and Page Blobs.

          People

          • Assignee:
            dchickabasapa Dushyanth
            Reporter:
            dchickabasapa Dushyanth
          • Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development