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

Correctly revoke file leases when closing files

    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: hdfs-client
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      As HADOOP-13264 memtioned, the code dfsClient.endFileLease(fileId) in DFSOutputStream will not be executed when the IOException happened in closeImpl().

        public void close() throws IOException {
          synchronized (this) {
            try (TraceScope ignored =
                dfsClient.newPathTraceScope("DFSOutputStream#close", src)) {
              closeImpl();
            }
          }
          dfsClient.endFileLease(fileId);
          }
        }
      

      This will cause that the files not be closed in DFSClient and finally lead to the memory leak. In DFSStripedOutputStream, it existed the same problem.

      1. HDFS-10549.001.patch
        3 kB
        Yiqun Lin
      2. HDFS-10549.002.patch
        0.9 kB
        Yiqun Lin
      3. HDFS-10549.003.patch
        3 kB
        Yiqun Lin
      4. HDFS-10549.004.patch
        6 kB
        Yiqun Lin
      5. HDFS-10549.005.patch
        7 kB
        Yiqun Lin

        Issue Links

          Activity

          Hide
          linyiqun Yiqun Lin added a comment -

          Attach a initial patch.

          Show
          linyiqun Yiqun Lin added a comment - Attach a initial patch.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 25s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
          +1 mvninstall 8m 5s trunk passed
          +1 compile 0m 38s trunk passed
          +1 checkstyle 0m 17s trunk passed
          +1 mvnsite 0m 42s trunk passed
          +1 mvneclipse 0m 14s trunk passed
          +1 findbugs 1m 45s trunk passed
          +1 javadoc 0m 25s trunk passed
          +1 mvninstall 0m 40s the patch passed
          +1 compile 0m 37s the patch passed
          +1 javac 0m 37s the patch passed
          -1 checkstyle 0m 14s hadoop-hdfs-project/hadoop-hdfs-client: The patch generated 1 new + 32 unchanged - 0 fixed = 33 total (was 32)
          +1 mvnsite 0m 40s the patch passed
          +1 mvneclipse 0m 11s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 44s the patch passed
          +1 javadoc 0m 17s the patch passed
          +1 unit 0m 56s hadoop-hdfs-client in the patch passed.
          +1 asflicense 0m 16s The patch does not generate ASF License warnings.
          19m 26s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:e2f6409
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12811742/HDFS-10549.001.patch
          JIRA Issue HDFS-10549
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 21bf0eff356c 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 / d0162f2
          Default Java 1.8.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/15829/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs-client.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/15829/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/15829/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 25s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 mvninstall 8m 5s trunk passed +1 compile 0m 38s trunk passed +1 checkstyle 0m 17s trunk passed +1 mvnsite 0m 42s trunk passed +1 mvneclipse 0m 14s trunk passed +1 findbugs 1m 45s trunk passed +1 javadoc 0m 25s trunk passed +1 mvninstall 0m 40s the patch passed +1 compile 0m 37s the patch passed +1 javac 0m 37s the patch passed -1 checkstyle 0m 14s hadoop-hdfs-project/hadoop-hdfs-client: The patch generated 1 new + 32 unchanged - 0 fixed = 33 total (was 32) +1 mvnsite 0m 40s the patch passed +1 mvneclipse 0m 11s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 44s the patch passed +1 javadoc 0m 17s the patch passed +1 unit 0m 56s hadoop-hdfs-client in the patch passed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 19m 26s Subsystem Report/Notes Docker Image:yetus/hadoop:e2f6409 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12811742/HDFS-10549.001.patch JIRA Issue HDFS-10549 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 21bf0eff356c 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 / d0162f2 Default Java 1.8.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/15829/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs-client.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/15829/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client Console output https://builds.apache.org/job/PreCommit-HDFS-Build/15829/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          linyiqun Yiqun Lin added a comment -

          I debug the test for TestCrcCorruption.testCorruptionDuringWrt in HDFS-6532, I found some IOExceptions after InterruptedException that threw in #DataStreamer#waitForAckedSeqno:

          2016-08-09 18:08:41,466 [Async disk worker #0 for volume /Users/lyq/Documents/work-project/hadoop-trunk/hadoop/hadoop-hdfs-project/hadoop-hdfs/target/test/data/dfs/data/data11/current] INFO  impl.FsDatasetAsyncDiskService (FsDatasetAsyncDiskService.java:run(296)) - Deleted BP-1814578309-127.0.0.1-1470737305923 blk_1073741825_1001 file /Users/lyq/Documents/work-project/hadoop-trunk/hadoop/hadoop-hdfs-project/hadoop-hdfs/target/test/data/dfs/data/data11/current/BP-1814578309-127.0.0.1-1470737305923/current/rbw/blk_1073741825
          2016-08-09 18:08:43,540 [Thread-0] ERROR hdfs.DFSClient (DFSClient.java:closeAllFilesBeingWritten(579)) - Failed to close file: /test_corruption_file with inode: 16387
          java.nio.channels.ClosedByInterruptException
          	at java.nio.channels.spi.AbstractInterruptibleChannel.end(AbstractInterruptibleChannel.java:202)
          	at sun.nio.ch.SocketChannelImpl.write(SocketChannelImpl.java:478)
          	at org.apache.hadoop.net.SocketOutputStream$Writer.performIO(SocketOutputStream.java:63)
          	at org.apache.hadoop.net.SocketIOWithTimeout.doIO(SocketIOWithTimeout.java:142)
          	at org.apache.hadoop.net.SocketOutputStream.write(SocketOutputStream.java:159)
          	at org.apache.hadoop.net.SocketOutputStream.write(SocketOutputStream.java:117)
          	at java.io.BufferedOutputStream.write(BufferedOutputStream.java:122)
          	at java.io.DataOutputStream.write(DataOutputStream.java:107)
          	at org.apache.hadoop.hdfs.DFSPacket.writeTo(DFSPacket.java:194)
          	at org.apache.hadoop.hdfs.DataStreamer.run(DataStreamer.java:660)
          2016-08-09 18:08:52,210 [Thread-0] ERROR hdfs.DFSClient (DFSClient.java:closeAllFilesBeingWritten(588)) - 
          

          It seems that this case was related to this jira.

          As the error logs shows that the files are not completely closed, then it will cause memory leak. We should close files again when the exception happens. Post a new patch for this to simplified v001 patch. This is a high priority issue, can take a look for this, Xiao Chen and Andrew Wang.

          Thanks.

          Show
          linyiqun Yiqun Lin added a comment - I debug the test for TestCrcCorruption.testCorruptionDuringWrt in HDFS-6532 , I found some IOExceptions after InterruptedException that threw in #DataStreamer#waitForAckedSeqno : 2016-08-09 18:08:41,466 [Async disk worker #0 for volume /Users/lyq/Documents/work-project/hadoop-trunk/hadoop/hadoop-hdfs-project/hadoop-hdfs/target/test/data/dfs/data/data11/current] INFO impl.FsDatasetAsyncDiskService (FsDatasetAsyncDiskService.java:run(296)) - Deleted BP-1814578309-127.0.0.1-1470737305923 blk_1073741825_1001 file /Users/lyq/Documents/work-project/hadoop-trunk/hadoop/hadoop-hdfs-project/hadoop-hdfs/target/test/data/dfs/data/data11/current/BP-1814578309-127.0.0.1-1470737305923/current/rbw/blk_1073741825 2016-08-09 18:08:43,540 [ Thread -0] ERROR hdfs.DFSClient (DFSClient.java:closeAllFilesBeingWritten(579)) - Failed to close file: /test_corruption_file with inode: 16387 java.nio.channels.ClosedByInterruptException at java.nio.channels.spi.AbstractInterruptibleChannel.end(AbstractInterruptibleChannel.java:202) at sun.nio.ch.SocketChannelImpl.write(SocketChannelImpl.java:478) at org.apache.hadoop.net.SocketOutputStream$Writer.performIO(SocketOutputStream.java:63) at org.apache.hadoop.net.SocketIOWithTimeout.doIO(SocketIOWithTimeout.java:142) at org.apache.hadoop.net.SocketOutputStream.write(SocketOutputStream.java:159) at org.apache.hadoop.net.SocketOutputStream.write(SocketOutputStream.java:117) at java.io.BufferedOutputStream.write(BufferedOutputStream.java:122) at java.io.DataOutputStream.write(DataOutputStream.java:107) at org.apache.hadoop.hdfs.DFSPacket.writeTo(DFSPacket.java:194) at org.apache.hadoop.hdfs.DataStreamer.run(DataStreamer.java:660) 2016-08-09 18:08:52,210 [ Thread -0] ERROR hdfs.DFSClient (DFSClient.java:closeAllFilesBeingWritten(588)) - It seems that this case was related to this jira. As the error logs shows that the files are not completely closed, then it will cause memory leak. We should close files again when the exception happens. Post a new patch for this to simplified v001 patch. This is a high priority issue, can take a look for this, Xiao Chen and Andrew Wang . Thanks.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 11s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
          +1 mvninstall 6m 36s trunk passed
          +1 compile 0m 30s trunk passed
          +1 checkstyle 0m 15s trunk passed
          +1 mvnsite 0m 32s trunk passed
          +1 mvneclipse 0m 12s trunk passed
          +1 findbugs 1m 22s trunk passed
          +1 javadoc 0m 20s trunk passed
          +1 mvninstall 0m 30s the patch passed
          +1 compile 0m 27s the patch passed
          +1 javac 0m 27s the patch passed
          +1 checkstyle 0m 12s the patch passed
          +1 mvnsite 0m 30s the patch passed
          +1 mvneclipse 0m 9s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 29s the patch passed
          +1 javadoc 0m 17s the patch passed
          +1 unit 0m 52s hadoop-hdfs-client in the patch passed.
          +1 asflicense 0m 16s The patch does not generate ASF License warnings.
          15m 49s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12822775/HDFS-10549.002.patch
          JIRA Issue HDFS-10549
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux ae4dbdbd8849 3.13.0-92-generic #139-Ubuntu SMP Tue Jun 28 20:42:26 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 522ddbd
          Default Java 1.8.0_101
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16358/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16358/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 11s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 mvninstall 6m 36s trunk passed +1 compile 0m 30s trunk passed +1 checkstyle 0m 15s trunk passed +1 mvnsite 0m 32s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 1m 22s trunk passed +1 javadoc 0m 20s trunk passed +1 mvninstall 0m 30s the patch passed +1 compile 0m 27s the patch passed +1 javac 0m 27s the patch passed +1 checkstyle 0m 12s the patch passed +1 mvnsite 0m 30s the patch passed +1 mvneclipse 0m 9s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 29s the patch passed +1 javadoc 0m 17s the patch passed +1 unit 0m 52s hadoop-hdfs-client in the patch passed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 15m 49s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12822775/HDFS-10549.002.patch JIRA Issue HDFS-10549 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux ae4dbdbd8849 3.13.0-92-generic #139-Ubuntu SMP Tue Jun 28 20:42:26 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 522ddbd Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16358/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client U: hadoop-hdfs-project/hadoop-hdfs-client Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16358/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          xiaochen Xiao Chen added a comment -

          Thanks Yiqun Lin for working on this.
          Looking at your comment on HADOOP-13264, should we mark that jira as a dup, and address all the places in this jira? Would be great to have unit tests too.

          Show
          xiaochen Xiao Chen added a comment - Thanks Yiqun Lin for working on this. Looking at your comment on HADOOP-13264 , should we mark that jira as a dup, and address all the places in this jira? Would be great to have unit tests too.
          Hide
          linyiqun Yiqun Lin added a comment - - edited

          Thanks Xiao Chen for the comments.

          should we mark that jira as a dup, and address all the places in this jira?

          Agree with your idea. Can you help mark that jira? It seems that I am not allowed to do that in HADOOP-COMMON jiras.

          Post the new patch for adding the unit test. The test has passed in my local env and the test will failed if we keep the origin logic in DFSClient#closeAllFilesBeingWritten. Thanks for the review.

          Show
          linyiqun Yiqun Lin added a comment - - edited Thanks Xiao Chen for the comments. should we mark that jira as a dup, and address all the places in this jira? Agree with your idea. Can you help mark that jira? It seems that I am not allowed to do that in HADOOP-COMMON jiras. Post the new patch for adding the unit test. The test has passed in my local env and the test will failed if we keep the origin logic in DFSClient#closeAllFilesBeingWritten . Thanks for the review.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s 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.
          0 mvndep 0m 29s Maven dependency ordering for branch
          +1 mvninstall 8m 26s trunk passed
          +1 compile 1m 25s trunk passed
          +1 checkstyle 0m 31s trunk passed
          +1 mvnsite 1m 34s trunk passed
          +1 mvneclipse 0m 26s trunk passed
          +1 findbugs 3m 40s trunk passed
          +1 javadoc 1m 29s trunk passed
          0 mvndep 0m 8s Maven dependency ordering for patch
          +1 mvninstall 1m 48s the patch passed
          +1 compile 1m 52s the patch passed
          +1 javac 1m 52s the patch passed
          +1 checkstyle 0m 37s the patch passed
          +1 mvnsite 1m 45s the patch passed
          +1 mvneclipse 0m 24s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 3m 56s the patch passed
          +1 javadoc 1m 27s the patch passed
          +1 unit 1m 5s hadoop-hdfs-client in the patch passed.
          -1 unit 86m 28s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 19s The patch does not generate ASF License warnings.
          119m 49s



          Reason Tests
          Failed junit tests hadoop.hdfs.server.blockmanagement.TestPendingInvalidateBlock
            hadoop.hdfs.server.datanode.TestLargeBlockReport
            hadoop.hdfs.server.datanode.TestDataNodeLifeline
            hadoop.hdfs.server.datanode.TestDataNodeVolumeFailure
            hadoop.hdfs.TestEncryptionZones
            hadoop.hdfs.server.datanode.TestDirectoryScanner



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12823181/HDFS-10549.003.patch
          JIRA Issue HDFS-10549
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 33c26b937873 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 / 666ad0b
          Default Java 1.8.0_101
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/16401/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16401/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16401/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s 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. 0 mvndep 0m 29s Maven dependency ordering for branch +1 mvninstall 8m 26s trunk passed +1 compile 1m 25s trunk passed +1 checkstyle 0m 31s trunk passed +1 mvnsite 1m 34s trunk passed +1 mvneclipse 0m 26s trunk passed +1 findbugs 3m 40s trunk passed +1 javadoc 1m 29s trunk passed 0 mvndep 0m 8s Maven dependency ordering for patch +1 mvninstall 1m 48s the patch passed +1 compile 1m 52s the patch passed +1 javac 1m 52s the patch passed +1 checkstyle 0m 37s the patch passed +1 mvnsite 1m 45s the patch passed +1 mvneclipse 0m 24s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 56s the patch passed +1 javadoc 1m 27s the patch passed +1 unit 1m 5s hadoop-hdfs-client in the patch passed. -1 unit 86m 28s hadoop-hdfs in the patch failed. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 119m 49s Reason Tests Failed junit tests hadoop.hdfs.server.blockmanagement.TestPendingInvalidateBlock   hadoop.hdfs.server.datanode.TestLargeBlockReport   hadoop.hdfs.server.datanode.TestDataNodeLifeline   hadoop.hdfs.server.datanode.TestDataNodeVolumeFailure   hadoop.hdfs.TestEncryptionZones   hadoop.hdfs.server.datanode.TestDirectoryScanner Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12823181/HDFS-10549.003.patch JIRA Issue HDFS-10549 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 33c26b937873 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 / 666ad0b Default Java 1.8.0_101 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HDFS-Build/16401/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16401/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16401/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          xiaochen Xiao Chen added a comment -

          Can you help mark that jira? It seems that I am not allowed to do that in HADOOP-COMMON jiras.

          Done. Seems like you're not listed as contributor on common, only hdfs. I can't add you though (it reports server issue when I tried), hope someone on watch could help. Thanks!

          Show
          xiaochen Xiao Chen added a comment - Can you help mark that jira? It seems that I am not allowed to do that in HADOOP-COMMON jiras. Done. Seems like you're not listed as contributor on common, only hdfs. I can't add you though (it reports server issue when I tried), hope someone on watch could help. Thanks!
          Hide
          xiaochen Xiao Chen added a comment -

          Thanks Yiqun Lin for the patch, idea looks good.

          I noticed the endFileLease is done in DFSClient#closeAllFilesBeingWritten instead of close and abort, any specific reasons? I think keeping the logic inside is safer, since the problem is when an exception is thrown within close/abort, the line endFileLease isn't executed. I hope we can just make sure it's run in any cases (basically your proposal in HADOOP-13264, but keep the original thrown exception and rethrow that after we call endFileLease).

          Also, seems DFSClient#endFileLease does not actually throw any IOExceptions. That could simplify things a little here too.

          Nit:
          In the test, can we rephrase this comment?

                // Construct new dfsClient to get same LeaseRenewer instance to avoid
                // that the origin dfsClient added in leaseRenewer again.
          

          Maybe to: Construct a new dfsClient to get the same LeaseRenewer instance, to avoid the original client being added to the leaseRenewer again.

          Show
          xiaochen Xiao Chen added a comment - Thanks Yiqun Lin for the patch, idea looks good. I noticed the endFileLease is done in DFSClient#closeAllFilesBeingWritten instead of close and abort , any specific reasons? I think keeping the logic inside is safer, since the problem is when an exception is thrown within close / abort , the line endFileLease isn't executed. I hope we can just make sure it's run in any cases (basically your proposal in HADOOP-13264 , but keep the original thrown exception and rethrow that after we call endFileLease ). Also, seems DFSClient#endFileLease does not actually throw any IOExceptions. That could simplify things a little here too. Nit: In the test, can we rephrase this comment? // Construct new dfsClient to get same LeaseRenewer instance to avoid // that the origin dfsClient added in leaseRenewer again. Maybe to: Construct a new dfsClient to get the same LeaseRenewer instance, to avoid the original client being added to the leaseRenewer again.
          Hide
          linyiqun Yiqun Lin added a comment -

          Thanks a lot for the review, Xiao Chen.

          I hope we can just make sure it's run in any cases.

          Agree with this. The logic in v003 patch seems not good.

          Also, seems DFSClient#endFileLease does not actually throw any IOExceptions.

          Yes, it will be better to throw IOExceptions for DFSClient#endFileLease as well.

          Post a new patch for addressing the comments.

          Show
          linyiqun Yiqun Lin added a comment - Thanks a lot for the review, Xiao Chen . I hope we can just make sure it's run in any cases. Agree with this. The logic in v003 patch seems not good. Also, seems DFSClient#endFileLease does not actually throw any IOExceptions. Yes, it will be better to throw IOExceptions for DFSClient#endFileLease as well. Post a new patch for addressing the comments.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 19s 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.
          0 mvndep 0m 7s Maven dependency ordering for branch
          +1 mvninstall 8m 26s trunk passed
          +1 compile 1m 55s trunk passed
          +1 checkstyle 0m 35s trunk passed
          +1 mvnsite 1m 52s trunk passed
          +1 mvneclipse 0m 27s trunk passed
          +1 findbugs 3m 49s trunk passed
          +1 javadoc 1m 17s trunk passed
          0 mvndep 0m 6s Maven dependency ordering for patch
          +1 mvninstall 1m 24s the patch passed
          +1 compile 1m 31s the patch passed
          +1 javac 1m 31s the patch passed
          +1 checkstyle 0m 30s the patch passed
          +1 mvnsite 1m 28s the patch passed
          +1 mvneclipse 0m 21s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 3m 23s the patch passed
          +1 javadoc 1m 15s the patch passed
          +1 unit 0m 54s hadoop-hdfs-client in the patch passed.
          +1 unit 71m 32s hadoop-hdfs in the patch passed.
          +1 asflicense 0m 19s The patch does not generate ASF License warnings.
          102m 52s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12823665/HDFS-10549.004.patch
          JIRA Issue HDFS-10549
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 4aa99074c3c5 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 / d677b68
          Default Java 1.8.0_101
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16421/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16421/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 19s 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. 0 mvndep 0m 7s Maven dependency ordering for branch +1 mvninstall 8m 26s trunk passed +1 compile 1m 55s trunk passed +1 checkstyle 0m 35s trunk passed +1 mvnsite 1m 52s trunk passed +1 mvneclipse 0m 27s trunk passed +1 findbugs 3m 49s trunk passed +1 javadoc 1m 17s trunk passed 0 mvndep 0m 6s Maven dependency ordering for patch +1 mvninstall 1m 24s the patch passed +1 compile 1m 31s the patch passed +1 javac 1m 31s the patch passed +1 checkstyle 0m 30s the patch passed +1 mvnsite 1m 28s the patch passed +1 mvneclipse 0m 21s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 23s the patch passed +1 javadoc 1m 15s the patch passed +1 unit 0m 54s hadoop-hdfs-client in the patch passed. +1 unit 71m 32s hadoop-hdfs in the patch passed. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 102m 52s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12823665/HDFS-10549.004.patch JIRA Issue HDFS-10549 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 4aa99074c3c5 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 / d677b68 Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16421/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16421/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          xiaochen Xiao Chen added a comment -

          Yes, it will be better to throw IOExceptions for DFSClient#endFileLease as well.

          I meant to say, although DFSClient#endFileLease is declared to throw IOExceptions, it's inner code doesn't really throw anything, so this method won't throw IOE. I think we can remove the throw IOException from the method signature and simplify the logic here, right?

          Show
          xiaochen Xiao Chen added a comment - Yes, it will be better to throw IOExceptions for DFSClient#endFileLease as well. I meant to say, although DFSClient#endFileLease is declared to throw IOExceptions, it's inner code doesn't really throw anything, so this method won't throw IOE. I think we can remove the throw IOException from the method signature and simplify the logic here, right?
          Hide
          linyiqun Yiqun Lin added a comment - - edited

          Sorry, Xiao, I'm misread for that.

          I think we can remove the throw IOException from the method signature and simplify the logic here, right?

          Yes, that's right. Attach a new patch to make a quick change for this.

          One point that I want to say: In my latest patch, I keep to use the object MultipleIOException. If there is only on IOExeption, it will just throw that one. And we can still use MultipleIOException to add IOException in the future.

          Hope my latest patch will satisfied with you.

          Show
          linyiqun Yiqun Lin added a comment - - edited Sorry, Xiao, I'm misread for that. I think we can remove the throw IOException from the method signature and simplify the logic here, right? Yes, that's right. Attach a new patch to make a quick change for this. One point that I want to say: In my latest patch, I keep to use the object MultipleIOException . If there is only on IOExeption, it will just throw that one. And we can still use MultipleIOException to add IOException in the future. Hope my latest patch will satisfied with you.
          Hide
          xiaochen Xiao Chen added a comment -

          Thanks Yiqun for revving! Patch 5 looks good to me. +1 pending jenkins.
          Will hold until Wednesday morning in case others have comments.

          Show
          xiaochen Xiao Chen added a comment - Thanks Yiqun for revving! Patch 5 looks good to me. +1 pending jenkins. Will hold until Wednesday morning in case others have comments.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s 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.
          0 mvndep 0m 30s Maven dependency ordering for branch
          +1 mvninstall 7m 51s trunk passed
          +1 compile 1m 55s trunk passed
          +1 checkstyle 0m 36s trunk passed
          +1 mvnsite 1m 53s trunk passed
          +1 mvneclipse 0m 28s trunk passed
          +1 findbugs 3m 19s trunk passed
          +1 javadoc 1m 26s trunk passed
          0 mvndep 0m 8s Maven dependency ordering for patch
          +1 mvninstall 1m 44s the patch passed
          +1 compile 1m 52s the patch passed
          +1 javac 1m 52s the patch passed
          +1 checkstyle 0m 33s the patch passed
          +1 mvnsite 1m 51s the patch passed
          +1 mvneclipse 0m 23s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 3m 32s the patch passed
          +1 javadoc 1m 17s the patch passed
          +1 unit 0m 57s hadoop-hdfs-client in the patch passed.
          -1 unit 78m 13s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 26s The patch does not generate ASF License warnings.
          110m 45s



          Reason Tests
          Failed junit tests hadoop.hdfs.server.namenode.TestINodeFile



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12823792/HDFS-10549.005.patch
          JIRA Issue HDFS-10549
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux f9d9047af496 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 / 864f878
          Default Java 1.8.0_101
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/16430/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16430/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16430/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s 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. 0 mvndep 0m 30s Maven dependency ordering for branch +1 mvninstall 7m 51s trunk passed +1 compile 1m 55s trunk passed +1 checkstyle 0m 36s trunk passed +1 mvnsite 1m 53s trunk passed +1 mvneclipse 0m 28s trunk passed +1 findbugs 3m 19s trunk passed +1 javadoc 1m 26s trunk passed 0 mvndep 0m 8s Maven dependency ordering for patch +1 mvninstall 1m 44s the patch passed +1 compile 1m 52s the patch passed +1 javac 1m 52s the patch passed +1 checkstyle 0m 33s the patch passed +1 mvnsite 1m 51s the patch passed +1 mvneclipse 0m 23s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 32s the patch passed +1 javadoc 1m 17s the patch passed +1 unit 0m 57s hadoop-hdfs-client in the patch passed. -1 unit 78m 13s hadoop-hdfs in the patch failed. +1 asflicense 0m 26s The patch does not generate ASF License warnings. 110m 45s Reason Tests Failed junit tests hadoop.hdfs.server.namenode.TestINodeFile Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12823792/HDFS-10549.005.patch JIRA Issue HDFS-10549 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux f9d9047af496 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 / 864f878 Default Java 1.8.0_101 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HDFS-Build/16430/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16430/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16430/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          xiaochen Xiao Chen added a comment -

          Test failure look unrelated an passed locally.
          +1, committing this shortly.

          Show
          xiaochen Xiao Chen added a comment - Test failure look unrelated an passed locally. +1, committing this shortly.
          Hide
          xiaochen Xiao Chen added a comment -

          Thanks Yiqun Lin for fixing the issue, Seb Mo for reporting the original HADOOP-13264, and Arpit Agarwal and Kihwal Lee for the help!

          I have committed this to trunk, branch-2 and branch-2.8. (Trivial conflict since DFSStripedOutputStream is 3.0 only, compiled and ran TestDistributedFileSystem before pushing)

          Show
          xiaochen Xiao Chen added a comment - Thanks Yiqun Lin for fixing the issue, Seb Mo for reporting the original HADOOP-13264 , and Arpit Agarwal and Kihwal Lee for the help! I have committed this to trunk, branch-2 and branch-2.8. (Trivial conflict since DFSStripedOutputStream is 3.0 only, compiled and ran TestDistributedFileSystem before pushing)
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10295 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10295/)
          HDFS-10549. Correctly revoke file leases when closing files. Contributed (xiao: rev 2aa5e2c40364cf1e90e6af7851801f5eda759002)

          • (edit) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSStripedOutputStream.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDistributedFileSystem.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSClient.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10295 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10295/ ) HDFS-10549 . Correctly revoke file leases when closing files. Contributed (xiao: rev 2aa5e2c40364cf1e90e6af7851801f5eda759002) (edit) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSStripedOutputStream.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDistributedFileSystem.java (edit) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSOutputStream.java (edit) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSClient.java
          Hide
          linyiqun Yiqun Lin added a comment -

          Thanks a lot for the commit, Xiao!

          Show
          linyiqun Yiqun Lin added a comment - Thanks a lot for the commit, Xiao!

            People

            • Assignee:
              linyiqun Yiqun Lin
              Reporter:
              linyiqun Yiqun Lin
            • Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development