Skip to content

test_async_llm: reusing an aborted request id races the core abort — 26% failure rate under contention, on main #294

Description

@localai-bot

test_async_llm's async_llm test_abort and test_multi_abort leave other requests healthy
case fails about a quarter of the time under CPU contention, on main, with no
change to the engine. Found while gating #277; it is not caused by that
change
, which is why it gets its own issue rather than a silent fix.

The assertion

tests/vllm/v1/test_async_llm.cpp:325

// Reusing an aborted external id is valid once cleanup completes.
AsyncRequest reused = engine.add_request(
    "abort-a", "hello", Params(3, RequestOutputKind::kDelta));
CHECK(Drain(engine, reused) == 3);      // observed: 4

abort-a is first admitted with max_tokens=100000, allowed to emit a partial
delta, then aborted together with abort-b. The case then re-admits the SAME
external id with max_tokens=3 and expects exactly 3 tokens. It periodically
gets 4.

Why

AsyncLLM::abort removes the frontend state under output_processor_mutex_ and
then queues the core abort asynchronously
(src/vllm/v1/engine/async_llm.cpp abortengine_core_.abort_requests_async).
The guard the case relies on before reusing the id —
CHECK_FALSE(engine.has_unfinished_requests()) — reads
OutputProcessor::get_num_unfinished_requests(), i.e. frontend state only.
It says nothing about whether EngineCoreProc has drained the abort.

So when the engine thread is starved, the old abort-a can still be in the
scheduler when the new abort-a is registered, and a token frame belonging to
the previous incarnation is delivered to the new collector — one extra token.

Measured, paired, same box

40 concurrent copies of the single case, three rounds each, alternating builds
of the same binary in one worktree:

Tree round 1 round 2 round 3 total
main @ 848d4a87 11/40 9/40 11/40 31/120 (25.8%)
main + the #277 wiring 9/40 15/40 7/40 31/120 (25.8%)

Identical rate, so the #277 async-metrics wiring neither causes nor worsens it.
Serially on an idle box the binary is 10/10 green, which is why this shows up
only as an intermittent CI/ctest -j failure and why the binary already carries
a "known flaky under parallel ctest" reputation — the reputation is real, but
the cause is a genuine engine-frontend race, not merely slow scheduling.

Two candidate fixes

  1. Make the test's precondition honest: expose a core-side quiescence signal and
    wait on it before reusing an id, rather than inferring it from frontend
    state.
  2. Make id reuse safe by construction: have the frontend refuse to bind a
    collector for an id whose previous incarnation the core has not confirmed
    finished, mirroring how vLLM's AsyncLLM keeps the request id unique for the
    lifetime of the core request.

(2) is the stronger guarantee and matches what a production caller reusing a
request id after abort() would expect; (1) alone would leave the underlying
race reachable from the C ABI.

Reproducer

cmake -S . -B build-cpu -G Ninja -DCMAKE_BUILD_TYPE=Release \
      -DVLLM_CPP_CUDA=OFF -DVLLM_CPP_VULKAN=OFF -DVLLM_CPP_METAL=OFF
cmake --build build-cpu -j 18
for i in $(seq 1 40); do
  ( ./build-cpu/tests/test_async_llm -tc="async_llm test_abort*" >/dev/null 2>&1 \
      || echo F ) &
done; wait

Row: SERVE-ASYNC-LLM.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions