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

Change Lazy Rename Pending Operation Completion of WASB to address case of potential data loss due to partial copy

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.8.0, 3.0.0-alpha1
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      HADOOP-12334 changed mode of Copy Operation of HBase WAL Archiving to bypass Azure Storage Throttling after retries. This was via client side copy. However a process crash when the copy is partially done would result in a scenario where the source and destination blobs will have different contents and lazy rename pending operation will not handle this thus causing data loss. We need to fix the lazy rename pending operation to address this issue

      1. HADOOP-12634.01.patch
        4 kB
        Gaurav Kanade
      2. HADOOP-12634.02.patch
        4 kB
        Gaurav Kanade

        Issue Links

          Activity

          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 1 new or modified test files.
          +1 mvninstall 8m 19s 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 9s trunk passed
          +1 mvnsite 0m 23s trunk passed
          +1 mvneclipse 0m 11s 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 18s the patch passed
          +1 compile 0m 14s the patch passed with JDK v1.8.0_66
          +1 javac 0m 14s the patch passed
          +1 compile 0m 16s the patch passed with JDK v1.7.0_91
          +1 javac 0m 15s the patch passed
          +1 checkstyle 0m 8s the patch passed
          +1 mvnsite 0m 22s the patch passed
          +1 mvneclipse 0m 13s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 0m 40s the patch passed
          +1 javadoc 0m 12s the patch passed with JDK v1.8.0_66
          +1 javadoc 0m 14s the patch passed with JDK v1.7.0_91
          +1 unit 1m 10s hadoop-azure in the patch passed with JDK v1.8.0_66.
          +1 unit 1m 23s hadoop-azure in the patch passed with JDK v1.7.0_91.
          +1 asflicense 0m 24s Patch does not generate ASF License warnings.
          17m 16s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12777080/HADOOP-12634.01.patch
          JIRA Issue HADOOP-12634
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 48df860487c7 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 / 576b569
          findbugs v3.0.0
          JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8222/testReport/
          modules C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure
          Max memory used 76MB
          Powered by Apache Yetus 0.1.0-SNAPSHOT http://yetus.apache.org
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8222/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 1 new or modified test files. +1 mvninstall 8m 19s 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 9s trunk passed +1 mvnsite 0m 23s trunk passed +1 mvneclipse 0m 11s 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 18s the patch passed +1 compile 0m 14s the patch passed with JDK v1.8.0_66 +1 javac 0m 14s the patch passed +1 compile 0m 16s the patch passed with JDK v1.7.0_91 +1 javac 0m 15s the patch passed +1 checkstyle 0m 8s the patch passed +1 mvnsite 0m 22s the patch passed +1 mvneclipse 0m 13s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 0m 40s the patch passed +1 javadoc 0m 12s the patch passed with JDK v1.8.0_66 +1 javadoc 0m 14s the patch passed with JDK v1.7.0_91 +1 unit 1m 10s hadoop-azure in the patch passed with JDK v1.8.0_66. +1 unit 1m 23s hadoop-azure in the patch passed with JDK v1.7.0_91. +1 asflicense 0m 24s Patch does not generate ASF License warnings. 17m 16s Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12777080/HADOOP-12634.01.patch JIRA Issue HADOOP-12634 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 48df860487c7 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 / 576b569 findbugs v3.0.0 JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8222/testReport/ modules C: hadoop-tools/hadoop-azure U: hadoop-tools/hadoop-azure Max memory used 76MB Powered by Apache Yetus 0.1.0-SNAPSHOT http://yetus.apache.org Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8222/console This message was automatically generated.
          Hide
          cnauroth Chris Nauroth added a comment -

          Gaurav Kanade, this mostly looks good to me. I just have a comment on testing and a nitpick.

          The test added in TestNativeAzureFileSystemLive does not exercise the code change. It does not execute against an atomic rename folder. See TestNativeAzureFileSystemContractPageBlobLive and TestNativeAzureFSPageBlobLive for examples of tests that configure the folder to enable atomic rename.

          Having said that, I'm not sure how much success we'll have writing a test to cover this. If you can't find a way to write a test, then please describe any additional manual testing that you did.

                }
                else if (!srcExists && dstExists) {
          

          The project coding standard is to place the braces inline, so please change to this:

                } else if (!srcExists && dstExists) {
          
          Show
          cnauroth Chris Nauroth added a comment - Gaurav Kanade , this mostly looks good to me. I just have a comment on testing and a nitpick. The test added in TestNativeAzureFileSystemLive does not exercise the code change. It does not execute against an atomic rename folder. See TestNativeAzureFileSystemContractPageBlobLive and TestNativeAzureFSPageBlobLive for examples of tests that configure the folder to enable atomic rename. Having said that, I'm not sure how much success we'll have writing a test to cover this. If you can't find a way to write a test, then please describe any additional manual testing that you did. } else if (!srcExists && dstExists) { The project coding standard is to place the braces inline, so please change to this: } else if (!srcExists && dstExists) {
          Hide
          gouravk Gaurav Kanade added a comment -

          Chris Nauroth - Thanks for the review.

          With regards to testing there are two aspects to consider here given the change we made to the behavior of lazy rename pending operation.
          1. Post the change the lazy rename pending will behave in the case of src and dest both existing the same way it will behave in the case of src only existing. The new test tries to somewhat cover this case - i.e. if we call a rename on a file already existing does rename happen correctly? Thus this part of the change effect is covered.

          2. The only other thing left to test is does the changed code path get executed in lazy rename pending - this as you see and as described is hard to capture in a single test given the various process crash and client side copy it requires to simulate. However the code change here is fairly straightforward; i.e. a simple change in condition; the fact that this condition change behaves appropriately is tested above.

          I am not clear yet as to have a manner of testing 2 cleanly; let me know your thoughts.

          I will also fix the braces issue with the next version of the patch

          Show
          gouravk Gaurav Kanade added a comment - Chris Nauroth - Thanks for the review. With regards to testing there are two aspects to consider here given the change we made to the behavior of lazy rename pending operation. 1. Post the change the lazy rename pending will behave in the case of src and dest both existing the same way it will behave in the case of src only existing. The new test tries to somewhat cover this case - i.e. if we call a rename on a file already existing does rename happen correctly? Thus this part of the change effect is covered. 2. The only other thing left to test is does the changed code path get executed in lazy rename pending - this as you see and as described is hard to capture in a single test given the various process crash and client side copy it requires to simulate. However the code change here is fairly straightforward; i.e. a simple change in condition; the fact that this condition change behaves appropriately is tested above. I am not clear yet as to have a manner of testing 2 cleanly; let me know your thoughts. I will also fix the braces issue with the next version of the patch
          Hide
          cnauroth Chris Nauroth added a comment -

          Gaurav Kanade, OK, let's keep the test you already wrote, with a few changes:

          1. Please change the name of the test to testLazyRenamePendingCanOverwriteExistingFile. I think this will help clarify that the intent is to test an internal implementation detail. The current name makes it sounds like we expect any FileSystem#rename operation to be able to overwrite the destination, which would not be true.
          2. Please ensure that any stream returned from a FileSystem#create call gets closed. try-with-resources or the IOUtils#cleanup utility method is helpful for this.
          3. The LOG statements are unnecessary. Please remove them.
          Show
          cnauroth Chris Nauroth added a comment - Gaurav Kanade , OK, let's keep the test you already wrote, with a few changes: Please change the name of the test to testLazyRenamePendingCanOverwriteExistingFile . I think this will help clarify that the intent is to test an internal implementation detail. The current name makes it sounds like we expect any FileSystem#rename operation to be able to overwrite the destination, which would not be true. Please ensure that any stream returned from a FileSystem#create call gets closed. try-with-resources or the IOUtils#cleanup utility method is helpful for this. The LOG statements are unnecessary. Please remove them.
          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 whitespace 0m 0s Patch has no whitespace issues.
          +1 asflicense 0m 26s Patch does not generate ASF License warnings.
          0m 44s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12780647/HADOOP-12634.02.patch
          JIRA Issue HADOOP-12634
          Optional Tests asflicense
          uname Linux da0855d711bf 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 / c52b407
          modules C: U:
          Max memory used 31MB
          Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8344/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 whitespace 0m 0s Patch has no whitespace issues. +1 asflicense 0m 26s Patch does not generate ASF License warnings. 0m 44s Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12780647/HADOOP-12634.02.patch JIRA Issue HADOOP-12634 Optional Tests asflicense uname Linux da0855d711bf 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 / c52b407 modules C: U: Max memory used 31MB Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8344/console This message was automatically generated.
          Hide
          gouravk Gaurav Kanade added a comment -

          Chris Nauroth Thanks. Please take a look at the latest patch.

          Show
          gouravk Gaurav Kanade added a comment - Chris Nauroth Thanks. Please take a look at the latest 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 1 new or modified test files.
          +1 mvninstall 7m 43s 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 8s trunk passed
          +1 mvnsite 0m 21s trunk passed
          +1 mvneclipse 0m 12s trunk passed
          +1 findbugs 0m 29s trunk passed
          +1 javadoc 0m 12s trunk passed with JDK v1.8.0_66
          +1 javadoc 0m 16s trunk passed with JDK v1.7.0_91
          +1 mvninstall 0m 15s the patch passed
          +1 compile 0m 10s the patch passed with JDK v1.8.0_66
          +1 javac 0m 10s 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 17s the patch passed
          +1 mvneclipse 0m 9s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 0m 33s the patch passed
          -1 javadoc 0m 57s hadoop-tools_hadoop-azure-jdk1.8.0_66 with JDK v1.8.0_66 generated 12 new issues (was 26, now 26).
          +1 javadoc 0m 10s the patch passed with JDK v1.8.0_66
          -1 javadoc 1m 24s hadoop-tools_hadoop-azure-jdk1.7.0_91 with JDK v1.7.0_91 generated 1 new issues (was 1, now 1).
          +1 javadoc 0m 12s the patch passed with JDK v1.7.0_91
          +1 unit 1m 1s hadoop-azure in the patch passed with JDK v1.8.0_66.
          +1 unit 1m 16s hadoop-azure in the patch passed with JDK v1.7.0_91.
          +1 asflicense 0m 17s Patch does not generate ASF License warnings.
          15m 34s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12780647/HADOOP-12634.02.patch
          JIRA Issue HADOOP-12634
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux c07d60acf11a 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 / c52b407
          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
          javadoc hadoop-tools_hadoop-azure-jdk1.8.0_66: https://builds.apache.org/job/PreCommit-HADOOP-Build/8345/artifact/patchprocess/diff-javadoc-javadoc-hadoop-tools_hadoop-azure-jdk1.8.0_66.txt
          javadoc hadoop-tools_hadoop-azure-jdk1.7.0_91: https://builds.apache.org/job/PreCommit-HADOOP-Build/8345/artifact/patchprocess/diff-javadoc-javadoc-hadoop-tools_hadoop-azure-jdk1.7.0_91.txt
          JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8345/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/8345/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 1 new or modified test files. +1 mvninstall 7m 43s 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 8s trunk passed +1 mvnsite 0m 21s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 0m 29s trunk passed +1 javadoc 0m 12s trunk passed with JDK v1.8.0_66 +1 javadoc 0m 16s trunk passed with JDK v1.7.0_91 +1 mvninstall 0m 15s the patch passed +1 compile 0m 10s the patch passed with JDK v1.8.0_66 +1 javac 0m 10s 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 17s the patch passed +1 mvneclipse 0m 9s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 0m 33s the patch passed -1 javadoc 0m 57s hadoop-tools_hadoop-azure-jdk1.8.0_66 with JDK v1.8.0_66 generated 12 new issues (was 26, now 26). +1 javadoc 0m 10s the patch passed with JDK v1.8.0_66 -1 javadoc 1m 24s hadoop-tools_hadoop-azure-jdk1.7.0_91 with JDK v1.7.0_91 generated 1 new issues (was 1, now 1). +1 javadoc 0m 12s the patch passed with JDK v1.7.0_91 +1 unit 1m 1s hadoop-azure in the patch passed with JDK v1.8.0_66. +1 unit 1m 16s hadoop-azure in the patch passed with JDK v1.7.0_91. +1 asflicense 0m 17s Patch does not generate ASF License warnings. 15m 34s Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12780647/HADOOP-12634.02.patch JIRA Issue HADOOP-12634 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux c07d60acf11a 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 / c52b407 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 javadoc hadoop-tools_hadoop-azure-jdk1.8.0_66: https://builds.apache.org/job/PreCommit-HADOOP-Build/8345/artifact/patchprocess/diff-javadoc-javadoc-hadoop-tools_hadoop-azure-jdk1.8.0_66.txt javadoc hadoop-tools_hadoop-azure-jdk1.7.0_91: https://builds.apache.org/job/PreCommit-HADOOP-Build/8345/artifact/patchprocess/diff-javadoc-javadoc-hadoop-tools_hadoop-azure-jdk1.7.0_91.txt JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8345/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/8345/console This message was automatically generated.
          Hide
          cnauroth Chris Nauroth added a comment -

          +1 for patch v02. The JavaDoc warnings from the last pre-commit run are pre-existing warnings unrelated to this patch. The patch shifted line numbers, which made them look like new warnings.

          I have committed this to trunk, branch-2 and branch-2.8. Gaurav Kanade, thank you for the patch.

          Show
          cnauroth Chris Nauroth added a comment - +1 for patch v02. The JavaDoc warnings from the last pre-commit run are pre-existing warnings unrelated to this patch. The patch shifted line numbers, which made them look like new warnings. I have committed this to trunk, branch-2 and branch-2.8. Gaurav Kanade , thank you for the patch.
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-trunk-Commit #9059 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9059/)
          HADOOP-12634. Change Lazy Rename Pending Operation Completion of WASB to (cnauroth: rev 978bbdfeb2d12efd6e750da6a14849e072fb814b)

          • 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/TestNativeAzureFileSystemLive.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #9059 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9059/ ) HADOOP-12634 . Change Lazy Rename Pending Operation Completion of WASB to (cnauroth: rev 978bbdfeb2d12efd6e750da6a14849e072fb814b) 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/TestNativeAzureFileSystemLive.java

            People

            • Assignee:
              gouravk Gaurav Kanade
              Reporter:
              gouravk Gaurav Kanade
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Due:
                Created:
                Updated:
                Resolved:

                Development