Basekick-Labs / Basekick-Labs/arc
cluster: replication_fetch_timeout_ms is not honored past the fetch ack header; a stalled peer blocks a pull worker without bound
- Dominant language
- Go
- Stars
- 677
- Forks
- 53
- Avg merge
- 9h 14m
- Merged PRs (30d)
- 164
Description
Found while reviewing the #759/#795 fix; pre-existing and independent of it.
`cluster.replication_fetch_timeout_ms` (default 60 s) is documented as the puller-side per-fetch overall timeout, and `Puller.pullOnce` does derive `fetchCtx` from it. But `FetchClient.Fetch` (`internal/cluster/filereplication/fetch_client.go`) only honors that deadline for the dial and the request send:
1. It sets `conn.SetDeadline(ctxDeadline)` right after dialing, then calls `protocol.ReceiveMessage(conn, f.ResponseHeaderTimeout)` for the ack header. `ReceiveMessage` (`internal/cluster/protocol/codec.go`) does `conn.SetReadDeadline(time.Now().Add(timeout))`, which **replaces** the ctx-derived deadline with the 15 s default, and `startFilePullerLocked` only sets `DialTimeout`, so `ResponseHeaderTimeout` is not tunable from cluster config. A peer that accepts and never answers costs 15 s per attempt regardless of `replication_fetch_timeout_ms`.
2. `ReceiveMessage` then runs `defer conn.SetReadDeadline(time.Time{})`, which **clears** the read deadline entirely, and the body transfer that follows is a bare `io.CopyN(mw, conn, ack.SizeBytes)` with no context and no deadline. A peer that acks and then stalls mid-body blocks the worker with no bound at all. With the default 4 workers, four such peers wedge the puller.
Live measurement while reproducing #795: three attempts against a paused peer took ~15 s in total (5 s handshake each), not the 60 s the config suggests; the stalled-body case is unbounded.
Fix shape: keep the ctx deadline authoritative (`ReceiveMessage` should not extend past it: use the earlier of the two, and not clear the deadline on return when the caller set one), and bound the body copy with the same ctx (a deadline-aware reader, or `SetReadDeadline` before `CopyN` and a goroutine that closes the conn on `ctx.Done()`). Expose `ResponseHeaderTimeout` from the coordinator or derive it from the fetch timeout.
Contributor guide
Research direction
Read internal/cluster/filereplication/fetch_client.go, internal/cluster/protocol/codec.go, and the Puller.pullOnce/startFilePullerLocked call path. Trace how replication_fetch_timeout_ms reaches FetchClient.Fetch, then verify that both the response header and body transfer stop within the same context deadline and that the header timeout is configurable or derived from the fetch timeout.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- databases, distributed-systems
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 58/100