Repository navigation
[Core] Add request-scoped producer state and fail processor exceptions #8530
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| # Request-scoped producer state | ||
|
|
||
| Custom incremental producers invoked by `OmniChunkTransferAdapter` can store | ||
| typed state in a namespace under the adapter's existing `request_payload` | ||
| container. This is a core implementation candidate for RFC #6453; model | ||
| migrations and model-specific segment resets are separate work. | ||
|
|
||
| ```python | ||
| from vllm_omni.distributed.omni_connectors.transfer_adapter.request_state import ( | ||
| StageRequestIdentity, | ||
| ) | ||
|
|
||
| identity = StageRequestIdentity(request.request_id, request.external_req_id) | ||
| state = transfer_manager.request_state.get_or_create( | ||
| identity, namespace="producer.stage_edge", factory=ModelState | ||
| ) | ||
| ``` | ||
|
|
||
| The accessor is available during a queued custom producer invocation. Both IDs | ||
| must come from the supplied request. A namespace returns the same state until | ||
| whole-request release. Factories construct model state without connector I/O; | ||
| they run outside the sender lock and may be called concurrently, so they must | ||
| not perform external side effects. | ||
|
|
||
| The runtime owns the request container. Producers may update their namespace | ||
| but must not remove the container. Existing processors can continue using raw | ||
| payload entries, including tensor-valued entries. A request cannot mix legacy | ||
| raw entries and namespaced state: access fails instead of overwriting model | ||
| data. | ||
|
|
||
| | Outcome | State lifetime | | ||
| | --- | --- | | ||
| | Producer buffers data and returns no payload | Retained | | ||
| | Nonterminal send succeeds | Retained | | ||
| | Resumable segment completes | Retained | | ||
| | Whole-request terminal send succeeds | Released by canonical cleanup | | ||
| | Processor raises, or connector fails/raises | Retained until failure finalization | | ||
| | Cancellation or timeout finalizes the request | Released by canonical cleanup | | ||
| | Duplicate cleanup | Harmless | | ||
| | External ID is reused after release | Fresh namespaces | | ||
|
|
||
| Generation validation uses the existing sender token and sender lock. A | ||
| retained accessor cannot recreate state after cancellation or release, even | ||
| when a new request reuses the external ID. Factories that overlap cleanup also | ||
| fail before committing their result. This does not make a retained model state | ||
| object immutable: callers must not mutate it after their invocation finishes. | ||
|
|
||
| Processor exceptions are recorded against the scheduler's internal request ID. | ||
| They do not produce a successful empty terminal payload. Existing scheduler | ||
| failure finalization releases receiver and sender state; this API adds no retry | ||
| policy, timeout mechanism, or process-global request map. | ||
|
|
||
| The CPU contract tests include a synthetic producer through the production | ||
| save queue, sender, connector outcomes, failure collection, and finalization. | ||
| They do not establish model correctness or a latency improvement. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
| # SPDX-FileCopyrightText: Copyright contributors to the vLLM-Omni project | ||
|
|
||
| import threading | ||
| from dataclasses import dataclass | ||
|
|
||
| import pytest | ||
|
|
||
| from vllm_omni.distributed.omni_connectors.transfer_adapter.request_state import ( | ||
| RequestStateAccessor, | ||
| RequestStateUnavailableError, | ||
| StageRequestIdentity, | ||
| ) | ||
|
|
||
| pytestmark = [pytest.mark.core_model, pytest.mark.cpu] | ||
|
|
||
|
|
||
| @dataclass | ||
| class ProducerState: | ||
| chunks: int = 0 | ||
|
|
||
|
|
||
| def make_view(payloads, owners, identity, lock=None): | ||
| owner = owners[identity.external_request_id] | ||
| return RequestStateAccessor( | ||
| payloads, | ||
| identity, | ||
| owner, | ||
| lock or threading.Lock(), | ||
| lambda: owners.get(identity.external_request_id) is owner, | ||
| ) | ||
|
|
||
|
|
||
| def test_namespaces_and_interleaved_requests_are_isolated(): | ||
| payloads: dict[str, object] = {} | ||
| owners = {"a": object(), "b": object()} | ||
| a = StageRequestIdentity("internal-a", "a") | ||
| b = StageRequestIdentity("internal-b", "b") | ||
| first = make_view(payloads, owners, a) | ||
| second = make_view(payloads, owners, b) | ||
| state = first.get_or_create(a, "producer.codec", ProducerState) | ||
| state.chunks = 3 | ||
| assert first.get_or_create(a, "producer.codec", ProducerState) is state | ||
| assert first.get_or_create(a, "producer.prompt", ProducerState).chunks == 0 | ||
| assert second.get_or_create(b, "producer.codec", ProducerState).chunks == 0 | ||
|
|
||
|
|
||
| def test_release_and_id_reuse_fence_retained_accessors(): | ||
| payloads: dict[str, object] = {} | ||
| owners = {"external": object()} | ||
| old_id = StageRequestIdentity("old-internal", "external") | ||
| old = make_view(payloads, owners, old_id) | ||
| old.get_or_create(old_id, "codec", ProducerState).chunks = 4 | ||
| payloads.pop("external", None) | ||
| owners.pop("external", None) | ||
| with pytest.raises(RequestStateUnavailableError): | ||
| old.get_or_create(old_id, "late", ProducerState) | ||
| owners["external"] = object() | ||
| new_id = StageRequestIdentity("new-internal", "external") | ||
| new = make_view(payloads, owners, new_id) | ||
| assert new.get_or_create(new_id, "codec", ProducerState).chunks == 0 | ||
| with pytest.raises(RequestStateUnavailableError): | ||
| old.get_or_create(old_id, "codec", ProducerState) | ||
| with pytest.raises(RequestStateUnavailableError): | ||
| new.get_or_create(old_id, "codec", ProducerState) | ||
|
|
||
|
|
||
| def test_cleanup_during_factory_cannot_resurrect_state(): | ||
| payloads: dict[str, object] = {} | ||
| owners = {"external": object()} | ||
| identity = StageRequestIdentity("internal", "external") | ||
| lock = threading.Lock() | ||
| view = make_view(payloads, owners, identity, lock) | ||
|
|
||
| def factory(): | ||
| # This would deadlock if the user factory held the sender lock. | ||
| with lock: | ||
| owners.pop("external") | ||
| return ProducerState() | ||
|
|
||
| with pytest.raises(RequestStateUnavailableError): | ||
| view.get_or_create(identity, "codec", factory) | ||
| assert payloads == {} | ||
|
|
||
|
|
||
| def test_factory_failure_leaves_no_partial_container(): | ||
| payloads: dict[str, object] = {} | ||
| owners = {"external": object()} | ||
| identity = StageRequestIdentity("internal", "external") | ||
| view = make_view(payloads, owners, identity) | ||
|
|
||
| def factory(): | ||
| raise ValueError("invalid conditioning") | ||
|
|
||
| with pytest.raises(ValueError, match="invalid conditioning"): | ||
| view.get_or_create(identity, "codec", factory) | ||
| assert payloads == {} | ||
| assert view.get_or_create(identity, "codec", ProducerState).chunks == 0 | ||
|
|
||
|
|
||
| def test_legacy_state_is_not_overwritten(): | ||
| legacy = {"frames": [1, 2]} | ||
| payloads = {"external": legacy} | ||
| owners = {"external": object()} | ||
| identity = StageRequestIdentity("internal", "external") | ||
| with pytest.raises(RequestStateUnavailableError, match="legacy"): | ||
| make_view(payloads, owners, identity).get_or_create(identity, "codec", ProducerState) | ||
| assert payloads["external"] is legacy | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("namespace", ["", " "]) | ||
| def test_empty_namespace_is_rejected(namespace): | ||
| identity = StageRequestIdentity("internal", "external") | ||
| with pytest.raises(ValueError, match="namespace"): | ||
| make_view({}, {"external": object()}, identity).get_or_create(identity, namespace, ProducerState) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,6 +25,7 @@ | |
| from ..utils.kv_utils import get_local_tp_rank, get_omni_replica_id | ||
| from ..utils.logging import get_connector_logger | ||
| from .base import OmniTransferAdapterBase | ||
| from .request_state import RequestStateAccessor, RequestStateUnavailableError, StageRequestIdentity | ||
|
|
||
| logger = get_connector_logger(__name__) | ||
|
|
||
|
|
@@ -153,6 +154,7 @@ def __init__(self, vllm_config: Any): | |
| # segment's queued or in-flight work after the same request id resumes. | ||
| self._registered_load_entries: dict[str, _LoadEntry] = {} | ||
| self._sender_tokens: dict[str, _SenderGeneration] = {} | ||
| self._processor_state_context = threading.local() | ||
| self.scheduler_max_num_seqs = vllm_config.scheduler_config.max_num_seqs | ||
| ( | ||
| self._max_model_len, | ||
|
|
@@ -195,7 +197,7 @@ def __init__(self, vllm_config: Any): | |
| self._adaptive_states: dict[str, Any] = {} | ||
| self.upstream_exhausted_requests: set[str] = set() | ||
| self.segment_finished_requests: set[str] = set() | ||
| self.request_payload = {} | ||
| self.request_payload: dict[str, Any] = {} | ||
| self.code_prompt_token_ids: dict[str, list[torch.Tensor]] = defaultdict(list) | ||
| self.request_ids_mapping: dict[str, str] = {} | ||
| # Save-thread only: running count of segments reclaimed after finish. | ||
|
|
@@ -235,6 +237,14 @@ def __init__(self, vllm_config: Any): | |
| # OmniSchedulingCoordinator._waiting_since on the full-payload path. | ||
| self._waiting_since: dict[str, float] = {} | ||
|
|
||
| @property | ||
| def request_state(self) -> RequestStateAccessor: | ||
| """Namespaced state for the current custom producer invocation.""" | ||
| accessor = getattr(self._processor_state_context, "accessor", None) | ||
| if accessor is None: | ||
| raise RequestStateUnavailableError("Request state is only available inside a queued producer invocation") | ||
| return accessor | ||
|
|
||
| @staticmethod | ||
| def _is_truthy_scalar(value: Any) -> bool: | ||
| if isinstance(value, torch.Tensor): | ||
|
|
@@ -803,10 +813,22 @@ def _send_single_request_for_generation( | |
| } | ||
| if self._accepts_new_token_ids(processor): | ||
| processor_kwargs["new_token_ids"] = task.get("new_token_ids", ()) | ||
| if sender_token is not None: | ||
| self._processor_state_context.accessor = RequestStateAccessor( | ||
| self.request_payload, | ||
| StageRequestIdentity(request.request_id, external_req_id), | ||
| sender_token, | ||
| self._sender_state_lock, | ||
| lambda: self._sender_tokens.get(external_req_id) is sender_token and not sender_token.cancelled, | ||
| ) | ||
| payload_data = processor(**processor_kwargs) | ||
|
|
||
| except Exception as e: | ||
| logger.error(f"Failed to use custom_process_input_func for payload extraction: {e}") | ||
| self.record_send_failure(request.request_id, f"Stage input processor failed: {e}") | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P1] This hunk adds Evidence and suggested fixThis hunk adds Evidence: chunk_transfer_adapter.py:828-829 Suggestion: self.record_send_failure(request.request_id, f"Stage input processor failed: {e}") |
||
| return | ||
| finally: | ||
| self._processor_state_context.accessor = None | ||
|
|
||
| if payload_data is None: | ||
| if not (is_segment_finished or is_finished): | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P2] This diff's `test_processor_exception_fails_request_without_empty_terminal_send…
Evidence and suggested fix
This diff's
test_processor_exception_fails_request_without_empty_terminal_sendends withwith pytest.raises(RequestStateUnavailableError): _ = adapter.request_state. The newrequest_stateproperty raises whenever_processor_state_context.accessoris None, and_send_single_request'sfinallyalways clears it on producer exit, so that assertion cannot fail even whilerequest_payloadstill holds namespaces (the test already asserts"external" in adapter.request_payloadbefore a manualfinish_requests). Capture the accessor inside the producer and assertget_or_createon that retained view, astest_namespaced_producer_state_terminal_commitalready does.Evidence: chunk_transfer_adapter.py:830-831: finally: self._processor_state_context.accessor = None. chunk_transfer_adapter.py:243-245: if accessor is None: raise RequestStateUnavailableError(...). test_chunk_transfer_adapter.py:64-66: view, identity = views[0] / view.get_or_create(identity, "late", list).