Consumer Thread Starvation in 2-Producer 1-Consumer Paradigm When Producer Fails Mid-Transfer Without TCP RST
- Dominant language
- C++
- Stars
- 132
- Forks
- 32
- Avg merge
- 1d 6h
- Merged PRs (30d)
- 173
Description
**Impact:** in a multi-producer disaggregated setup, a single prefiller that stops responding without closing its socket blocks KV-cache reads from every *other* prefiller for up to `timeout_s_` (default 120s), for as long as requests to the dead peer keep arriving. There is no fault isolation between peers on the consumer's handshake path.
Line numbers below are at `083aec4`.
## Mechanism
1. `push_pool_` is a fixed 4-thread pool (`tpu_sync/kv_cache/kv_cache_manager_base.cc:391-394`, and again at `491-494`).
2. `StartRead` schedules the *entire* control handshake onto that pool as one blocking closure (`tpu_sync/core/kv_cache_manager_with_transfer.cc:1982-2038`): `ConnectTcp` at 1988, header/block-id writes, then `ReadControlResponseHeader` at 2016.
3. Each of those calls is bounded by `timeout_s_`, which defaults to **120.0** (`tpu_sync/core/kv_cache_manager_with_transfer.h:576`). The handshake is a few hundred bytes but inherits the bulk-transfer timeout.
So 4 in-flight reads to a peer that accepts connections and never answers occupy all 4 workers for 120s each. Reads to healthy peers sit in the pool's queue behind them. If traffic to the dead peer continues, the pool never drains.
## This is already tested, just not across peers
`tpu_sync/core/kv_cache_manager_with_transfer_control_test.cc:247` (`ConsumerGivesUpOnProducerThatNeverAnswers`) dispatches `kPoolSize + 1` reads at a silent producer and asserts the last connects only after a worker gives up at the transfer timeout. The header comment at line 50 states it plainly:
> Both ends of a control handshake share one pool of four workers, so four stuck handshakes are enough to starve either side.
With one producer that's benign — the delayed request was going to the dead peer regardless. The gap is that the pool is shared across *all* peers, so the same mechanism delays traffic to healthy ones. `HandlersOutliveConsumersThatNeverSpeak` (line 184) shows the mirror case on the producer side.
## Reproduction
Extend the existing harness: stand up two `SilentProducer`-style listeners, make P0 accept-and-never-answer and P1 answer normally, issue `kPoolSize` reads to P0, then one to P1. P1's `accepted()` does not increment until a P0 worker times out. With `kTimeoutS = 0.5` in the test that's a ~0.5s delay; in production with the 120s default it's two minutes.
Contributor guide
Research direction
Start with the push_pool_ setup in tpu_sync/kv_cache/kv_cache_manager_base.cc and the StartRead handshake closure in tpu_sync/core/kv_cache_manager_with_transfer.cc, then read the timeout definition in tpu_sync/core/kv_cache_manager_with_transfer.h. Extend tpu_sync/core/kv_cache_manager_with_transfer_control_test.cc using the existing silent-producer harness, preserving the current timeout tests. Done means requests to a responsive producer are not delayed by stuck handshakes to another producer.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- backend, distributed-systems, networking
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 64/100