-
Notifications
You must be signed in to change notification settings - Fork 101
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
unix: make timeout and buffer size configurable #64
Conversation
This helps avoiding errors when sending a lot of metrics with a low frequency.
Sorry to be dense. What settings do you use to help your case? |
@@ -56,6 +56,8 @@ | |||
public final class NonBlockingStatsDClient implements StatsDClient { | |||
|
|||
private static final int PACKET_SIZE_BYTES = 1400; | |||
private static final int SOCKET_TIMEOUT_MS = 100; | |||
private static final int SOCKET_BUFFER_BYTES = -1; |
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.
Could you confirm where this default comes from? Would this be the same as not providing a default and falling back on whatever the system has set, or would this not set a limit?
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 would keep the exact settings what we use today:
- 100ms timeout
- default socket write buffer size (because any size <= 0 is ignored)
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.
Ok great, the timeout made sense, but wasn't aware of the socket size being ignored if less than zero. Thanks!
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.
It's not in the library but in our own code :)
if (bufferSize > 0) {
clientChannel.setOption(UnixSocketOptions.SO_SNDBUF, bufferSize);
}
Currently those can not be changed. But having a higher timeout and bigger buffer size would help us send burst of metrics without having to smooth that in the calling code. |
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.
Thanks @iksaif for this contribution! Looks good to me :) 🥇
This helps avoiding errors when sending a lot of metrics with a low frequency.