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

Replace method-local TransferManager object with S3AFileSystem#transfers

    Details

    • Type: Task
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.7.0
    • Fix Version/s: 2.7.0
    • Component/s: fs/s3
    • Labels:
      None
    • Target Version/s:

      Description

      This is continuation of HADOOP-11446.
      The following changes are made according to Thomas Demoor's comments:

      1. Replace method-local TransferManager object with S3AFileSystem#transfers
      2. Do not shutdown TransferManager after purging existing multipart file - otherwise the current transfer is unable to proceed
      3. Shutdown TransferManager instance in the close method of S3AFileSystem

      1. hadoop-11463-001.patch
        3 kB
        Ted Yu
      2. hadoop-11463-002.patch
        3 kB
        Ted Yu
      3. hadoop-11463-003.patch
        3 kB
        Ted Yu

        Issue Links

          Activity

          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12690381/hadoop-11463-001.patch
          against trunk revision 4cd66f7.

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

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

          +1 javadoc. There were no new javadoc warning messages.

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

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

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

          +1 core tests. The patch passed unit tests in hadoop-tools/hadoop-aws.

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/5368//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/5368//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12690381/hadoop-11463-001.patch against trunk revision 4cd66f7. +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-tools/hadoop-aws. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/5368//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/5368//console This message is automatically generated.
          Hide
          thodemoor Thomas Demoor added a comment -

          This addresses my remarks. I have run the tests vs AWS S3 and all passed. LGTM

          Show
          thodemoor Thomas Demoor added a comment - This addresses my remarks. I have run the tests vs AWS S3 and all passed. LGTM
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Looks good, but one question:

          Will the FS shutdown() operation block until transfers have finished? I've hit problems in HDFS where the client blocks due to connection retries, so the a client app blocks on a control-C-initiated shutdown —as that is when FileSystem tries to shutdown all its instances.

          Show
          stevel@apache.org Steve Loughran added a comment - Looks good, but one question: Will the FS shutdown() operation block until transfers have finished? I've hit problems in HDFS where the client blocks due to connection retries, so the a client app blocks on a control-C-initiated shutdown —as that is when FileSystem tries to shutdown all its instances.
          Hide
          yuzhihong@gmail.com Ted Yu added a comment -

          With HADOOP-10714, the upper bound on un-transferred objects is 1000.
          Meaning, the wait for closing S3AFileSystem would be short.

          Show
          yuzhihong@gmail.com Ted Yu added a comment - With HADOOP-10714 , the upper bound on un-transferred objects is 1000. Meaning, the wait for closing S3AFileSystem would be short.
          Hide
          yuzhihong@gmail.com Ted Yu added a comment -

          Without the patch, I observed the following exception in test (fs.s3a.multipart.purge being true):

          testRenameFileAsExistingFile(org.apache.hadoop.fs.s3a.TestS3AFileSystemContract)  Time elapsed: 1.944 sec  <<< ERROR!
          java.util.concurrent.RejectedExecutionException: Task java.util.concurrent.FutureTask@14b43b9c rejected from java.util.concurrent.ThreadPoolExecutor@44b58962[Terminated, pool size = 0, active threads = 0, queued tasks = 0, completed tasks = 0]
                  at java.util.concurrent.ThreadPoolExecutor$AbortPolicy.rejectedExecution(ThreadPoolExecutor.java:2048)
                  at java.util.concurrent.ThreadPoolExecutor.reject(ThreadPoolExecutor.java:821)
                  at java.util.concurrent.ThreadPoolExecutor.execute(ThreadPoolExecutor.java:1372)
                  at java.util.concurrent.AbstractExecutorService.submit(AbstractExecutorService.java:132)
                  at com.amazonaws.services.s3.transfer.internal.UploadMonitor.<init>(UploadMonitor.java:129)
                  at com.amazonaws.services.s3.transfer.TransferManager.upload(TransferManager.java:449)
                  at com.amazonaws.services.s3.transfer.TransferManager.upload(TransferManager.java:382)
                  at org.apache.hadoop.fs.s3a.S3AOutputStream.close(S3AOutputStream.java:125)
                  at org.apache.hadoop.fs.FSDataOutputStream$PositionCache.close(FSDataOutputStream.java:72)
                  at org.apache.hadoop.fs.FSDataOutputStream.close(FSDataOutputStream.java:101)
                  at org.apache.hadoop.fs.FileSystemContractBaseTest.createFile(FileSystemContractBaseTest.java:484)
                  at org.apache.hadoop.fs.s3a.TestS3AFileSystemContract.testRenameFileAsExistingFile(TestS3AFileSystemContract.java:68)
          

          The above is fixed by patch.

          Show
          yuzhihong@gmail.com Ted Yu added a comment - Without the patch, I observed the following exception in test (fs.s3a.multipart.purge being true): testRenameFileAsExistingFile(org.apache.hadoop.fs.s3a.TestS3AFileSystemContract) Time elapsed: 1.944 sec <<< ERROR! java.util.concurrent.RejectedExecutionException: Task java.util.concurrent.FutureTask@14b43b9c rejected from java.util.concurrent.ThreadPoolExecutor@44b58962[Terminated, pool size = 0, active threads = 0, queued tasks = 0, completed tasks = 0] at java.util.concurrent.ThreadPoolExecutor$AbortPolicy.rejectedExecution(ThreadPoolExecutor.java:2048) at java.util.concurrent.ThreadPoolExecutor.reject(ThreadPoolExecutor.java:821) at java.util.concurrent.ThreadPoolExecutor.execute(ThreadPoolExecutor.java:1372) at java.util.concurrent.AbstractExecutorService.submit(AbstractExecutorService.java:132) at com.amazonaws.services.s3.transfer.internal.UploadMonitor.<init>(UploadMonitor.java:129) at com.amazonaws.services.s3.transfer.TransferManager.upload(TransferManager.java:449) at com.amazonaws.services.s3.transfer.TransferManager.upload(TransferManager.java:382) at org.apache.hadoop.fs.s3a.S3AOutputStream.close(S3AOutputStream.java:125) at org.apache.hadoop.fs.FSDataOutputStream$PositionCache.close(FSDataOutputStream.java:72) at org.apache.hadoop.fs.FSDataOutputStream.close(FSDataOutputStream.java:101) at org.apache.hadoop.fs.FileSystemContractBaseTest.createFile(FileSystemContractBaseTest.java:484) at org.apache.hadoop.fs.s3a.TestS3AFileSystemContract.testRenameFileAsExistingFile(TestS3AFileSystemContract.java:68) The above is fixed by patch.
          Hide
          thodemoor Thomas Demoor added a comment -

          Steve Loughran shutDown terminates immediately. In-process transfers are abandoned. For regular uploads, S3 simply discards the object. However, for multi-part uploads, parts that were completely uploaded are stored (and paid for) forever and the multi-part upload is "in progress". The purging functionality alleviates this. If fs.s3a.multipart.purge == true, the constructor of S3AFileSystem aborts all in process multi-part uploads older than fs.s3a.multipart.purge.age seconds (this protects "active" multi-part uploads).

          Ted Yu Yeah, that was to be expected. See comment 2 in the description above.

          Show
          thodemoor Thomas Demoor added a comment - Steve Loughran shutDown terminates immediately. In-process transfers are abandoned. For regular uploads, S3 simply discards the object. However, for multi-part uploads, parts that were completely uploaded are stored (and paid for) forever and the multi-part upload is "in progress". The purging functionality alleviates this. If fs.s3a.multipart.purge == true, the constructor of S3AFileSystem aborts all in process multi-part uploads older than fs.s3a.multipart.purge.age seconds (this protects "active" multi-part uploads). Ted Yu Yeah, that was to be expected. See comment 2 in the description above.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          -1

          I want one minor change, which is the {close()}} operation must check for transfers != null before doing its work. There's some stuff done in the initialize() method before the field is assigned, so there's a risk that if something went wrong there, close() would fail.

          Once that's done I'll approve the patch

          Show
          stevel@apache.org Steve Loughran added a comment - -1 I want one minor change, which is the {close()}} operation must check for transfers != null before doing its work. There's some stuff done in the initialize() method before the field is assigned, so there's a risk that if something went wrong there, close() would fail. Once that's done I'll approve the patch
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12692782/hadoop-11463-002.patch
          against trunk revision 000ca83.

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

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

          +1 javadoc. There were no new javadoc warning messages.

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

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

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

          +1 core tests. The patch passed unit tests in hadoop-tools/hadoop-aws.

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/5417//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/5417//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12692782/hadoop-11463-002.patch against trunk revision 000ca83. +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-tools/hadoop-aws. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/5417//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/5417//console This message is automatically generated.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          I had a look at the other FileSystem classes -afraid you must also call super.close() for its cleanup work

          Show
          stevel@apache.org Steve Loughran added a comment - I had a look at the other FileSystem classes -afraid you must also call super.close() for its cleanup work
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12692927/hadoop-11463-003.patch
          against trunk revision 2908fe4.

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

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

          +1 javadoc. There were no new javadoc warning messages.

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

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

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

          +1 core tests. The patch passed unit tests in hadoop-tools/hadoop-aws.

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/5423//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/5423//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12692927/hadoop-11463-003.patch against trunk revision 2908fe4. +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-tools/hadoop-aws. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/5423//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/5423//console This message is automatically generated.
          Hide
          yuzhihong@gmail.com Ted Yu added a comment -

          Is there any review comment that I haven't addressed, Steve Loughran ?

          Show
          yuzhihong@gmail.com Ted Yu added a comment - Is there any review comment that I haven't addressed, Steve Loughran ?
          Hide
          stevel@apache.org Steve Loughran added a comment -

          s3 test suites against S3 EU all passed

          +1

          Show
          stevel@apache.org Steve Loughran added a comment - s3 test suites against S3 EU all passed +1
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-trunk-Commit #7019 (See https://builds.apache.org/job/Hadoop-trunk-Commit/7019/)
          HADOOP-11463 Replace method-local TransferManager object with S3AFileSystem#transfers. (Ted Yu via stevel) (stevel: rev 4e7ad4f0a88bd36bc91db6b1bd311d7f5c6bebee)

          • hadoop-common-project/hadoop-common/CHANGES.txt
          • hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #7019 (See https://builds.apache.org/job/Hadoop-trunk-Commit/7019/ ) HADOOP-11463 Replace method-local TransferManager object with S3AFileSystem#transfers. (Ted Yu via stevel) (stevel: rev 4e7ad4f0a88bd36bc91db6b1bd311d7f5c6bebee) hadoop-common-project/hadoop-common/CHANGES.txt hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #96 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/96/)
          HADOOP-11463 Replace method-local TransferManager object with S3AFileSystem#transfers. (Ted Yu via stevel) (stevel: rev 4e7ad4f0a88bd36bc91db6b1bd311d7f5c6bebee)

          • hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
          • hadoop-common-project/hadoop-common/CHANGES.txt
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #96 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/96/ ) HADOOP-11463 Replace method-local TransferManager object with S3AFileSystem#transfers. (Ted Yu via stevel) (stevel: rev 4e7ad4f0a88bd36bc91db6b1bd311d7f5c6bebee) hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java hadoop-common-project/hadoop-common/CHANGES.txt
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk #2046 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2046/)
          HADOOP-11463 Replace method-local TransferManager object with S3AFileSystem#transfers. (Ted Yu via stevel) (stevel: rev 4e7ad4f0a88bd36bc91db6b1bd311d7f5c6bebee)

          • hadoop-common-project/hadoop-common/CHANGES.txt
          • hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #2046 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2046/ ) HADOOP-11463 Replace method-local TransferManager object with S3AFileSystem#transfers. (Ted Yu via stevel) (stevel: rev 4e7ad4f0a88bd36bc91db6b1bd311d7f5c6bebee) hadoop-common-project/hadoop-common/CHANGES.txt hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #96 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/96/)
          HADOOP-11463 Replace method-local TransferManager object with S3AFileSystem#transfers. (Ted Yu via stevel) (stevel: rev 4e7ad4f0a88bd36bc91db6b1bd311d7f5c6bebee)

          • hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
          • hadoop-common-project/hadoop-common/CHANGES.txt
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #96 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/96/ ) HADOOP-11463 Replace method-local TransferManager object with S3AFileSystem#transfers. (Ted Yu via stevel) (stevel: rev 4e7ad4f0a88bd36bc91db6b1bd311d7f5c6bebee) hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java hadoop-common-project/hadoop-common/CHANGES.txt
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Yarn-trunk #830 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/830/)
          HADOOP-11463 Replace method-local TransferManager object with S3AFileSystem#transfers. (Ted Yu via stevel) (stevel: rev 4e7ad4f0a88bd36bc91db6b1bd311d7f5c6bebee)

          • hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
          • hadoop-common-project/hadoop-common/CHANGES.txt
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk #830 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/830/ ) HADOOP-11463 Replace method-local TransferManager object with S3AFileSystem#transfers. (Ted Yu via stevel) (stevel: rev 4e7ad4f0a88bd36bc91db6b1bd311d7f5c6bebee) hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java hadoop-common-project/hadoop-common/CHANGES.txt
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #93 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/93/)
          HADOOP-11463 Replace method-local TransferManager object with S3AFileSystem#transfers. (Ted Yu via stevel) (stevel: rev 4e7ad4f0a88bd36bc91db6b1bd311d7f5c6bebee)

          • hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
          • hadoop-common-project/hadoop-common/CHANGES.txt
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #93 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/93/ ) HADOOP-11463 Replace method-local TransferManager object with S3AFileSystem#transfers. (Ted Yu via stevel) (stevel: rev 4e7ad4f0a88bd36bc91db6b1bd311d7f5c6bebee) hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java hadoop-common-project/hadoop-common/CHANGES.txt
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Hdfs-trunk #2028 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2028/)
          HADOOP-11463 Replace method-local TransferManager object with S3AFileSystem#transfers. (Ted Yu via stevel) (stevel: rev 4e7ad4f0a88bd36bc91db6b1bd311d7f5c6bebee)

          • hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
          • hadoop-common-project/hadoop-common/CHANGES.txt
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Hdfs-trunk #2028 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2028/ ) HADOOP-11463 Replace method-local TransferManager object with S3AFileSystem#transfers. (Ted Yu via stevel) (stevel: rev 4e7ad4f0a88bd36bc91db6b1bd311d7f5c6bebee) hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java hadoop-common-project/hadoop-common/CHANGES.txt

            People

            • Assignee:
              yuzhihong@gmail.com Ted Yu
              Reporter:
              yuzhihong@gmail.com Ted Yu
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development