Support setting options of boss in NettyServer#6947
Support setting options of boss in NettyServer#6947creamsoup merged 3 commits intogrpc:mastergrpc/grpc-java:masterfrom asdf2014:support_options_for_nettyserverCopy head branch name to clipboard
Conversation
|
|
creamsoup
left a comment
There was a problem hiding this comment.
just out of curiosity, which options are you planning to use?
| * Specifies a channel option. As the underlying channel as well as network implementation may | ||
| * ignore this value applications should consider it a hint. | ||
| * | ||
| * @since 1.28.1 |
There was a problem hiding this comment.
1.29.0 branch cut is already done. it should 1.30.0
| * @since 1.28.1 | |
| * @since 1.30.0 |
| addr, | ||
| new ReflectiveChannelFactory<>(NioServerSocketChannel.class), | ||
| new HashMap<ChannelOption<?>, Object>(), | ||
| channelOptions, |
There was a problem hiding this comment.
nit: can you rename this to childChannelOptions to avoid confusion?
|
Hi, @creamsoup . Thanks for your comments. All of them has been patched. Many options related to the boss ELG will be set, mainly the SO_BACKLOG parameter, because this has been hard-coded to 128, which is not enough in my case. |
|
that's good to know. thanks. |
|
I don't have expertise to review this and getting its context takes time. Maybe request review from someone else? 😄 |
|
Hi, @creamsoup . As @voidzcy said, would you please ask others to check it out. |
ejona86
left a comment
There was a problem hiding this comment.
I changed the PR title to remove "worker" since worker configuration was already supported.
|
merged, thanks @asdf2014! |
|
And actually, it looks like us setting the SO_BACKLOG was just cargo-culting (that is some very old code). We should probably just remove it and use Netty's default, which is 200 on Windows and the system max on Linux (commonly 128). https://github.com/netty/netty/blob/0cde4d9cb4d19ddc0ecafc5be7c5f7c781a1f6e9/transport/src/main/java/io/netty/channel/socket/DefaultServerSocketChannelConfig.java#L44 |
|
Hi, @ejona86 . Thanks for your approval. I agree with you, we shouldn't hard code the value of SO_BACKLOG option in |
No description provided.