-
Notifications
You must be signed in to change notification settings - Fork 275
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
An async writable channel implementation that can send the bytes to multiple Readable channels #2898
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2898 +/- ##
============================================
+ Coverage 64.24% 67.35% +3.11%
- Complexity 10398 11441 +1043
============================================
Files 840 863 +23
Lines 71755 73289 +1534
Branches 8611 8815 +204
============================================
+ Hits 46099 49364 +3265
+ Misses 23004 21297 -1707
+ Partials 2652 2628 -24 ☔ View full report in Codecov by Sentry. |
ambry-commons/src/main/java/com/github/ambry/commons/PipedAsyncWritableChannel.java
Outdated
Show resolved
Hide resolved
ambry-commons/src/main/java/com/github/ambry/commons/PipedAsyncWritableChannel.java
Outdated
Show resolved
Hide resolved
if (exception == null) { | ||
totalBytesRead.getAndAdd(result); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only need to add once for primary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering if totalBytesRead
for primary and secondary may not match in case we close secondary early. Due to that, I was thinking to increment it for both. What do you say?
ambry-commons/src/main/java/com/github/ambry/commons/PipedAsyncWritableChannel.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall lgtm! Left some minor comments.
ambry-commons/src/main/java/com/github/ambry/commons/PipedAsyncWritableChannel.java
Show resolved
Hide resolved
ambry-commons/src/main/java/com/github/ambry/commons/PipedAsyncWritableChannel.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
This can be used to write uploaded bytes to multiple readers. This is inspired from POC by @justinlin-linkedin here - #2890!