[ 
https://issues.apache.org/jira/browse/CASSANDRA-15900?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17144408#comment-17144408
 ] 

Caleb Rackliffe edited comment on CASSANDRA-15900 at 6/24/20, 9:17 PM:
-----------------------------------------------------------------------

[~jasonstack] Made a couple comments inline in the PR where I thought that 
would be easier. Then there are a few higher level things:

* It might be worthwhile to have a test in {{AsyncStreamingOutputPlusTest}} 
that verifies {{AsyncStreamingOutputPlus#writeFileToChannel()}} closes the 
provided channel.
* {{AsyncStreamingOutputPlus#writeFileToChannel(FileChannel, StreamRateLimiter, 
int)}} and {{AsyncStreamingOutputPlus#writeFileToChannelZeroCopy()}} may be 
better off at {{private}} visibility, given we're treating them as 
transport-level implementation details. (Perhaps {{writeFileToChannel}} would 
be easier to test at package-private though.)
* The JavaDoc for {{writeFileToChannel(FileChannel, StreamRateLimiter)}} is 
slightly out-of date now, given we've lowered the batch size for the SSL case. 
(We should make sure to preserve the bit about the method taking ownership of 
the {{FileChannel}}.)


was (Author: maedhroz):
[~jasonstack] Made a couple comments inline in the PR where I thought that 
would be easier. Then there are a few higher level things:

* It might be worthwhile to have a test in {{AsyncStreamingOutputPlusTest}} 
that verifies {{AsyncStreamingOutputPlus#writeFileToChannel()}} closes the 
provided channel.
* {{AsyncStreamingOutputPlus#writeFileToChannel(FileChannel, StreamRateLimiter, 
int)}} and {{AsyncStreamingOutputPlus#writeFileToChannelZeroCopy()}} may be 
better off at `private` visibility, given we're treating them as 
transport-level implementation details. (Perhaps {{writeFileToChannel}} would 
be easier to test at package-private though.)
* The JavaDoc for {{writeFileToChannel(FileChannel, StreamRateLimiter)}} is 
slightly out-of date now, given we've lowered the batch size for the SSL case. 
(We should make sure to preserve the bit about the method taking ownership of 
the {{FileChannel}}.)

> Close channel and reduce buffer allocation during entire sstable streaming 
> with SSL
> -----------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-15900
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-15900
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Legacy/Streaming and Messaging
>            Reporter: ZhaoYang
>            Assignee: ZhaoYang
>            Priority: Normal
>             Fix For: 4.0-beta
>
>
> CASSANDRA-15740 added the ability to stream entire sstable by loading on-disk 
> file into user-space off-heap buffer when SSL is enabled, because netty 
> doesn't support zero-copy with SSL.
> But there are two issues:
>  # file channel is not closed.
>  # 1mb batch size is used. 1mb exceeds buffer pool's max allocation size, 
> thus it's all allocated outside the pool and will cause large amount of 
> allocations.
> [Patch|https://github.com/apache/cassandra/pull/651]:
>  # close file channel when the last batch is loaded into off-heap bytebuffer. 
> I don't think we need to wait until buffer is flushed by netty.
>  # reduce the batch to 64kb which is more buffer pool friendly when streaming 
> entire sstable with SSL.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

Reply via email to