-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
HBASE-23113 IPC Netty Optimization #679
Conversation
💔 -1 overall
This message was automatically generated. |
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.
What is this patch doing and do you have proof it does it? I see the notes on netty configs and how we intro configs for this base parameters. Have you been playing with these settings and if so, what have you found? Thanks you.
* and the decoder implementation this may be slower then just use the {@link #MERGE_CUMULATOR}. | ||
*/ | ||
public static final Cumulator COMPOSITE_CUMULATOR = new Cumulator() { | ||
@Override |
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.
Formatting is very strange shifted way to the right.
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.
@saintstack I reformat AbstractBatchDecoder class file.
@Override | ||
public ByteBuf cumulate(ByteBufAllocator alloc, | ||
ByteBuf cumulation, | ||
ByteBuf in) { | ||
ByteBuf buffer; | ||
if (cumulation.writerIndex() > cumulation | ||
.maxCapacity() | ||
- in.readableBytes() | ||
|| cumulation.refCnt() > 1) { | ||
// Expand cumulation (by replace it) when either there is not more room in the buffer | ||
// or if the refCnt is greater then 1 which may happen when the user use slice().retain() or | ||
// duplicate().retain(). | ||
// | ||
// See: | ||
// - https://github.com/netty/netty/issues/2327 | ||
// - https://github.com/netty/netty/issues/1764 | ||
buffer = expandCumulation(alloc, | ||
cumulation, | ||
in.readableBytes()); | ||
} else { | ||
buffer = cumulation; | ||
} | ||
buffer.writeBytes(in); | ||
in.release(); |
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.
This is all shifted to the right.
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.
@saintstack I have already reformat the whole code of this class.
@saintstack These netty configs improve the rpc performance of NettyRpcServer and NettyRpcConnection.And AbstractBatchDecoder improves decoding throughput.Not only the netty configs but also AbstractBatchDecoder have already used in other open source project like SOFA-RPC,which proves that RPC is greatly optimized. |
💔 -1 overall
This message was automatically generated. |
this.maxRetries = conf.getInt(CLIENT_CONNECT_MAX_RETRIES, 0); | ||
this.tcpNoDelay = conf.getBoolean(CLIENT_TCP_NODELAY, true); | ||
this.tcpKeepAlive = conf.getBoolean(CLIENT_TCP_KEEPALIVE, true); | ||
this.bufferLowWatermark = conf.getInt(CLIENT_BUFFER_LOW_WATERMARK, DEFAULT_CLIENT_BUFFER_LOW_WATERMARK); |
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.
This is only used for NettyRpcClient so better put this into the NettyRpcClient?
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.
This is only used for NettyRpcClient so better put this into the NettyRpcClient?
Yeah,I referred to other constant of RpcClient like SOCKET_TIMEOUT_WRITE so that I put this into RpcClient.I will put these constants into NettyRpcClient.
@@ -257,6 +258,7 @@ private void connect() { | |||
.option(ChannelOption.TCP_NODELAY, rpcClient.isTcpNoDelay()) | |||
.option(ChannelOption.SO_KEEPALIVE, rpcClient.tcpKeepAlive) | |||
.option(ChannelOption.CONNECT_TIMEOUT_MILLIS, rpcClient.connectTO) | |||
.option(ChannelOption.WRITE_BUFFER_WATER_MARK, new WriteBufferWaterMark(rpcClient.bufferLowWatermark, rpcClient.bufferHighWatermark)) |
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.
Create a WriteBufferWaterMark and store it in NettyRpcClient so we do not need to create it everytime?
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.
Create a WriteBufferWaterMark and store it in NettyRpcClient so we do not need to create it everytime?
There is really no need to create a WriteBufferWaterMark every time.I will modify this definition in NettyRpcConnection.
* You can check the method {@link AbstractBatchDecoder#channelRead(ChannelHandlerContext, Object)} ()} | ||
* to know the detail modification. | ||
*/ | ||
public abstract class AbstractBatchDecoder extends ChannelInboundHandlerAdapter { |
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.
Any idea that why netty does not implement this by default if this could greatly increase the performance? Performance is one of the most important thing in netty.
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.
Any idea that why netty does not implement this by default if this could greatly increase the performance? Performance is one of the most important thing in netty.
I will mention this idea to netty issue.As mentioned above,this optimization has no advantage in low-concurrency scenarios, and has a significant performance boost in boost throughput in high-concurrency scenarios.
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.
Just extends ByteToMessageDecoder and override the channelRead method? You can declare this class under the o.a.h.thirdparty.io.netty.handler.codec, so you can use the package private classes of netty.
And maybe we could land this on a feature branch first, and collect some performance numbers so it will also help the netty issue netty/netty#9637?
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.
Just extends ByteToMessageDecoder and override the channelRead method? You can declare this class under the o.a.h.thirdparty.io.netty.handler.codec, so you can use the package private classes of netty.
And maybe we could land this on a feature branch first, and collect some performance numbers so it will also help the netty issue netty/netty#9637?
Okay, I will try this.
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.
@Apache9 I have already removed this AbstactBatchDecoder in this pull request.
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
@SteNicholas Could you check why the errors w/ different hadoops please? Yeah, as per @Apache9 , is there test to show improvement? Thanks. |
@saintstack I wil check today. |
Any luck @SteNicholas w/ some numbers? Thanks. |
Any news @SteNicholas ? Thanks. |
Netty options in IPC Server/Client optimization:
1.SO_BACKLOG setting:Two queues are maintained in the Linux system kernel: syns queue and accept queue. The first is a semi-join queue that saves the connections to the synrecv state after receiving the client syn. The default netty is 128,io.netty.util.NetUtil#SOMAXCONN , and then read /proc/sys/net/core /somaxconn to continue to determine, and then there are some system level coverage logic.In some scenarios, if the client is far redundant to the server and the connection is established, it may not be enough. This value should not be too large, otherwise it will not prevent SYN-Flood attacks. The current value has been changed to 1024. After setting, the value set by yourself is equivalent to setting the upper limit because of the setting of the system and the size of the system. If some settings of the Linux system operation and maintenance are wrong, it can be avoided at the code level.At present, our Linux level is usually set to 128, and the final calculation will be set to 128.
2.WRITE_BUFFER_WATER_MARK setting:After WRITEBUFFERWATERMARK sets the maximum and minimum Buffer that can be temporarily stored on a connection, isWritable returns unwritable if the amount of data waiting to be sent for the connection is greater than the set value. In this way, the client can no longer send, preventing this amount of continuous backlog, and eventually the client may hang. If this happens, it is usually caused by slow processing on the server side. This value can effectively protect the client. At this point the data was not sent.
3.SO_REUSEADDR - Port multiplexing (allowing multiple sockets to listen on the same IP+ port): For time-wait links, it ensures that the server restarts successfully. In the case where some servers start up very quickly, it can prevent startup failure.
Netty decoder in IPC Server optimization:
Netty provides a convenient decoding tool class ByteToMessageDecoder, as shown in the top half of the figure, this class has accumulate bulk unpacking capability, can read bytes from the socket as much as possible, and then synchronously call the decode method to decode the business object. And compose a List. Finally, the traversal traverses the List and submits it to ChannelPipeline for processing. Here we made a small change, as shown in the bottom half of the figure, the content to be submitted is changed from a single command to the entire List, which reduces the number of pipeline executions and improves throughput. This mode has no advantage in low-concurrency scenarios, and has a significant performance boost in boost throughput in high-concurrency scenarios.