[ 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