-
Notifications
You must be signed in to change notification settings - Fork 349
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
switch from sysctl to setsockopt for setting recv/send buffer sizes #3594
switch from sysctl to setsockopt for setting recv/send buffer sizes #3594
Conversation
bbe102c
to
8253fe7
Compare
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.
There will be follow up PR(s) to ...
Obviously out of scope for this PR, but we might consider hidden CLI args (maybe non-hidden at some point) to control this as well
8253fe7
to
ab0eefc
Compare
I'd probably punt cli args altogether. The next iteration will be to selectively set buffers on the ports that actually need it. We don't even offer the ability to configure service port number individually. If we intend to support precision service socket configurations, we should do that all in one change set with consideration for consistency |
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. thanks!
@@ -421,6 +426,10 @@ fn udp_socket_with_config(config: SocketConfig) -> io::Result<Socket> { | |||
|
|||
let sock = Socket::new(Domain::IPV4, Type::DGRAM, None)?; |
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 think some places in the code are bypassing this method; e.g.
https://github.com/anza-xyz/agave/blob/21d754000/core/src/banking_stage/forwarder.rs#L53
https://github.com/anza-xyz/agave/blob/21d754000/gossip/src/cluster_info.rs#L227
https://github.com/anza-xyz/agave/blob/21d754000/gossip/src/cluster_info.rs#L3005
Problem
receive and send buffer sizes should not be set system wide to 128MB.
Summary of Changes
secksockopt
to set the send and receive buffer sizesNOTES
sysctl
.a) reduce the size of the send or recv buffer on the unused side of sockets
b) make better sizing decisions on socket buffers based on actual use