HADOOP-16906. Abortable #2684
Merged
HADOOP-16906. Abortable #2684
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
steveloughran
force-pushed
the
incoming/limj/HADOOP-16906-abort-trunk
branch
2 times, most recently
from
c80e45b
to
6daa199
Compare
|
test run against s3 london; known failures... buffer underfill and a recent s3 change breaking ITestAssumeRole. Both unrelated |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
The additional commit looks pretty great. Probably need to fix checkstyle/whitespace complains if they're not false alarms. |
There was a problem hiding this comment.
Changes looks really good now. Thanks @steveloughran
Ready to go in once Yetus gives +1.
| Strictly then: | ||
|
|
||
| > if `Abortable.abort()` does not raise `UnsupportedOperationException` | ||
| > then returns, then it guarantees that the write SHALL NOT become visible |
|
Thanks @mukund-thakur Had some thoughts over the w/e
The s3a stream abort() does do retries right now, and I think that's probably a mistake for something which is only cleanup and is usually invoked when things are starting to go wrong (e.g. data post/complete). Saying "no retries here", means that we can have a fast abort() rather than a 60-120 second spin before the failure is caught and swallowed. I'll change the relevant bits of s3a abort to Makes sense? |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
mehakmeet
reviewed
Feb 10, 2021
|
|
||
| /** | ||
| * Abort any active uploads, enter closed state. | ||
| * @return |
There was a problem hiding this comment.
missing Javadoc after @return
steveloughran
force-pushed
the
incoming/limj/HADOOP-16906-abort-trunk
branch
from
8cb92e3
to
af303a1
Compare
…ble output stream to be terminated 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.)
* markdown spec (really needs outputstream.md in, but...)
* stats collected on invocations of abort and multpart uploads
-counters and durations
* stats of streams propagated to filesystem in abort()
* tests use stats to verify what happens
* test to verify that aborted stream doesn't delete/overwrite an existing file
* tests verify hasCapability("abortable")
Change-Id: I104a8e6ca6d16aa9706c411ffae871409ea9ec22
HADOOP-16906. Abortable
* Added a result which is used in testing.
* added hasPathSupport() in S3A FS, changed capability name to suit
* clarified what happens on a closed stream.
* declared that cleanup (as opposed to the actual cancel)
MUST BE non-retrying, so abort() doesn't spin for a long time in
the presence of network failures.
* modified S3A BlockOutputStream to not retry. there or in the cleanup in close()
Change-Id: I82b8991c99faef8f5ff7d11c3648ac73d6540f85
Change-Id: Iced57b7402649d77cb6dfc243e6fcc80f6448a23
steveloughran
force-pushed
the
incoming/limj/HADOOP-16906-abort-trunk
branch
from
af303a1
to
b5c8163
Compare
|
💔 -1 overall
This message was automatically generated. |
|
final bits of style.... |
Change-Id: I258e06b0b9ee4fce15162b57eb7ce4ea7012e485
|
💔 -1 overall
This message was automatically generated. |
|
test run failures are all container OOM. As the last run was only to verify style &c -checks which pass, ignoring that and going with success of previous runs |
Change-Id: I78b62ab746ace501f260b9e933004d454b4cbbf3
|
🎊 +1 overall
This message was automatically generated. |
|
Finally! Thanks again everyone! |
asfgit
pushed a commit
that referenced
this pull request
Feb 17, 2021
Adds an Abortable.abort() interface for streams to enable output streams to be terminated; this is implemented by the S3A connector's output stream. It allows for commit protocols to be implemented which commit/abort work by writing to the final destination and using the abort() call to cancel any write which is not intended to be committed. Consult the specification document for information about the interface and its use. Contributed by Jungtaek Lim and Steve Loughran. Change-Id: I7fcc25e9dd8c10ce6c29f383529f3a2642a201ae
jojochuang
pushed a commit
to jojochuang/hadoop
that referenced
this pull request
May 23, 2023
Adds an Abortable.abort() interface for streams to enable output streams to be terminated; this is implemented by the S3A connector's output stream. It allows for commit protocols to be implemented which commit/abort work by writing to the final destination and using the abort() call to cancel any write which is not intended to be committed. Consult the specification document for information about the interface and its use. Contributed by Jungtaek Lim and Steve Loughran. Change-Id: Ifcc5a13b07376ebebcc21c7b21f4aed4a84214ac
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is #2667 with an extra commit; my changes
-counters and durations
Testing in progress.