You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2021/02/01 09:40:51 UTC
[GitHub] [hadoop] HeartSaVioR opened a new pull request #2667: WIP. HADOOP-16906. Add Abortable.abort() interface for streams to enable output stream to be terminated
HeartSaVioR opened a new pull request #2667:
URL: https://github.com/apache/hadoop/pull/2667
NOTE: WIP. DO-NOT-MERGE.
No meaningful tests have been added, as I have no idea where I can add it, and how s3a has been tested
with integration test manner. (Tests in TestS3ABlockOutputStream only check simple things with mocking
everything, so can't do some write/upload test with it.)
I got some review comments from @steveloughran in my commit, and will reflect these review comments.
Once it's done I'll remove WIP.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] mukund-thakur commented on pull request #2667: HADOOP-16906. Add Abortable.abort() interface for streams to enable output stream to be terminated
Posted by GitBox <gi...@apache.org>.
mukund-thakur commented on pull request #2667:
URL: https://github.com/apache/hadoop/pull/2667#issuecomment-774044341
> I don't get the warning sign findbugs provided; `multipartUpload` field is already accessed via both synchronized/unsynchronized ways before this change, so probably something to ignore here?
>
> https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2667/1/artifact/out/new-findbugs-hadoop-tools_hadoop-aws.html
This is the reason.
The number of unsynchronized field accesses (reads and writes) was no more than one third of all accesses, with writes being weighed twice as high as reads
I think we can ignore this. What do you think @steveloughran ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #2667: WIP. HADOOP-16906. Add Abortable.abort() interface for streams to enable output stream to be terminated
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2667:
URL: https://github.com/apache/hadoop/pull/2667#issuecomment-771378333
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #2667: HADOOP-16906. Add Abortable.abort() interface for streams to enable output stream to be terminated
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2667:
URL: https://github.com/apache/hadoop/pull/2667#issuecomment-771517998
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 1m 13s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | | 0m 0s | [test4tests](test4tests) | The patch appears to include 3 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 14m 6s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 20m 7s | | trunk passed |
| +1 :green_heart: | compile | 20m 30s | | trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | compile | 17m 50s | | trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| +1 :green_heart: | checkstyle | 3m 44s | | trunk passed |
| +1 :green_heart: | mvnsite | 2m 28s | | trunk passed |
| +1 :green_heart: | shadedclient | 20m 9s | | branch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 1m 40s | | trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javadoc | 2m 18s | | trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| +0 :ok: | spotbugs | 1m 17s | | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 3m 32s | | trunk passed |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 25s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 27s | | the patch passed |
| +1 :green_heart: | compile | 19m 49s | | the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javac | 19m 49s | | the patch passed |
| +1 :green_heart: | compile | 17m 50s | | the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| +1 :green_heart: | javac | 17m 50s | | the patch passed |
| +1 :green_heart: | checkstyle | 3m 41s | | the patch passed |
| +1 :green_heart: | mvnsite | 2m 21s | | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | | The patch has no whitespace issues. |
| +1 :green_heart: | shadedclient | 13m 23s | | patch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 1m 39s | | the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javadoc | 2m 20s | | the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| -1 :x: | findbugs | 1m 24s | [/new-findbugs-hadoop-tools_hadoop-aws.html](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2667/4/artifact/out/new-findbugs-hadoop-tools_hadoop-aws.html) | hadoop-tools/hadoop-aws generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 17m 19s | | hadoop-common in the patch passed. |
| +1 :green_heart: | unit | 2m 7s | | hadoop-aws in the patch passed. |
| +1 :green_heart: | asflicense | 0m 56s | | The patch does not generate ASF License warnings. |
| | | 193m 59s | | |
| Reason | Tests |
|-------:|:------|
| FindBugs | module:hadoop-tools/hadoop-aws |
| | Inconsistent synchronization of org.apache.hadoop.fs.s3a.S3ABlockOutputStream.multiPartUpload; locked 50% of time Unsynchronized access at S3ABlockOutputStream.java:50% of time Unsynchronized access at S3ABlockOutputStream.java:[line 569] |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2667/4/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/2667 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux 3f942e07395b 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 1b893e10aed |
| Default Java | Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2667/4/testReport/ |
| Max. process+thread count | 1489 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2667/4/console |
| versions | git=2.25.1 maven=3.6.3 findbugs=4.0.6 |
| Powered by | Apache Yetus 0.13.0-SNAPSHOT https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #2667: WIP. HADOOP-16906. Add Abortable.abort() interface for streams to enable output stream to be terminated
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2667:
URL: https://github.com/apache/hadoop/pull/2667#issuecomment-771378333
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 1m 5s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | | 0m 0s | [test4tests](test4tests) | The patch appears to include 3 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 14m 5s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 20m 9s | | trunk passed |
| +1 :green_heart: | compile | 20m 28s | | trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | compile | 17m 50s | | trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| +1 :green_heart: | checkstyle | 3m 41s | | trunk passed |
| +1 :green_heart: | mvnsite | 2m 23s | | trunk passed |
| +1 :green_heart: | shadedclient | 19m 56s | | branch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 1m 41s | | trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javadoc | 2m 23s | | trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| +0 :ok: | spotbugs | 1m 14s | | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 3m 30s | | trunk passed |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 25s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 25s | | the patch passed |
| +1 :green_heart: | compile | 19m 47s | | the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javac | 19m 47s | | the patch passed |
| +1 :green_heart: | compile | 17m 49s | | the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| +1 :green_heart: | javac | 17m 49s | | the patch passed |
| -0 :warning: | checkstyle | 3m 44s | [/diff-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2667/3/artifact/out/diff-checkstyle-root.txt) | root: The patch generated 4 new + 2 unchanged - 0 fixed = 6 total (was 2) |
| +1 :green_heart: | mvnsite | 2m 26s | | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | | The patch has no whitespace issues. |
| +1 :green_heart: | shadedclient | 13m 25s | | patch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 1m 38s | | the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javadoc | 2m 17s | | the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| -1 :x: | findbugs | 1m 25s | [/new-findbugs-hadoop-tools_hadoop-aws.html](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2667/3/artifact/out/new-findbugs-hadoop-tools_hadoop-aws.html) | hadoop-tools/hadoop-aws generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 17m 14s | | hadoop-common in the patch passed. |
| +1 :green_heart: | unit | 2m 4s | | hadoop-aws in the patch passed. |
| +1 :green_heart: | asflicense | 0m 55s | | The patch does not generate ASF License warnings. |
| | | 193m 13s | | |
| Reason | Tests |
|-------:|:------|
| FindBugs | module:hadoop-tools/hadoop-aws |
| | Inconsistent synchronization of org.apache.hadoop.fs.s3a.S3ABlockOutputStream.multiPartUpload; locked 50% of time Unsynchronized access at S3ABlockOutputStream.java:50% of time Unsynchronized access at S3ABlockOutputStream.java:[line 569] |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2667/3/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/2667 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux 2cbffd8ea39d 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 21a3fc3d2d7 |
| Default Java | Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2667/3/testReport/ |
| Max. process+thread count | 3152 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2667/3/console |
| versions | git=2.25.1 maven=3.6.3 findbugs=4.0.6 |
| Powered by | Apache Yetus 0.13.0-SNAPSHOT https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] steveloughran commented on pull request #2667: HADOOP-16906. Add Abortable.abort() interface for streams to enable output stream to be terminated
Posted by GitBox <gi...@apache.org>.
steveloughran commented on pull request #2667:
URL: https://github.com/apache/hadoop/pull/2667#issuecomment-774251906
Added an extra patch to this, see the PR #2684 .
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] HeartSaVioR commented on pull request #2667: WIP. HADOOP-16906. Add Abortable.abort() interface for streams to enable output stream to be terminated
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on pull request #2667:
URL: https://github.com/apache/hadoop/pull/2667#issuecomment-770721288
I'll migrate @steveloughran 's review comments to the diff in PR. It doesn't seem to be done automatically - these comments have been shown as "normal review comments".
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #2667: WIP. HADOOP-16906. Add Abortable.abort() interface for streams to enable output stream to be terminated
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #2667:
URL: https://github.com/apache/hadoop/pull/2667#issuecomment-770846140
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 1m 17s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | | 0m 0s | [test4tests](test4tests) | The patch appears to include 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 13m 57s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 23m 29s | | trunk passed |
| +1 :green_heart: | compile | 24m 14s | | trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | compile | 20m 47s | | trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| +1 :green_heart: | checkstyle | 4m 2s | | trunk passed |
| +1 :green_heart: | mvnsite | 2m 16s | | trunk passed |
| +1 :green_heart: | shadedclient | 22m 24s | | branch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 1m 26s | | trunk passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javadoc | 2m 6s | | trunk passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| +0 :ok: | spotbugs | 1m 12s | | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 3m 28s | | trunk passed |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 21s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 27s | | the patch passed |
| +1 :green_heart: | compile | 21m 32s | | the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javac | 21m 32s | | the patch passed |
| +1 :green_heart: | compile | 19m 2s | | the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| +1 :green_heart: | javac | 19m 2s | | the patch passed |
| -0 :warning: | checkstyle | 3m 51s | [/diff-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2667/1/artifact/out/diff-checkstyle-root.txt) | root: The patch generated 4 new + 2 unchanged - 0 fixed = 6 total (was 2) |
| +1 :green_heart: | mvnsite | 2m 13s | | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | | The patch has no whitespace issues. |
| +1 :green_heart: | shadedclient | 15m 58s | | patch has no errors when building and testing our client artifacts. |
| +1 :green_heart: | javadoc | 1m 25s | | the patch passed with JDK Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 |
| +1 :green_heart: | javadoc | 2m 9s | | the patch passed with JDK Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| -1 :x: | findbugs | 1m 21s | [/new-findbugs-hadoop-tools_hadoop-aws.html](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2667/1/artifact/out/new-findbugs-hadoop-tools_hadoop-aws.html) | hadoop-tools/hadoop-aws generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 17m 18s | | hadoop-common in the patch passed. |
| +1 :green_heart: | unit | 1m 53s | | hadoop-aws in the patch passed. |
| +1 :green_heart: | asflicense | 0m 46s | | The patch does not generate ASF License warnings. |
| | | 209m 22s | | |
| Reason | Tests |
|-------:|:------|
| FindBugs | module:hadoop-tools/hadoop-aws |
| | Inconsistent synchronization of org.apache.hadoop.fs.s3a.S3ABlockOutputStream.multiPartUpload; locked 50% of time Unsynchronized access at S3ABlockOutputStream.java:50% of time Unsynchronized access at S3ABlockOutputStream.java:[line 569] |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2667/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/2667 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
| uname | Linux 45196ae1217f 4.15.0-128-generic #131-Ubuntu SMP Wed Dec 9 06:57:35 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 1a205cc3adf |
| Default Java | Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.9.1+1-Ubuntu-0ubuntu1.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_275-8u275-b01-0ubuntu1~20.04-b01 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2667/1/testReport/ |
| Max. process+thread count | 2568 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2667/1/console |
| versions | git=2.25.1 maven=3.6.3 findbugs=4.0.6 |
| Powered by | Apache Yetus 0.13.0-SNAPSHOT https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] HeartSaVioR commented on a change in pull request #2667: WIP. HADOOP-16906. Add Abortable.abort() interface for streams to enable output stream to be terminated
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on a change in pull request #2667:
URL: https://github.com/apache/hadoop/pull/2667#discussion_r567685195
##########
File path: hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3ABlockOutputStream.java
##########
@@ -551,6 +556,24 @@ public IOStatistics getIOStatistics() {
return iostatistics;
}
+ @Override
+ public void abort() {
Review comment:
Migrating comment in https://github.com/HeartSaVioR/hadoop/commit/63c5588d46d422a66ec620ae310f9a860db890b3#r46507150
> the new "cloud ready" API calls always return a CompletableFuture, to emphasise that the op may take time and to allow the caller to do something while waiting. Would we want to do this here? I'm not convinced it is appropriate. Instead we say
>
> 1. call must guarantee that after this is invoked,. close() will not materialize the file at its final path
> 2. it may communicate with the store to cancel an operation; which may retry. Errors will be stored.
> 3. there may still/also be async IO to the store after the call returns, but this must maintain the requirement "not visible"
> 4. And close() may do some IO to cancel
##########
File path: hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3ABlockOutputStream.java
##########
@@ -551,6 +556,24 @@ public IOStatistics getIOStatistics() {
return iostatistics;
}
+ @Override
+ public void abort() {
+ if (closed.getAndSet(true)) {
+ // already closed
+ LOG.debug("Ignoring abort() as stream is already closed");
+ return;
+ }
+
+ S3ADataBlocks.DataBlock block = getActiveBlock();
Review comment:
Migrating comment in https://github.com/HeartSaVioR/hadoop/commit/63c5588d46d422a66ec620ae310f9a860db890b3#r46506593
> I think we are going to have to worry about this a bit more, because we may have queued >1 block for upload in a separate thread. They'll maybe need interruption, or at least, when they finish, see if they should immediately cancel the upload. This won't make any difference in the semantics of abort() (the final upload has been killed), I just don't want to run up any bills.
##########
File path: hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3ABlockOutputStream.java
##########
@@ -551,6 +556,24 @@ public IOStatistics getIOStatistics() {
return iostatistics;
}
+ @Override
+ public void abort() {
+ if (closed.getAndSet(true)) {
+ // already closed
+ LOG.debug("Ignoring abort() as stream is already closed");
+ return;
+ }
+
+ S3ADataBlocks.DataBlock block = getActiveBlock();
+ try {
+ if (multiPartUpload != null) {
+ multiPartUpload.abort();
Review comment:
Migrating comment in https://github.com/HeartSaVioR/hadoop/commit/63c5588d46d422a66ec620ae310f9a860db890b3#r46506765
> ok, don't worry so much about my prev comment. That cancels all the outstanding futures.
##########
File path: hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3ABlockOutputStream.java
##########
@@ -82,4 +82,22 @@ public void testWriteOperationHelperPartLimits() throws Throwable {
() -> woh.newUploadPartRequest(key,
"uploadId", 50000, 1024, inputStream, null, 0L));
}
+
+ @Test
+ public void testStreamClosedAfterAbort() throws Exception {
+ stream.abort();
+
+ // This verification replaces testing various operations after calling abort:
+ // after calling abort, stream is closed like calling close().
+ intercept(IOException.class, () -> stream.checkOpen());
Review comment:
Migrating comment in https://github.com/HeartSaVioR/hadoop/commit/63c5588d46d422a66ec620ae310f9a860db890b3#r46507220
> should also verify that stream.write() raises an IOE. We could raise a subclass of IOE to indicate this was a checkOpen failure for a stricter test
##########
File path: hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3ABlockOutputStream.java
##########
@@ -82,4 +82,22 @@ public void testWriteOperationHelperPartLimits() throws Throwable {
() -> woh.newUploadPartRequest(key,
"uploadId", 50000, 1024, inputStream, null, 0L));
}
+
+ @Test
Review comment:
Migrating comment in https://github.com/HeartSaVioR/hadoop/commit/63c5588d46d422a66ec620ae310f9a860db890b3#r46506646
> tests are good. We will need to do an ITest too, which can be done in ITestS3ABlockOutputArray
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] HeartSaVioR commented on pull request #2667: WIP. HADOOP-16906. Add Abortable.abort() interface for streams to enable output stream to be terminated
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on pull request #2667:
URL: https://github.com/apache/hadoop/pull/2667#issuecomment-771308533
Addressed the basic UT, IT, scaled IT. Once the Yetus is happy with the change, I'll remove WIP and ping again for review.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] HeartSaVioR commented on pull request #2667: HADOOP-16906. Add Abortable.abort() interface for streams to enable output stream to be terminated
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on pull request #2667:
URL: https://github.com/apache/hadoop/pull/2667#issuecomment-771410586
cc. @steveloughran Could you please review this PR? Thanks in advance!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] HeartSaVioR commented on pull request #2667: WIP. HADOOP-16906. Add Abortable.abort() interface for streams to enable output stream to be terminated
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on pull request #2667:
URL: https://github.com/apache/hadoop/pull/2667#issuecomment-771310932
I don't get the warning sign findbugs provided; `multipartUpload` field is already accessed via both synchronized/unsynchronized ways before this change, so probably something to ignore here?
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-2667/1/artifact/out/new-findbugs-hadoop-tools_hadoop-aws.html
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] HeartSaVioR commented on pull request #2667: WIP. HADOOP-16906. Add Abortable.abort() interface for streams to enable output stream to be terminated
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on pull request #2667:
URL: https://github.com/apache/hadoop/pull/2667#issuecomment-771308533
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] HeartSaVioR commented on pull request #2667: HADOOP-16906. Add Abortable.abort() interface for streams to enable output stream to be terminated
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on pull request #2667:
URL: https://github.com/apache/hadoop/pull/2667#issuecomment-774388262
Thanks @steveloughran for helping! I see you've addressed lots of points especially amazing efforts on doc. Let me close this and jump on your PR. Thanks again!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] mehakmeet commented on a change in pull request #2667: HADOOP-16906. Add Abortable.abort() interface for streams to enable output stream to be terminated
Posted by GitBox <gi...@apache.org>.
mehakmeet commented on a change in pull request #2667:
URL: https://github.com/apache/hadoop/pull/2667#discussion_r570838580
##########
File path: hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/ITestS3AMultipartUploadSizeLimits.java
##########
@@ -20,6 +20,9 @@
import java.io.File;
+import org.apache.hadoop.fs.FSDataOutputStream;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.io.IOUtils;
Review comment:
reposition in ```org.apache.*``` block below.
##########
File path: hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/ITestS3AMultipartUploadSizeLimits.java
##########
@@ -118,4 +121,29 @@ public void testCommitLimitFailure() throws Throwable {
describedAs("commit abort count")
.isEqualTo(initial + 1);
}
+
+ @Test
+ public void testAbortAfterTwoPartUpload() throws Throwable {
Review comment:
javadoc or describe() to explain the test.
##########
File path: hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3ABlockOutputStream.java
##########
@@ -37,6 +37,7 @@
import com.amazonaws.services.s3.model.PutObjectRequest;
import com.amazonaws.services.s3.model.PutObjectResult;
import com.amazonaws.services.s3.model.UploadPartRequest;
+import org.apache.hadoop.fs.Abortable;
Review comment:
reposition in ```org.apache.*``` block below.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] HeartSaVioR closed pull request #2667: HADOOP-16906. Add Abortable.abort() interface for streams to enable output stream to be terminated
Posted by GitBox <gi...@apache.org>.
HeartSaVioR closed pull request #2667:
URL: https://github.com/apache/hadoop/pull/2667
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] HeartSaVioR commented on pull request #2667: WIP. HADOOP-16906. Add Abortable.abort() interface for streams to enable output stream to be terminated
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on pull request #2667:
URL: https://github.com/apache/hadoop/pull/2667#issuecomment-771410257
I'll remove WIP tag as I don't have any idea on findbugs failure and I guess I've addressed everything except "cloud-friendly" requirement on API. This doesn't sound something strictly bound to this PR, but please correct me if I'm missing here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] mukund-thakur commented on a change in pull request #2667: HADOOP-16906. Add Abortable.abort() interface for streams to enable output stream to be terminated
Posted by GitBox <gi...@apache.org>.
mukund-thakur commented on a change in pull request #2667:
URL: https://github.com/apache/hadoop/pull/2667#discussion_r570770903
##########
File path: hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3ABlockOutputStream.java
##########
@@ -541,6 +542,10 @@ public boolean hasCapability(String capability) {
case StreamCapabilities.IOSTATISTICS:
return true;
+ // S3A supports abort.
+ case StreamCapabilities.ABORTABLE:
Review comment:
We can merge both case statements
##########
File path: hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3ABlockOutputArray.java
##########
@@ -155,4 +156,22 @@ public void testMarkReset() throws Throwable {
markAndResetDatablock(createFactory(getFileSystem()));
}
+ @Test
+ public void testAbortAfterWrite() throws Throwable {
+ Path dest = path("testAbortAfterWrite");
Review comment:
use getMethodName()
##########
File path: hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3ABlockOutputStream.java
##########
@@ -551,6 +556,24 @@ public IOStatistics getIOStatistics() {
return iostatistics;
}
+ @Override
+ public void abort() {
+ if (closed.getAndSet(true)) {
+ // already closed
+ LOG.debug("Ignoring abort() as stream is already closed");
+ return;
+ }
+
+ S3ADataBlocks.DataBlock block = getActiveBlock();
+ try {
+ if (multiPartUpload != null) {
Review comment:
Got it,we are closing the stream before only.
##########
File path: hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3ABlockOutputStream.java
##########
@@ -551,6 +556,24 @@ public IOStatistics getIOStatistics() {
return iostatistics;
}
+ @Override
+ public void abort() {
+ if (closed.getAndSet(true)) {
+ // already closed
+ LOG.debug("Ignoring abort() as stream is already closed");
+ return;
+ }
+
+ S3ADataBlocks.DataBlock block = getActiveBlock();
+ try {
+ if (multiPartUpload != null) {
Review comment:
Wondering what happens in case of non multipart upload
##########
File path: hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3ABlockOutputArray.java
##########
@@ -155,4 +156,22 @@ public void testMarkReset() throws Throwable {
markAndResetDatablock(createFactory(getFileSystem()));
}
+ @Test
+ public void testAbortAfterWrite() throws Throwable {
+ Path dest = path("testAbortAfterWrite");
+ describe(" testAbortAfterWrite");
Review comment:
nit: A bit more explanatory.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org