Add udp_buffer_size option to udp_listener

[sebito91]

Required for all PRs:

• CHANGELOG.md updated (we recommend not updating this until the PR has been approved by a maintainer)
• Sign CLA (if not already signed)
• README.md updated (if adding a new plugin)

Adding a config option to allow users to set the ReadBuffer for UDP sockets when created. This is intentionally separate from the OS settings which you may not want to make global for one socket call. If the user doesn't specify the setting, it defaults to the OS definition; otherwise it's set to udp_buffer_size.

NOTE: golint is blowing up with alerts for underscores, happy to change these if you'd like.
NOTE_2: @wrigtim contributed too!

[sparrc]

I'm not 100% sure I want this in, the reason being that I think linux will silently fail if this number is larger than the max allowed size (net.core.rmem_max).

I just don't want users to expect to be able to increase this buffer from this config setting alone......they need to increase net.core.rmem_max and increase this setting together, and it's easier to just provide the default way of increasing it via net.core.rmem_default.

[sparrc]

I think SetReadBuffer can return an error, probably best to check and log it if it errors

[sparrc]

I believe the buffer only needs to be set once, up here

[daviesalex]

Background for this patch/request - changing net.core.rmem_default system side is not possible for us. This determines the buffer used for every new socket, and high buffer values causes high memory utilization and bad performance (many, many cache misses) system wide. See https://blog.cloudflare.com/the-story-of-one-latency-spike/ for a good reason why you dont want to set the default value too high for all applications, but sadly we need away to boost this for telegraf because - as demonstrated below - without this we drop frames for telegraf.

I believe this is option read-buffer for the InfluxDB server; its there for the same reason I believe - this was put there by @sparrc in ./services/udp/service.go:

78e6979b (Cameron Sparr         2015-11-05 13:36:25 -0700 103)  if s.config.ReadBuffer != 0 {
78e6979b (Cameron Sparr         2015-11-05 13:36:25 -0700 104)          err = s.conn.SetReadBuffer(s.config.ReadBuffer)
78e6979b (Cameron Sparr         2015-11-05 13:36:25 -0700 105)          if err != nil {
78e6979b (Cameron Sparr         2015-11-05 13:36:25 -0700 106)                  s.Logger.Printf("Failed to set UDP read buffer to %d: %s",
78e6979b (Cameron Sparr         2015-11-05 13:36:25 -0700 107)                          s.config.ReadBuffer, err)
78e6979b (Cameron Sparr         2015-11-05 13:36:25 -0700 108)                  return err
78e6979b (Cameron Sparr         2015-11-05 13:36:25 -0700 109)          }
78e6979b (Cameron Sparr         2015-11-05 13:36:25 -0700 110)  }

To show why we need this for anything that receives UDP metrics, I produced this test server:

package main

import (
    "fmt"
    "net"
    "os"
)

const BufferSize = 1500

/* A Simple function to verify error */
func CheckError(err error) {
    if err  != nil {
        fmt.Println("Error: " , err)
        os.Exit(0)
    }
}

func main() {
    /* Lets prepare a address at any address at port 10001*/   
    ServerAddr,err := net.ResolveUDPAddr("udp","127.0.0.1:10001")
    CheckError(err)

    /* Now listen at selected port */
    ServerConn, err := net.ListenUDP("udp", ServerAddr)
    CheckError(err)
    defer ServerConn.Close()


    err = ServerConn.SetReadBuffer(BufferSize) 
    if err != nil {
        fmt.Printf("Failed to set UDP read buffer: %s", err)
    }


    total := 0
    l := 0
    buf := make([]byte, 1500)  // This will be the MTU of the interface

    for {

        n,addr,err := ServerConn.ReadFromUDP(buf)
        l = len(buf[0:n])
        total = total + l
        fmt.Println("Received ", l, " from ", addr, "(total ", total, ")")

        if err != nil {
            fmt.Println("Error: ",err)
        } 

    }
}

