feat(upstream): add configurable transport buffer sizes for large uploads
Large PUT/POST uploads (>60MB) fail with context canceled when an nginx reverse proxy sits in front of oauth2-proxy. The root cause is that http.Transport inherits Go's default 4KB WriteBufferSize, requiring ~15,000 write syscalls for a 60MB upload. This generates backpressure on the nginx->oauth2-proxy pipe. Once nginx hits proxy_read_timeout between consecutive write ops, it closes the connection, canceling req.Context(), which propagates as context canceled on the in-flight RoundTrip to the upstream. Expose writeBufferSize and readBufferSize on the Upstream config struct, wired to transport.WriteBufferSize and transport.ReadBufferSize in newReverseProxy. Both default to 0 (preserving current behavior, Go uses 4KB). Setting writeBufferSize to 65536 (64KB) reduces write syscalls by 16x and resolves the timeout correlation for large uploads. Fixes #3389 Signed-off-by: Mateen Anjum <mateenali66@gmail.com>
This commit is contained in:
parent
da9123f740
commit
c2a5dfa4ea
|
|
@ -126,6 +126,15 @@ type Upstream struct {
|
||||||
// Defaults to 30 seconds.
|
// Defaults to 30 seconds.
|
||||||
Timeout *time.Duration `yaml:"timeout,omitempty"`
|
Timeout *time.Duration `yaml:"timeout,omitempty"`
|
||||||
|
|
||||||
|
// WriteBufferSize specifies the size of the write buffer used when writing to the upstream transport.
|
||||||
|
// A larger buffer reduces the number of syscalls for large request bodies (e.g., file uploads).
|
||||||
|
// If zero or not set, Go's default (currently 4KB) is used. Recommended: 65536 (64KB) for large upload scenarios.
|
||||||
|
WriteBufferSize *int `yaml:"writeBufferSize,omitempty"`
|
||||||
|
|
||||||
|
// ReadBufferSize specifies the size of the read buffer used when reading from the upstream transport.
|
||||||
|
// If zero or not set, Go's default (currently 4KB) is used.
|
||||||
|
ReadBufferSize *int `yaml:"readBufferSize,omitempty"`
|
||||||
|
|
||||||
// DisableKeepAlives disables HTTP keep-alive connections to the upstream server.
|
// DisableKeepAlives disables HTTP keep-alive connections to the upstream server.
|
||||||
// Defaults to false.
|
// Defaults to false.
|
||||||
DisableKeepAlives *bool `yaml:"disableKeepAlives,omitempty"`
|
DisableKeepAlives *bool `yaml:"disableKeepAlives,omitempty"`
|
||||||
|
|
|
||||||
|
|
@ -141,6 +141,16 @@ func newReverseProxy(target *url.URL, upstream options.Upstream, errorHandler Pr
|
||||||
transport.ResponseHeaderTimeout = *upstream.Timeout
|
transport.ResponseHeaderTimeout = *upstream.Timeout
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Configure transport buffer sizes for large request/response bodies.
|
||||||
|
// The default 4KB write buffer causes excessive syscalls for large uploads (e.g., 60MB = ~15,000 writes),
|
||||||
|
// which can trigger upstream proxy timeouts. See: https://github.com/oauth2-proxy/oauth2-proxy/issues/3389
|
||||||
|
if upstream.WriteBufferSize != nil {
|
||||||
|
transport.WriteBufferSize = *upstream.WriteBufferSize
|
||||||
|
}
|
||||||
|
if upstream.ReadBufferSize != nil {
|
||||||
|
transport.ReadBufferSize = *upstream.ReadBufferSize
|
||||||
|
}
|
||||||
|
|
||||||
// Configure options on the SingleHostReverseProxy
|
// Configure options on the SingleHostReverseProxy
|
||||||
if upstream.FlushInterval != nil {
|
if upstream.FlushInterval != nil {
|
||||||
proxy.FlushInterval = *upstream.FlushInterval
|
proxy.FlushInterval = *upstream.FlushInterval
|
||||||
|
|
|
||||||
|
|
@ -335,6 +335,30 @@ var _ = Describe("HTTP Upstream Suite", func() {
|
||||||
}),
|
}),
|
||||||
)
|
)
|
||||||
|
|
||||||
|
It("should configure transport buffer sizes when set", func() {
|
||||||
|
u, err := url.Parse("http://upstream:1234")
|
||||||
|
Expect(err).ToNot(HaveOccurred())
|
||||||
|
|
||||||
|
upstream := options.Upstream{
|
||||||
|
ID: "bufferSizeTest",
|
||||||
|
FlushInterval: &defaultFlushInterval,
|
||||||
|
InsecureSkipTLSVerify: ptr.To(false),
|
||||||
|
ProxyWebSockets: ptr.To(false),
|
||||||
|
Timeout: &defaultTimeout,
|
||||||
|
WriteBufferSize: ptr.To(65536),
|
||||||
|
ReadBufferSize: ptr.To(32768),
|
||||||
|
}
|
||||||
|
|
||||||
|
handler := newReverseProxy(u, upstream, nil)
|
||||||
|
proxy, ok := handler.(*httputil.ReverseProxy)
|
||||||
|
Expect(ok).To(BeTrue())
|
||||||
|
|
||||||
|
transport, ok := proxy.Transport.(*http.Transport)
|
||||||
|
Expect(ok).To(BeTrue())
|
||||||
|
Expect(transport.WriteBufferSize).To(Equal(65536))
|
||||||
|
Expect(transport.ReadBufferSize).To(Equal(32768))
|
||||||
|
})
|
||||||
|
|
||||||
It("ServeHTTP, when not passing a host header", func() {
|
It("ServeHTTP, when not passing a host header", func() {
|
||||||
req := httptest.NewRequest("", "http://example.localhost/foo", nil)
|
req := httptest.NewRequest("", "http://example.localhost/foo", nil)
|
||||||
req = middlewareapi.AddRequestScope(req, &middlewareapi.RequestScope{})
|
req = middlewareapi.AddRequestScope(req, &middlewareapi.RequestScope{})
|
||||||
|
|
|
||||||
|
|
@ -45,6 +45,13 @@ func validateUpstream(upstream options.Upstream, ids, paths map[string]struct{})
|
||||||
}
|
}
|
||||||
paths[upstream.Path] = struct{}{}
|
paths[upstream.Path] = struct{}{}
|
||||||
|
|
||||||
|
if upstream.WriteBufferSize != nil && *upstream.WriteBufferSize < 0 {
|
||||||
|
msgs = append(msgs, "upstream writeBufferSize must be greater than or equal to 0")
|
||||||
|
}
|
||||||
|
if upstream.ReadBufferSize != nil && *upstream.ReadBufferSize < 0 {
|
||||||
|
msgs = append(msgs, "upstream readBufferSize must be greater than or equal to 0")
|
||||||
|
}
|
||||||
|
|
||||||
msgs = append(msgs, validateUpstreamURI(upstream)...)
|
msgs = append(msgs, validateUpstreamURI(upstream)...)
|
||||||
msgs = append(msgs, validateStaticUpstream(upstream)...)
|
msgs = append(msgs, validateStaticUpstream(upstream)...)
|
||||||
return msgs
|
return msgs
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue