-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Description
Hey team 👋
We were recently looking at performance of a system that uses a lot of pipelines of moderate size (~100-200 commands) and noticed how based on the pipeline size its execution would take multiples of our normal latency (for example if normally a command has 5ms latency sometimes these pipelines would take 5/10/15/20ms).
I'm pretty sure this is happening because the bufio.Writer
pool.Conn
uses is created using the default size of 4kb
go-redis/internal/pool/conn.go
Line 36 in 0decfdc
cn.bw = bufio.NewWriter(netConn) |
// from https://github.com/golang/go/blob/master/src/bufio/bufio.go#L18-L20
const (
defaultBufSize = 4096
)
// ...
// from https://github.com/golang/go/blob/master/src/bufio/bufio.go#L607-L612
// NewWriter returns a new [Writer] whose buffer has the default size.
// If the argument io.Writer is already a [Writer] with large enough buffer size,
// it returns the underlying [Writer].
func NewWriter(w io.Writer) *Writer {
return NewWriterSize(w, defaultBufSize)
}
When looking for what other defaults look like I found redis/rueidis defaults both read and write buffers to 0.5Mib:
[permalink]
// ReadBufferEachConn is the size of the bufio.NewReaderSize for each connection, default to DefaultReadBuffer (0.5 MiB).
ReadBufferEachConn int
// WriteBufferEachConn is the size of the bufio.NewWriterSize for each connection, default to DefaultWriteBuffer (0.5 MiB).
WriteBufferEachConn int
I think having the ability to at least configure those sizes would be a big performance boost for users issuing many and/or "big" commands (scripts or long lists or args). What do you think? Also I'd be more than happy to contribute these changes, just wanted to see if this is something you agree with first. Thanks! ❤️