diff --git a/docs/LEARNINGS.md b/docs/LEARNINGS.md index c365dbe..c6448f4 100644 --- a/docs/LEARNINGS.md +++ b/docs/LEARNINGS.md @@ -759,3 +759,10 @@ generation streams with an empty `modal.exception.RemoteError`. Fixes: - `_replace_outdated_modal_app` refuses to stop an app whose remote fingerprint this process already deployed or validated, so divergent callers can no longer stop each other's deployments. +- Follow-up: hybrid mapped execution builds its static, mapped, and cleanup phase + payloads in `_shared_subgraph_payload_fields`, which also dropped `modal_gpu` and + `modal_max_containers`. The refusal above caught it, raising + `ModalRemoteAppOutOfDateError` instead of killing jobs. All derived payloads now + copy `settings.MODAL_DEPLOYMENT_PAYLOAD_FIELDS` through + `modal_deployment_payload_fields()`; add any new fingerprint-shaping payload + field there. diff --git a/prompt_affinity_planning.py b/prompt_affinity_planning.py index 14c6996..18133f1 100644 --- a/prompt_affinity_planning.py +++ b/prompt_affinity_planning.py @@ -24,6 +24,7 @@ if __package__: _node_output_refs, ) from .remote_plan_types import InputTarget, LinkedOutputRef + from .settings import MODAL_DEPLOYMENT_PAYLOAD_FIELDS else: # pragma: no cover - flat import inside the Modal container. from component_planning import _component_ancestors_of_local_source from execution_environments import ExecutionProvider @@ -40,6 +41,7 @@ else: # pragma: no cover - flat import inside the Modal container. _node_output_refs, ) from remote_plan_types import InputTarget, LinkedOutputRef + from settings import MODAL_DEPLOYMENT_PAYLOAD_FIELDS logger = logging.getLogger(__name__) @@ -48,8 +50,7 @@ _SPECULATIVE_PREWARM_PAYLOAD_FIELDS = frozenset( { "component_id", "prompt_id", - "modal_gpu", - "modal_max_containers", + *MODAL_DEPLOYMENT_PAYLOAD_FIELDS, "execution_provider", "execution_environment_id", "remote_worker_affinity_group", diff --git a/remote/mapped_execution.py b/remote/mapped_execution.py index a1bfed0..0e06cde 100644 --- a/remote/mapped_execution.py +++ b/remote/mapped_execution.py @@ -43,7 +43,7 @@ from .modal_warmup import ( _prompt_warmup_head_start_seconds, boost_mapped_component_warmup, ) -from ..settings import get_settings +from ..settings import get_settings, modal_deployment_payload_fields logger = logging.getLogger(__name__) @@ -144,6 +144,7 @@ def _shared_subgraph_payload_fields(payload: dict[str, Any]) -> dict[str, Any]: payload.get("terminate_container_on_error", True) ), "custom_nodes_bundle": payload.get("custom_nodes_bundle"), + **modal_deployment_payload_fields(payload), } snapshot_profile_key = payload.get("snapshot_profile_key") if isinstance(snapshot_profile_key, str) and snapshot_profile_key.strip(): diff --git a/remote/modal_warmup.py b/remote/modal_warmup.py index dbab99a..06d1be6 100644 --- a/remote/modal_warmup.py +++ b/remote/modal_warmup.py @@ -16,7 +16,11 @@ import threading import time from typing import Any, Callable, Iterator, Mapping -from ..settings import get_settings, modal_deployment_app_name +from ..settings import ( + get_settings, + modal_deployment_app_name, + modal_deployment_payload_fields, +) from .local_execution import _is_link, _resolve_required_subgraph_nodes from .modal_deployment import ( ModalRemoteInvocationError, @@ -542,8 +546,8 @@ def _build_prompt_warmup_request(payload: dict[str, Any]) -> dict[str, Any] | No payload.get("component_id", "modal-warmup"), ) ), + **modal_deployment_payload_fields(payload), "modal_gpu": settings.modal_gpu, - "modal_max_containers": payload.get("modal_max_containers"), "remote_worker_affinity_group": payload.get( "remote_worker_affinity_group" ), diff --git a/settings.py b/settings.py index 302f8ba..2bb3b37 100644 --- a/settings.py +++ b/settings.py @@ -10,6 +10,7 @@ import re from dataclasses import dataclass, replace from functools import lru_cache from pathlib import Path +from typing import Any, Mapping if __package__: from .instance_identity import ( @@ -250,6 +251,23 @@ def modal_deployment_app_name(settings: ModalSyncSettings) -> str: return f"{prefix}-{digest}{suffix}" +MODAL_DEPLOYMENT_PAYLOAD_FIELDS = ("modal_gpu", "modal_max_containers") +"""Payload fields that select deployment settings and so shape the runtime fingerprint. + +Any payload derived from another (warmups, prewarm targets, mapped phases) must +copy these, or it will expect a different fingerprint for the same Modal app. +""" + + +def modal_deployment_payload_fields(payload: Mapping[str, Any]) -> dict[str, Any]: + """Return the deployment-identity fields present on one payload.""" + return { + field_name: payload[field_name] + for field_name in MODAL_DEPLOYMENT_PAYLOAD_FIELDS + if payload.get(field_name) is not None + } + + def _read_path_env(name: str) -> Path | None: """Resolve an environment variable into a path when present.""" value = os.getenv(name) diff --git a/tests/test_mapped_execution.py b/tests/test_mapped_execution.py index 33edf8b..6c6a05b 100644 --- a/tests/test_mapped_execution.py +++ b/tests/test_mapped_execution.py @@ -2532,3 +2532,60 @@ def test_trim_subgraph_payload_to_required_nodes_drops_unrelated_mapped_branch( assert trimmed_payload["execute_node_ids"] == ["3"] assert trimmed_payload["mapped_execute_node_ids"] == [] assert trimmed_payload["static_execute_node_ids"] == ["3"] + + +def test_mapped_phase_payloads_preserve_parent_runtime_fingerprint( + mapped_execution_module: Any, + modal_deployment_module: Any, + monkeypatch: Any, +) -> None: + """Static, mapped, and cleanup phases must expect the parent's deployed fingerprint.""" + phase_definition = { + "component_node_ids": ["9"], + "subgraph_prompt": {}, + "boundary_inputs": [], + "boundary_outputs": [], + "execute_node_ids": ["9"], + } + parent_payload = { + "payload_kind": "mapped_subgraph", + "component_id": "1", + "modal_gpu": "B300", + "modal_max_containers": 4, + "static_phase": phase_definition, + "mapped_phase": phase_definition, + "remote_session": {"session_id": "session-1"}, + "clear_remote_session": True, + } + derived_payloads = [ + mapped_execution_module._build_phase_subgraph_payload( + parent_payload, "static_phase", "1" + ), + mapped_execution_module._build_phase_subgraph_payload( + parent_payload, "mapped_phase", "1__mapped" + ), + ] + monkeypatch.setattr( + mapped_execution_module, + "is_remote_session_handle_payload", + lambda remote_session: True, + ) + derived_payloads.append( + mapped_execution_module._build_remote_session_cleanup_payload(parent_payload) + ) + parent_fingerprint = modal_deployment_module._expected_remote_runtime_fingerprint( + parent_payload + ) + + assert parent_fingerprint != ( + modal_deployment_module._expected_remote_runtime_fingerprint( + {"modal_gpu": "B300"} + ) + ) + for derived_payload in derived_payloads: + assert derived_payload["modal_gpu"] == "B300" + assert derived_payload["modal_max_containers"] == 4 + assert ( + modal_deployment_module._expected_remote_runtime_fingerprint(derived_payload) + == parent_fingerprint + )