google / google/tpu-sync

Consumer Thread Starvation in 2-Producer 1-Consumer Paradigm When Producer Fails Mid-Transfer Without TCP RST

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

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.