Skip to content
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.

Sign up for GitHub

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

moving UDP payload size to config option, improve throughput performance #5201

Merged
merged 3 commits into from
Dec 22, 2015
Merged

moving UDP payload size to config option, improve throughput performance #5201

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3bf5d9c
moving UDP payload size to config option, improve throughput performance
Dec 22, 2015
d885f41
removing golang reference
Dec 22, 2015
8057d6f
adding language to config sample for udp-payload-size
Dec 22, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions etc/config.sample.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,9 @@ reporting-disabled = false
# batch-pending = 5 # number of batches that may be pending in memory
# batch-timeout = "1s" # will flush at least this often even if we haven't hit buffer limit
# read-buffer = 0 # UDP Read buffer size, 0 means OS default. UDP listener will fail if set above OS max.

# set the expected UDP payload size; lower values tend to yield better performance, default is max UDP size 65536
# udp-payload-size = 65536

###
### [continuous_queries]
Expand Down
22 changes: 22 additions & 0 deletions services/udp/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,24 @@ const (
// Linux: sudo sysctl -w net.core.rmem_max=<read-buffer>
// BSD/Darwin: sudo sysctl -w kern.ipc.maxsockbuf=<read-buffer>
DefaultReadBuffer = 0

// DefaultUDPPayloadSize sets the default value of the incoming UDP packet
// to the spec max, i.e. 65536. That being said, this value should likely
// be tuned lower to match your udp_payload size if using tools like
// telegraf.
//
// https://en.wikipedia.org/wiki/User_Datagram_Protocol#Packet_structure
//
// Reading packets from a UDP socket in go actually only pulls
// one packet at a time, requiring a very fast reader to keep up with
// incoming data at scale. Reducing the overhead of the expected packet
// helps allocate memory faster (~10-25µs --> ~150ns with go1.5.2), thereby
// speeding up the processing of data coming in.
//
// NOTE: if you send a payload greater than the UDPPayloadSize, you will
// cause a buffer overflow...tune your application very carefully to match
// udp_payload for your metrics source
DefaultUDPPayloadSize = 65536
)

// Config holds various configuration settings for the UDP listener.
Expand All @@ -44,6 +62,7 @@ type Config struct {
BatchPending int `toml:"batch-pending"`
ReadBuffer int `toml:"read-buffer"`
BatchTimeout toml.Duration `toml:"batch-timeout"`
UDPPayloadSize int `toml:"udp-payload-size"`
}

// WithDefaults takes the given config and returns a new config with any required
Expand All @@ -65,5 +84,8 @@ func (c *Config) WithDefaults() *Config {
if d.ReadBuffer == 0 {
d.ReadBuffer = DefaultReadBuffer
}
if d.UDPPayloadSize == 0 {
d.UDPPayloadSize = DefaultUDPPayloadSize
}
return &d
}
3 changes: 3 additions & 0 deletions services/udp/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ retention-policy = "awesomerp"
batch-size = 100
batch-pending = 9
batch-timeout = "10ms"
udp-payload-size = 1500
`, &c); err != nil {
t.Fatal(err)
}
Expand All @@ -38,5 +39,7 @@ batch-timeout = "10ms"
t.Fatalf("unexpected batch pending: %d", c.BatchPending)
} else if time.Duration(c.BatchTimeout) != (10 * time.Millisecond) {
t.Fatalf("unexpected batch timeout: %v", c.BatchTimeout)
} else if c.UDPPayloadSize != 1500 {
t.Fatalf("unexpected udp-payload-size: %d", c.UDPPayloadSize)
}
}
6 changes: 1 addition & 5 deletions services/udp/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,6 @@ import (
)

const (
// UDPBufferSize is the maximum UDP packet size
// see https://en.wikipedia.org/wiki/User_Datagram_Protocol#Packet_structure
UDPBufferSize = 65536

// Arbitrary, testing indicated that this doesn't typically get over 10
parserChanLen = 1000
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you rename this constant to MaxUDPBufferSize and modify the service's Open function to return an error when the payload size is set too high in the config?

It would be nice to either error or give some kind of warning to the user if they set the buffer size unnecessarily high.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This diff actually removes that const in lieu of a config var. See original commit:

sebito91@3bf5d9c

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I know. I thought it might be valuable to keep this constant and use it to ensure that the payload size doesn't get set unnecessarily high by a user.

It's a usability concern, not one of correctness.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's nothing wrong with allocating a buffer size > 65536, the only reason that value was selected was to provide guidance on an upper bound. Nothing stopping you from setting that higher if you really want (in terms of usability).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never mind then.

Expand Down Expand Up @@ -163,7 +159,7 @@ func (s *Service) serve() {
return
default:
// Keep processing.
buf := make([]byte, UDPBufferSize)
buf := make([]byte, s.config.UDPPayloadSize)
n, _, err := s.conn.ReadFromUDP(buf)
if err != nil {
s.statMap.Add(statReadFail, 1)
Expand Down