HADOOP-16132 Support multipart download in S3AFileSystem #645
Conversation
This file contains hidden or 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
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
noslowerdna
reviewed
Mar 27, 2019
| */ | ||
| public static final boolean CHANGE_DETECT_REQUIRE_VERSION_DEFAULT = true; | ||
|
|
||
| public static final String MULTIPART_DOWNLOAD_ENABLED = |
Contributor
There was a problem hiding this comment.
should add javadoc for each of these, and add to core-default.xml
Contributor
There was a problem hiding this comment.
Some documentation should be added to hadoop-aws/index.md and hadoop-aws/performance.md about this capability.
noslowerdna
reviewed
Mar 27, 2019
|
|
||
| public static final String MULTIPART_DOWNLOAD_CHUNK_SIZE = | ||
| "fs.s3a.multipartdownload.chunk-size"; | ||
| public static final long DEFAULT_MULTIPART_DOWNLOAD_CHUNK_SIZE = 262_144; |
Contributor
There was a problem hiding this comment.
the javadoc should explain the difference between part size and chunk size, also why a new configuration property is needed instead of reusing fs.s3a.multipart.size
noslowerdna
reviewed
Mar 27, 2019
|
|
||
| public static final String MULTIPART_DOWNLOAD_NUM_THREADS = | ||
| "fs.s3a.multipartdownload.num-threads"; | ||
| public static final int DEFAULT_MULTIPART_DOWNLOAD_NUM_THREADS = 8; |
Contributor
There was a problem hiding this comment.
why not reuse fs.s3a.threads.max (and ideally, the existing thread pool corresponding to that config)?
noslowerdna
reviewed
Mar 27, 2019
| String text = String.format("%s %s at %d", | ||
| operation, uri, rangeStart); | ||
|
|
||
| S3Object object = Invoker.once(text, "s3a://" + key + "/" + bucket, |
Contributor
There was a problem hiding this comment.
Do the key and bucket need to be switched here? Also, use requestBucket instead of bucket?
noslowerdna
reviewed
Mar 27, 2019
| */ | ||
| public abstract class AbortableInputStream extends InputStream { | ||
|
|
||
| public abstract void abort(); |
noslowerdna
reviewed
Mar 27, 2019
|
|
||
| private final S3ObjectInputStream s3ObjectInputStream; | ||
|
|
||
| public AbortableS3ObjectInputStream(S3ObjectInputStream s3ObjectInputStream) { |
Contributor
There was a problem hiding this comment.
add constructor javadoc
noslowerdna
reviewed
Mar 27, 2019
|
|
||
| public static final String MULTIPART_DOWNLOAD_BUFFER_SIZE = | ||
| "fs.s3a.multipartdownload.buffer-size"; | ||
| public static final long DEFAULT_MULTIPART_DOWNLOAD_BUFFER_SIZE = 20_000_000; |
Contributor
There was a problem hiding this comment.
How did you determine the optimal value for this? Javadoc for it should specify that the size is "(in bytes)".
steveloughran
added
the
fs/s3
|
@justinuang are you still working on this? |
Author
|
Sorry, I am not anymore. We can close it.
…On Wed, Jul 10, 2019 at 5:09 PM bolkedebruin ***@***.***> wrote:
@justinuang <https://github.com/justinuang> are you still working on this?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#645?email_source=notifications&email_token=AACHNJ4Z7ZI4II47SIUXCKTP6ZFWXA5CNFSM4HBQ2PH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZUXYEY#issuecomment-510229523>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AACHNJ7OTVPX3IWWXFAODSDP6ZFWXANCNFSM4HBQ2PHQ>
.
|
shanthoosh
pushed a commit
to shanthoosh/hadoop
that referenced
this pull request
Oct 15, 2019
… instead of run-time objects As per subject, changed caching table descriptor to take table descriptor instead of run-time objects - Added BaseHybridTableDescriptor, which models a hybrid table that may contain other tables - Modified StreamApplicationDescriptorImpl to also include tables contained within a hybrid table Author: Wei Song <wsong@linkedin.com> Reviewers: Jagadish Venkatraman <jvenkatraman@linkedin.com> Closes apache#645 from weisong44/SAMZA-1854 and squashes the following commits: 2c0d1362 [Wei Song] Updated based on review comments dd18bbee [Wei Song] Merge branch 'master' into SAMZA-1854 a6c94ad [Wei Song] Merge remote-tracking branch 'upstream/master' 41299b5 [Wei Song] Merge remote-tracking branch 'upstream/master' 239a095 [Wei Song] Merge remote-tracking branch 'upstream/master' a87a9b04 [Wei Song] SAMZA-1854: Changed caching table descriptor to take table descriptor instead of run-time objects eca0020 [Wei Song] Merge remote-tracking branch 'upstream/master' 5156239 [Wei Song] Merge remote-tracking branch 'upstream/master' de708f5 [Wei Song] Merge remote-tracking branch 'upstream/master' df2f8d7 [Wei Song] Merge remote-tracking branch 'upstream/master' f28b491 [Wei Song] Merge remote-tracking branch 'upstream/master' 4782c61 [Wei Song] Merge remote-tracking branch 'upstream/master' 0440f75 [Wei Song] Merge remote-tracking branch 'upstream/master' aae0f38 [Wei Song] Merge remote-tracking branch 'upstream/master' a15a7c9 [Wei Song] Merge remote-tracking branch 'upstream/master' 5cbf9af [Wei Song] Merge remote-tracking branch 'upstream/master' 3f7ed71 [Wei Song] Added self to committer list
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
No description provided.