HDFS-14564: Add libhdfs APIs for readFully; add readFully to ByteBufferPositionedReadable #963
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. |
|
Thanks for the patch @sahilTakiar . Well-written test case. Looks good to me. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
+1 |
jojochuang
reviewed
Jun 21, 2019
| checkStream(); | ||
| if (!(in instanceof ByteBufferPositionedReadable)) { | ||
| throw new UnsupportedOperationException("This stream does not support " + | ||
| "positioned reads with byte buffers."); |
There was a problem hiding this comment.
The exception message could be confusing; it's not the CryptoInputStream that doesn't support byte buffer positioned reads, it's the input stream wrapped by it. How about printing the class name of the object in?
jojochuang
reviewed
Jun 21, 2019
| * <p> | ||
| * This does not change the current offset of a file, and is thread-safe. | ||
| * | ||
| * <i>Warning: Not all filesystems satisfy the thread-safety requirement.</i> |
There was a problem hiding this comment.
I don't understand what this means. Does it mean the implementation of this interface must guanrantee thread safety despite the fact that not all file systems provide intrinsic thread-safety?
There was a problem hiding this comment.
I originally copied this from PositionedReadable, but after taking a second look, the only two filesystems that implement this interface are CryptoInputStream and DFSInputStream and both of their implementations are thread safe. So I removed this.
jojochuang
reviewed
Jun 21, 2019
| ((ByteBufferPositionedReadable) in).readFully(position, buf); | ||
| } else { | ||
| throw new UnsupportedOperationException("Byte-buffer pread " + | ||
| "unsupported by input stream"); |
There was a problem hiding this comment.
Similarly, would be nice to log the class name of the input stream.
jojochuang
requested changes
Jun 21, 2019
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
sahilTakiar
force-pushed
the
HDFS-14564
branch
from
6f976b1 to
11fdcb6
Compare
|
💔 -1 overall
This message was automatically generated. |
…erPositionedReadable
sahilTakiar
force-pushed
the
HDFS-14564
branch
from
11fdcb6 to
e5c7553
Compare
|
💔 -1 overall
This message was automatically generated. |
|
@jojochuang @smengcl any other comments? |
|
Sorry, I missed to update. +1 |
|
Looks good. Thanks for the work @sahilTakiar . Thanks for committing @jojochuang ! |
smengcl
pushed a commit
to smengcl/hadoop
that referenced
this pull request
Oct 8, 2019
…erPositionedReadable (apache#963) Contributed by Sahil Takiar. Reviewed-by: Siyao Meng <smeng@cloudera.com> (cherry picked from commit 13b427f) Change-Id: I66ecf3a4c0657ca2129e3ab5ec04739c0f096903
shanthoosh
pushed a commit
to shanthoosh/hadoop
that referenced
this pull request
Oct 15, 2019
prateekm jagadish-v0 please take a look. Would be nice to deploy this before the meetup talking about Samza 1.0 🙂 Author: Daniel Chen <dchen1@linkedin.com> Reviewers: Jagadish <jagadish@apache.org> Closes apache#963 from dxichen/update-website-release-meetups
amahussein
pushed a commit
to amahussein/hadoop
that referenced
this pull request
Oct 29, 2019
…erPositionedReadable (apache#963) Contributed by Sahil Takiar. Reviewed-by: Siyao Meng <smeng@cloudera.com>
RogPodge
pushed a commit
to RogPodge/hadoop
that referenced
this pull request
Mar 25, 2020
…erPositionedReadable (apache#963) Contributed by Sahil Takiar. Reviewed-by: Siyao Meng <smeng@cloudera.com>
deepakdamri
pushed a commit
to acceldata-io/hadoop
that referenced
this pull request
Oct 25, 2024
…erPositionedReadable (apache#963) Contributed by Sahil Takiar. Reviewed-by: Siyao Meng <smeng@cloudera.com>
deepakdamri
added a commit
to acceldata-io/hadoop
that referenced
this pull request
Oct 25, 2024
…erPositionedReadable (apache#963) Contributed by Sahil Takiar. Reviewed-by: Siyao Meng <smeng@cloudera.com>
deepakdamri
pushed a commit
to acceldata-io/hadoop
that referenced
this pull request
Oct 25, 2024
…erPositionedReadable (apache#963) Contributed by Sahil Takiar. Reviewed-by: Siyao Meng <smeng@cloudera.com>
deepakdamri
pushed a commit
to acceldata-io/hadoop
that referenced
this pull request
Oct 29, 2024
…erPositionedReadable (apache#963) Contributed by Sahil Takiar. Reviewed-by: Siyao Meng <smeng@cloudera.com>
deepakdamri
pushed a commit
to acceldata-io/hadoop
that referenced
this pull request
Nov 20, 2024
…erPositionedReadable (apache#963) Contributed by Sahil Takiar. Reviewed-by: Siyao Meng <smeng@cloudera.com> asd
deepakdamri
pushed a commit
to acceldata-io/hadoop
that referenced
this pull request
Nov 28, 2024
…erPositionedReadable (apache#963) Contributed by Sahil Takiar. Reviewed-by: Siyao Meng <smeng@cloudera.com>
deepakdamri
pushed a commit
to acceldata-io/hadoop
that referenced
this pull request
Dec 2, 2024
…erPositionedReadable (apache#963) Contributed by Sahil Takiar. Reviewed-by: Siyao Meng <smeng@cloudera.com>
deepakdamri
pushed a commit
to acceldata-io/hadoop
that referenced
this pull request
Dec 2, 2024
…erPositionedReadable (apache#963) Contributed by Sahil Takiar. Reviewed-by: Siyao Meng <smeng@cloudera.com>
deepakdamri
pushed a commit
to acceldata-io/hadoop
that referenced
this pull request
Dec 2, 2024
…erPositionedReadable (apache#963) Contributed by Sahil Takiar. Reviewed-by: Siyao Meng <smeng@cloudera.com>
harshith-212
added a commit
to acceldata-io/hadoop
that referenced
this pull request
Mar 5, 2025
…ByteBufferPositionedReadable (apache#963) Contributed by Sahil Takiar." This reverts commit 12d7d26.
shubhluck
pushed a commit
to acceldata-io/hadoop
that referenced
this pull request
Mar 5, 2025
…ByteBufferPositionedReadable (apache#963) Contributed by Sahil Takiar." This reverts commit 12d7d26.
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.
HDFS-14564: Add libhdfs APIs for readFully; add readFully to ByteBufferPositionedReadable
readFullytoByteBufferPositionedReadableand exposes it via libhdfsPositionedReadable#readFullyvia libhdfshdfsPreadandhdfsReadif the underlying stream supportsByteBufferreads, theByteBufferAPIs will be used