fix(s3): add HTTP/2 keep-alive and write timeouts to avoid wedged requests (#1490) - #1491
fix(s3): add HTTP/2 keep-alive and write timeouts to avoid wedged requests (#1490)#1491Alexays wants to merge 5 commits into
Conversation
…uests The S3 client sets no per-request timeout, so a connection that stops making progress (a throttling / back-pressuring endpoint that stops reading the request or sending a response) wedges an in-flight request forever. During object-disk server-side copy this manifests as many UploadPartCopy goroutines stuck in http2 (*clientStream).writeRequest with no way to recover, hanging the whole backup (and, in watch mode, the daemon). Always build a custom HTTP transport and configure the HTTP/2 transport with a periodic keep-alive PING (ReadIdleTimeout / PingTimeout) and a WriteByteTimeout. A connection that stalls is now closed, the in-flight request fails, and it is retried on a fresh connection instead of blocking indefinitely. Fixes Altinity#1490
Slach
left a comment
There was a problem hiding this comment.
Thanks — the diagnosis in #1490 holds up (218 goroutines parked in http2clientStream.writeRequest, never recovering), and dropping a stalled h2 connection is the right direction. The implementation needs changes before it can land:
-
Hardcoded constants.
30s / 15s / 60sbaked into the code is the main blocker. Every other transport knob here is configurable (http_max_idle_conns*,http_write_buffer_size,http_idle_conn_timeout), these must be too —s3.http2_send_ping_timeout/http2_ping_timeout/http2_write_byte_timeout, empty = disabled. -
Unconditional custom transport is a regression of #1376. That guard was not cosmetic.
http.DefaultTransport.Clone()gives Go's defaults —MaxIdleConnsPerHost = 2. The AWS SDK's own transport (aws/transport/http/client.go) usesMaxIdleConnsPerHost = 10,MaxConnsPerHost = 2048, plus its dialer timeouts and dial tracing. Since allhttp_max_*default to0, this PR silently drops every user — including plain AWS S3 — from 10 idle conns per host to 2, i.e. TCP+TLS re-established for most requests atupload_concurrency > 2. Build the transport fromawshttp.NewBuildableClient().GetTransport()instead ofhttp.DefaultTransport. -
golang.org/x/net/http2isn't needed. This repo isgo 1.26;http.Transport.HTTP2(Go 1.24+) exposesSendPingTimeout/PingTimeout/WriteByteTimeoutfor the bundled h2 stack, no extra dependency and noConfigureTransportsordering concerns. -
h2Erris silently swallowed — ifConfigureTransportsfails the fix quietly does nothing. -
For the record, scope: AWS S3 does not negotiate h2 over ALPN, so these settings only take effect on S3-compatible endpoints (Scaleway, MinIO, Ceph). Plain HTTP/1.1 stalls are still uncovered — no
ResponseHeaderTimeoutanywhere. Worth a follow-up, not a blocker here.
I'm landing an adjusted version of this with the config options and the SDK-default transport; will credit this PR. Thanks for the thorough goroutine dump — that's what made it diagnosable.
An S3 request has no per-request timeout, so on endpoints which negotiate HTTP/2 (AWS S3 doesn't, most S3-compatible providers do) a connection that stops making progress wedges every request multiplexed over it forever -- hundreds of UploadPartCopy goroutines stuck in http2 writeRequest during object-disk server-side copy, never recovering. Configure http.Transport.HTTP2 (Go 1.24+, no golang.org/x/net/http2 needed) so an idle connection is PINGed and dropped when the peer stops answering, and a stalled write closes the connection, failing in-flight requests so they are retried on a fresh one. Timeouts are configurable via s3.http2_send_ping_timeout / http2_ping_timeout / http2_write_byte_timeout, empty disables them. The transport is now always built, so it starts from the AWS SDK default transport instead of http.DefaultTransport -- the latter would drop MaxIdleConnsPerHost from 10 to 2 for every user, regressing Altinity#1376. Fixes Altinity#1490, based on Altinity#1491
…ickhouse-backup Keep the reworked transport (configurable http2.* timeouts via buildHTTPTransport) from the amended commit; the original commit's hardcoded http2.ConfigureTransports block is superseded by it.
…ix/s3-http2-timeouts # Conflicts: # pkg/config/config_test.go # test/testflows/clickhouse_backup/tests/snapshots/cli.py.cli.snapshot
Signed-off-by: slach <bloodjazman@gmail.com>
Problem
Fixes #1490.
The S3 client sets no per-request timeout. When a connection stops making progress — a throttling / back-pressuring endpoint (e.g. Scaleway Object Storage) that stops reading the request or sending a response — an in-flight request wedges forever with no way to recover.
During object-disk server-side copy this is very visible: dozens of concurrent
UploadPartCopyrequests are multiplexed over a few HTTP/2 connections, and once one stalls, the goroutines pile up innet/http.(*http2clientStream).writeRequest(serialized per connection). A goroutine dump from a wedgedcreate_remoteshowed ~218 requests stuck there, 0 network, and the backup never progressing (inwatchmode the whole daemon wedges). It reproduces intermittently under load and never self-recovers.http_idle_conn_timeoutdoesn't help here — it only covers idle connections, not an active request whose write/response has stalled.Fix
Always build a custom transport and configure the HTTP/2 transport with health checks and a write timeout:
ReadIdleTimeout = 30s+PingTimeout = 15s— PING the peer when the connection goes idle; if there's no response, the connection is closed.WriteByteTimeout = 60s— close the connection if a single write stalls for too long (this is exactly thewriteRequeststall above).When a connection stalls it is now dropped, the in-flight request fails, and it is retried on a fresh connection instead of blocking indefinitely. Validated against Scaleway Object Storage: object-disk backups that previously wedged indefinitely now complete.
The timeouts are conservative constants here for a minimal change; happy to expose them as
s3config options if you'd prefer.