[Bugfix][KV Offload] Do not let mark_miss() flip a pending or in-flight async lookup probe

[sohom-cs]

Purpose

When a slow KV offload tier (the filesystem or object-store tier) fails to load a block, it records "this block is not really there" so the scheduler stops retrying it. That note is written against whatever lookup currently exists for the block. If the request that started the load was cancelled in the meantime, and another request has since asked about the same block, the note lands on the newer, unfinished lookup, and the engine crashes on an internal consistency check. The trigger is ordinary traffic: a client disconnects while its cached prefix is being loaded, and another request shares that prefix. The result is that the whole engine goes down, not just one request. This PR makes the note apply only to lookups that have already finished.

Concretely, AsyncLookupManager.mark_miss() (vllm/v1/kv_offload/tiering/async_lookup.py:216-224) forces a cached lookup entry to RESOLVED/False without checking its phase. If the entry belongs to a newer probe that is still PENDING or IN_FLIGHT, that probe's own result later trips the phase asserts on the scheduler thread (drain_results() at :204, or flush() at :181), and the engine core dies.

How it happens with the FS tier (the OBJ tier calls mark_miss the same way, obj/manager.py:400):

1. Request A looks up key K and gets a hit. The tier starts a promotion (load job) for K.
2. A is aborted or finishes while the load is still in flight. on_request_finished calls cleanup("A"), which deletes the RESOLVED entry for K.
3. Request B looks up K. This creates a new PENDING entry, and flush() at the end of the step moves it to IN_FLIGHT.
4. A's load fails (file evicted, truncated or unreadable). get_finished_jobs() calls mark_miss([K]) (fs/manager.py:306), which flips B's IN_FLIGHT entry to RESOLVED/False.
5. B's next lookup() drains the probe result and hits assert state.phase is LookupPhase.IN_FLIGHT. If step 4 lands while the entry is still PENDING, flush() asserts instead.

Needed: a client disconnect during a promotion, a failed load, and another request that shares the prefix.

Fix: mark_miss() now overrides only RESOLVED entries. A PENDING or IN_FLIGHT entry belongs to a newer probe, and that probe returns its own verdict. The #49176 livelock guard is unchanged: if the block is still bad, the next failed load marks the resolved entry False exactly as before, and the request stops re-issuing the promotion. One side effect: a newer probe that read the file before the failure can still return True once, which costs one more failed load. It does not loop.

Not a duplicate: I checked the open PRs that touch async_lookup.py: #57474 (draft, per-request-group result publishing), #58168 (draft, manager lock()/unlock()) and #52103 (draft, provenance). None of them changes mark_miss or the phase handling. mark_miss itself came in with #49328; #55823, #54872 and #55075 are the neighbouring merged fixes.

Test Plan

python -m pytest tests/v1/kv_offload/tiering/test_async_lookup.py tests/v1/kv_offload/tiering/test_fs_tier.py -q -k "mark_miss or failed_load"
python -m pytest tests/v1/kv_offload/tiering/ -q
pre-commit run --from-ref origin/main --to-ref HEAD

New tests:

• test_async_lookup.py::test_mark_miss_skips_newer_unresolved_probe[in_flight|pending]: the unit sequence above, for both phases.
• test_fs_tier.py::test_failed_load_after_abort_does_not_break_newer_probe: the same sequence end to end on FileSystemTierManager, with a load that really fails.

Test Result

With the fix (macOS arm64, CPU, rebased on 32cc3f1ea):

tests/v1/kv_offload/tiering/test_async_lookup.py + test_fs_tier.py -k "mark_miss_skips_newer or failed_load_after_abort" ... 3 passed
tests/v1/kv_offload/tiering/ ... 436 passed, 1 skipped

On main (fix reverted, new tests kept): all 3 new tests fail on the engine asserts:

vllm/v1/kv_offload/tiering/async_lookup.py:204: AssertionError   (in_flight)
vllm/v1/kv_offload/tiering/async_lookup.py:181: AssertionError   (pending)
vllm/v1/kv_offload/tiering/async_lookup.py:204: AssertionError   (FS tier end to end)
3 failed

pre-commit (--from-ref origin/main --to-ref HEAD) and mypy-3.12 (manual stage): clean.

This is a scheduler-side control-path change; model outputs are unaffected, so no evals are needed.

Related: one of a few independent fixes from an audit of the KV transfer paths (CPU offload, NIXL, P2P): #59099, #59102, #59325, #59329. None depends on another; they can be reviewed and merged in any order.

AI assistance

I used an AI coding assistant (Claude) to audit this code path, write the fix and write the tests. I reviewed every changed line and ran the tests above myself. The commit carries a Co-authored-by trailer, as AGENTS.md asks.

[claude]

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

[github-actions]

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

PRs do not trigger a full CI run by default. Reviewers with write access and configured trusted contributors can comment /ci run for upstream CI or /amd-ci run for AMD CI only whenever CI signals are needed.

Once the PR is approved or has the ready label, the PR author can also use the corresponding /ci run, /ci retry, and /ci cancel commands, or their /amd-ci variants. New commits do not start upstream CI automatically.

If you have any questions, please reach out to us on Slack at https://slack.vllm.ai.

Agent Guidelines

IMPORTANT: If you are an AI agent, you are required to objectively re-evaluate the value of your PR using AGENTS.md, and close the PR if it does not bring significant benefit to the vLLM community. Failure to do so may result in an immediate ban.

🚀

[Alex-ai-future]

This is indeed a bug. Thanks for fixing it.
Since this branch is already guarded by state.phase is LookupPhase.RESOLVED, the following assignment appears redundant and could be removed:

state.phase = LookupPhase.RESOLVED

For a stricter design, mark_miss() could also take the lookup generation and update the state only when the generation matches. This would prevent a delayed failure from an older lookup from modifying a newer lookup for the same key.

[sohom-cs]

Thanks for the review. You're right about the assignment; removed in 986a55484.

On the generation check: I kept this PR to the crash fix. With the RESOLVED guard, a late failure from an older load can still turn a newer RESOLVED verdict into a miss, but that costs one extra recompute, not a crash. And a failed load usually means the file is bad, which is the case mark_miss exists for (#49176). Tracking a generation would mean the FS and OBJ managers recording it per load job, so I'd rather do that as a follow-up if @orozery wants the stricter version.