-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Update collectd and graphite UDP listeners with perf enhancements #4681
Conversation
fe03f84
to
4fea514
Compare
@@ -53,7 +53,7 @@ type Service struct { | |||
wg sync.WaitGroup | |||
err chan error | |||
stop chan struct{} | |||
ln *net.UDPConn | |||
conn *net.UDPConn |
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.
changed this just for consistency with graphite and udp services
4fea514
to
e0bcb57
Compare
We need the CHANGELOG updated, and https://github.com/influxdb/influxdb/blob/master/etc/config.sample.toml updated to include these new values. |
CHANGELOG should be updated in the "Features" section -- allow read buffer size to be set. |
DefaultBatchDuration = toml.Duration(10 * time.Second) | ||
|
||
DefaultTypesDB = "/usr/share/collectd/types.db" | ||
|
||
// DefaultUDPReadBuffer is the default UDP read buffer | ||
// increasing this increases the number of UDP packets that can be handled. |
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 a little vague. Why not be precise and repeat the text in the Go docs?
"Sets the size of the operating system's receive buffer associated with the UDP traffic "
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.
kk 👍
Does the buffer size apply to TCP traffic? If so, we should set it for Graphite TCP, right? |
4c8a3da
to
d219361
Compare
For graphite we can't change the tcp read buffer at the moment. We would have the change the |
Somewhere deep down, SetReadBuffer() will be doing the equivalent of a setsockopt() with SO_RCVBUF option. Note that Linux (presumably other *nixes) has maximums that this can be set to. The following are from my workstation, running kernel 4.2:
These can of course be modified with |
You can do |
@dswarbrick That is also what I get:
maybe it can adjust the max? I'm not sure... |
@nkatsaros, from what I understand, |
@sparrc Need to be root to modify the OS defaults / max. These should not be fiddled with directly by InfluxDB, as they will affect all future sockets, in all applications. It would be worth documenting how to change them, or linking to relevant documentation. |
oh, looks like BSD already has the max set higher, that would explain why it worked OSX 10.11 (8MB)
FreeBSD 10.1 (2MB)
Get it together Linux!! I'll put a note in the documentation |
f1eafb1
to
3696213
Compare
Thanks for the heads-up @dswarbrick @sparrc -- looks like we have documentation issue here. |
716768e
to
23510bd
Compare
0bff268
to
6b37d29
Compare
3475c72
to
7d38c59
Compare
7d38c59
to
10307fe
Compare
10307fe
to
9625953
Compare
|
||
### BSD/Darwin | ||
|
||
On BSD/Darwin systems you need to add about a 15% padding to the kernel 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.
@otoolep finally tracked down the BSD/Darwin failures I was seeing related to this. This PR should be good to merge now.
closes #4678
related to #4676
cc @otoolep