And ran a test:

dd if=/dev/zero bs=1k count=1024 |  nc -u 127.0.0.1 10001

Over 3 runs, this clearly shows that we drop a % of metrics:

Received  1500  from  127.0.0.1:44985 (total  147000 )
Received  1500  from  127.0.0.1:39648 (total  132000 )
Received  1500  from  127.0.0.1:42209 (total  130500 )

On a default box:

[root@chl-ftwl12 fd]# cat /proc/sys/net/core/rmem_default
524288
[root@chl-ftwl12 fd]# cat /proc/sys/net/core/rmem_max
33554432

Even boosting rmem_default:

[root@chl-ftwl12 fd]# echo 33554432 > /proc/sys/net/core/rmem_default

And restarting the server didnt help much:

Received  1500  from  127.0.0.1:38855 (total  141000 )
Received  1500  from  127.0.0.1:48477 (total  108000 )
Received  1500  from  127.0.0.1:49675 (total  136500 )

I honestly dont know why, I probably need to change the middle value in /proc/sys/net/ipv4/udp_mem or something. Regardless, thats not what we want to do - we want this only to affect telegraf (as we are trying to run telegraf on every machine, so changing the default value for every telegraf machine is totally impossible for us). Making a one line change:

const BufferSize = 33554432

And now we drop nothing, ever:

Received  1500  from  127.0.0.1:42310 (total  192000 )
Received  1500  from  127.0.0.1:54516 (total  192000 )
Received  1500  from  127.0.0.1:49174 (total  192000 )

Now I set it to a crazy value:

const BufferSize = 335544320000

And it seems to simply have set it to the maximum value, because there are still no drops. There is certainly no silent failure:

Received  1500  from  127.0.0.1:57161 (total  192000 )
Received  1500  from  127.0.0.1:40230 (total  192000 )
Received  1500  from  127.0.0.1:40230 (total  192000 )

We really believe this is pretty compelling. metric senders can burst to very, very high rates (as they are local) and are much more likely to overwhelm kernel buffers than most applications. It does not appear to silently drop anything if set to a crazy high value on Linux (its very possible that other OS's do something else - Google suggests BSD does odd things?)

What we hope is to merge this with a clear warning that this is unnecessary to tune unless you have collectors injecting megabytes of metrics in very short periods of time (which we do) and that any changes should be tested very thoroughly.

[sparrc]

You may be right that it doesn't silently fail, but instead just sets it to the maximum.

fwiw I don't think doing echo 33554432 > /proc/sys/net/core/rmem_default will immediately set the default higher without a reboot of the machine, I believe you need to use sysctl for it to take effect immediately.

Anyways, I understand now the use-case of wanting to set the max for just a single server rather than for everything. I can merge the change but just put a note in the config explaining that the user first needs to adjust rmem_max using sysctl -w net.core.rmem_max=N before this config option will have any effect.

[sebito91]

Some settings do require you to reboot, others don't (you can reload sysctl settings and any new sockets created will pick up the new settings, at least in normal linux distros hehe, see here).

Made the err code change you requested and will add some docs about boosting the max and allocating something underneath this where possible.

(Let me know if you want me to squash...)

[daviesalex]

@sparrc writing into /proc/ should take effect for all future sockets, but as @sebito91 pointed out is not super relevant. Is there anything else we can provide in this ticket to help?

We absolutely need this to make effective use of telegraf as a source for UDP data; updating the defaults is impossible for us and so merging this is important if we are not have to indefinitely patch telegraf. It seems very likely that anybody else who is using the UDP listener for bursty data sources (which UDP is likely to produce) will also want this.

[sparrc]

nope, I don't think there are any other changes needed, just need to get around to merging it

[daviesalex]

Any chance this can be added to a v1.2 nightly early in the cycle (if so, we can roll out and test)

[sparrc]

yes, will do