Repository navigation
fix(vlm): isolate short multimodal embeddings - #36724
kflansburg wants to merge 13 commits into
Conversation
Fresh encoder output can contain fewer embedding rows than the request's multimodal placeholder span. The late length assertion then exits the scheduler and restarts every TP rank.\n\nValidate fresh outputs before caching or concatenation, preserve batch shape for failed rows, synchronize request failures across ranks, and return an internal request error while healthy rows continue. Fence radix and disaggregated KV state until overlap validation completes.\n\nAccuracy: unchanged for valid requests; malformed requests now fail with HTTP 500.\nSpeed: valid multimodal forwards add a small fixed error-consensus collective.\nTests: focused multimodal, scheduler, disaggregation, beam, overlap, and cache tests.
Cover malformed fresh outputs, mixed healthy and failed requests, overlap validation fences, TP and PP error propagation, disaggregated cleanup, beam teardown, cache admission, and output cursor accounting.\n\nTests: 48 focused CPU tests.
Multimodal embedding validation relies on extend-only metadata. Pure decode and target-verify batches can retain stale request-shaped multimodal state after disaggregated merges, so inspecting those lists can terminate the scheduler before model execution. Run validation only for non-speculative extend modes, including mixed batches, and apply the same rule to batch-less worker calls. Accuracy: unchanged. Speed: decode and target-verify avoid unnecessary validation work. Tests: focused validation-index and request-isolation unit tests.
Cover pure decode, target verify, disaggregated decode merges, aligned extend and mixed batches, strict malformed extend metadata, and batch-less worker calls. Tests: 26 focused validation and request-isolation tests.
Overlap scheduling can plan the next multimodal chunk before the previous validation result is processed. Because unvalidated KV is intentionally excluded from radix cache, replanning rematches the old prefix and repeatedly allocates the same chunk. Park the current chunk until validation completes, register validation before disaggregated early-send, fence KV transfer, and drain pending PP-disaggregated aborts before chunk processing. Accuracy: unchanged. Speed: affected chunked requests wait one result-processing turn instead of replaying work. Tests: focused chunk progress, transfer fencing, and abort accounting tests.
Cover scheduler parking, single chunk advancement, queued-owner isolation, disaggregated transfer fencing, bootstrap and yield bookkeeping, early-send ordering, and abort pool accounting. Tests: 24 focused scheduler and embedding-error tests.
|
I fault-injected the short-embedding error after a real Qwen3.5-0.8B hybrid forward on H200 with SGLANG_ENABLE_STRICT_MEM_CHECK_DURING_IDLE=2. The failed image request returned 500, a following image request returned 200, and the idle KV/Mamba invariant stayed healthy; this validates the non-inserting rollback path. Please add one hybrid-pool regression test that exercises real KV and Mamba release rather than mocking release_kv_cache, since the production symptom was exactly one request worth of both resources. |
|
The per-item paths still |
|
Commit |
|
The idle InvariantChecker report is downstream evidence that this exception skipped request teardown: exactly one 16,384-token KV reservation and 3 Mamba states remained allocated. The recovery boundary should therefore finish request-level KV/Mamba rollback before returning the 500; please do not catch or suppress the invariant itself, since at that point global accounting is already inconsistent. |
|
There is still a symmetric fresh-output escape: _split_or_retry_short_combined_embedding() calls torch.split whenever actual_total >= expected_total, so an overlong combined output raises before the request-level error channel and can skip teardown. _get_chunked_embedding_full() also caches any overlong fresh result and downstream _adjust_embedding_length() silently truncates it from the end, which can feed the wrong visual tokens. Please treat every non-EVS actual != expected as a mismatch, retry only when per-item attribution is needed, and route all per-item count/length mismatches (including the remaining len assertions) through the same padded request-local failure path. |
|
The VLM error path still calls |
|
After rebasing, please remove the new reflective fallbacks: |
|
Waiting for #36926 to land, then I’ll rebase once and resolve the merged-main overlap. |
This branch has not been deployed
3 participants
Motivation
Malformed fresh multimodal embeddings can currently terminate the scheduler instead of failing only the affected request. Cache-entry validation in #36595 does not cover fresh encoder output.
This PR contains the failure; it does not diagnose why an encoder produced incorrect embeddings.
Changes
Validation
Regression coverage includes mixed healthy/malformed outputs, runner batch-copy boundaries, bundled and precomputed embeddings, two-process retry/error consensus (Gloo and optional NVIDIA NCCL), real KV/Mamba allocator recovery after repeated failures, chunked abort races, and hidden-state/logprob cursor alignment. Registered tests have executable entry points.
Remaining merge gates:
The PR is non-draft to allow CI to run, not a declaration that these integration gates have passed.
CI States
Latest PR Test (Base): ❌ Run #34298142921
Latest PR Test (Extra): ❌ Run #34298142863
Latest PR Test (AMD ROCm 7.2): ❌ Run #34298143057