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

Open
#796 0 comments 0 reactions 0 assignees View on GitHub
bug
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.