vllm-project / vllm-project/aibrix

[Bug] Batch inference retries default to 120, letting a single request retry for ~70 minutes and outlive its request lock

Open
#2,546 2 comments 0 reactions 1 assignee Claimed by @Jeffwan View on GitHub
area/batch kind/bug
Dominant language
Go
Stars
5.1k
Forks
697
Avg merge
1d 19h
Merged PRs (30d)
104

Description

### Summary

`_DEFAULT_INFERENCE_MAX_RETRIES` is set to `120` for the per-send inference retry path. Combined with the capped exponential backoff and the channel's 30s HTTP timeout, a single failing request can stay in the retry loop for roughly **70 minutes** before it finally surfaces as a failure. During that time it is invisible in telemetry, and it outlives its own request lock, which allows another worker to dispatch the same request again.

### Environment

- Component: `python/aibrix` batch smart client
- Introduced in #2339 (`3fa8caa1`)

### Details

**1. The retry budget is far larger than intended**

```python
# python/aibrix/aibrix/batch/job_driver/base.py:90
_DEFAULT_INFERENCE_MAX_RETRIES = 120
_DEFAULT_NO_ENDPOINT_MAX_RETRIES = 120
_DEFAULT_RETRY_BASE_DELAY_SECONDS = 0.5
_DEFAULT_RETRY_MAX_DELAY_SECONDS = 5.0
```

`120` is a reasonable budget for the **NO_ENDPOINT** path — each attempt only re-queries service discovery, so `120 × 5s ≈ 10 minutes` is a sensible window to wait out a cold-starting GPU deployment.

It is not reasonable for the **per-send** path, where every attempt carries a full HTTP round trip. Before #2339 this value was `max_retries=5`; the two constants were introduced side by side and the inference one was raised along with the endpoint-recovery one.

**2. Worst-case duration**

Backoff (`engine.py:404`) is `min(0.5 * 2**attempt, 5.0)`, so it saturates at 5s after 4 attempts. Total sleep across 120 retries is `0.5+1+2+4 + 116×5 ≈ 588s`. Channels default to `timeout=30.0`, so:

| Failure mode | Per attempt | 121 attempts total |
|---|---|---|
| HTTP timeout | 30s | 3630s + 588s ≈ **70 min** |
| Fast 5xx | ~0s | ≈ **10 min** |

**3. The retry loop is invisible in telemetry**

`stats.record_start()` fires once in the feeder loop and `stats.record_complete()` once in `_process`'s `finally` block, while retries happen *inside* `_send_with_failover`. A request spinning in the retry loop therefore increments no window counter at all. The `Batch dispatch telemetry` line (`base.py:1431`) shows a fully idle window even though work is stuck:

```json
{
"job_id": "f0826a30-...",
"final": false,
"started_qps": 0,
"completed_qps": 0,
"failed_qps": 0,
"inflight": 14,
"concurrency_limit": 600,
"max_inflight": 600,
"started": 6882,
"completed": 6869,
"failed": 0,
"window_started": 0,
"window_completed": 0,
"window_failed": 0,
"avg_latency_seconds": null,
"p95_latency_seconds": null,
"event": "Batch dispatch telemetry"
}
```

`started - completed = 13` requests are outstanding, `failed: 0`, and nothing moved during the window. From an operator's view the job simply hangs near the tail with no signal explaining why.

**4. Requests outlive their lock — duplicate dispatch**

`BatchStorageAdapter` acquires the per-request lock with `expiration_seconds=envs.INFERENCE_TASK_TIMEOUT` (`adapter.py:124`), which defaults to **600s** (`envs.py:145`). Since a request can remain in flight for up to ~70 minutes, the lock expires roughly 60 minutes before the request resolves. Another worker will then see the request as unclaimed and dispatch it again, causing duplicate inference, duplicate cost, and potentially duplicate output rows.

### Expected behavior

A single request should give up within a bounded, operator-comprehensible time, and it must never remain in flight longer than the lock that protects it from duplicate dispatch.

### Proposed fix

1. Restore `_DEFAULT_INFERENCE_MAX_RETRIES` to a small value (~5). Leave `_DEFAULT_NO_ENDPOINT_MAX_RETRIES` at 120 — that path is correctly tuned.
2. Add a per-request wall-clock deadline around `_send_with_failover` and keep it strictly below `INFERENCE_TASK_TIMEOUT`, so lock lifetime always covers request lifetime regardless of how the retry count is configured.
3. Optionally surface retry activity in telemetry (e.g. a `retrying` counter) so a stalled tail is diagnosable without reading code.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.