-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix(health): skip canary health check for trtllm/vllm disagg decode workers #8215
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
Closed
nnshah1
wants to merge
13
commits into
main
from
neelays/dis-1737-bug-canary-health-check-incompatible-with-disagg-decode
Closed
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
3ea94d7
fix(trtllm): skip canary health check for disagg decode workers
nnshah1 4edc2bb
fix(vllm): skip canary health check for multimodal disagg decode workers
nnshah1 774f594
fix: correct ModelType check and add copyright header
nnshah1 190ce67
style: black formatting for worker_factory.py
nnshah1 abe3eed
fix(trtllm): add pytest markers to health check test
nnshah1 73c3c57
fix(health): per-backend disagg decode canary + unstick no-target end…
nnshah1 1d342b4
test(fault_tolerance): rank-pause canary detection harness
nnshah1 26fd9bd
style(fault_tolerance): black format test_canary_rank_pause
nnshah1 1c2c3fc
fix(dis-1737 tests): add missing Lifecycle/Hardware markers + remove …
nnshah1 54f8ca2
feat(trtllm): canary probe short-circuit for disagg decode
nnshah1 c751020
test(fault_tolerance): extend rank-pause matrix to all three backends
nnshah1 1a9b89c
style: black 23.1.0 format (matches pre-commit hook)
nnshah1 98452df
fix(rank-pause): match mpi4py.futures.server for trtllm rank discovery
nnshah1 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
67 changes: 67 additions & 0 deletions
67
components/src/dynamo/trtllm/tests/test_health_check_disagg.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| # SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| import pytest | ||
|
|
||
| from dynamo.trtllm.constants import DisaggregationMode | ||
| from dynamo.trtllm.health_check import ( | ||
| CANARY_PROBE_KEY, | ||
| TrtllmDisaggDecodeHealthCheckPayload, | ||
| TrtllmHealthCheckPayload, | ||
| build_worker_health_check_payload, | ||
| ) | ||
|
|
||
| pytestmark = [ | ||
| pytest.mark.unit, | ||
| pytest.mark.trtllm, | ||
| pytest.mark.gpu_1, # needs trtllm packages installed but does not use GPU | ||
| pytest.mark.profiled_vram_gib(0), | ||
| pytest.mark.pre_merge, | ||
| ] | ||
|
|
||
|
|
||
| def test_trtllm_health_check_payload_has_no_disagg_params(): | ||
| """Standard TrtllmHealthCheckPayload should NOT include disaggregated_params.""" | ||
| payload = TrtllmHealthCheckPayload().to_dict() | ||
| assert "disaggregated_params" not in payload | ||
| assert "prefill_result" not in payload | ||
| assert "token_ids" in payload | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "mode", | ||
| [DisaggregationMode.AGGREGATED, DisaggregationMode.PREFILL], | ||
| ) | ||
| def test_non_decode_modes_register_canary_payload(mode): | ||
| """Aggregated and prefill workers register the standard canary payload.""" | ||
| payload = build_worker_health_check_payload(disaggregation_mode=mode) | ||
| assert payload is not None | ||
| assert "token_ids" in payload | ||
| assert "sampling_options" in payload | ||
|
|
||
|
|
||
| def test_decode_mode_registers_probe_payload(): | ||
| """Decode workers register a probe payload carrying CANARY_PROBE_KEY. | ||
|
|
||
| The handler detects the marker in `_setup_disaggregated_params_for_mode` | ||
| and routes the probe through `request_type="context_and_generation"` | ||
| so the engine runs it as a local agg request (no cache transceiver). | ||
| Pattern mirrors SGLang's FAKE_BOOTSTRAP_HOST. | ||
| """ | ||
| payload = build_worker_health_check_payload( | ||
| disaggregation_mode=DisaggregationMode.DECODE | ||
| ) | ||
| assert payload is not None | ||
| assert payload.get(CANARY_PROBE_KEY) is True | ||
| # Standard fields must still be present so the handler's downstream logic | ||
| # (sampling, stop conditions, token_ids) runs normally. | ||
| assert "token_ids" in payload | ||
| assert "sampling_options" in payload | ||
| assert "stop_conditions" in payload | ||
|
|
||
|
|
||
| def test_disagg_decode_payload_class_sets_probe_marker(): | ||
| """Direct construction of TrtllmDisaggDecodeHealthCheckPayload carries the marker.""" | ||
| payload = TrtllmDisaggDecodeHealthCheckPayload().to_dict() | ||
| assert payload.get(CANARY_PROBE_KEY) is True | ||
| assert "disaggregated_params" not in payload # real params built by handler | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
38 changes: 38 additions & 0 deletions
38
components/src/dynamo/vllm/tests/test_vllm_health_check.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| # SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| """Regression guard: vLLM workers must always register a canary payload. | ||
|
|
||
| DIS-1737 originally routed disagg decode through a `payload = None` branch in | ||
| `worker_factory.py`. That silently opted decode workers out of canary, which | ||
| after DIS-1185 (canary = sole readiness authority) left them stuck NotReady. | ||
| These tests ensure the payload constructor works for both agg and decode paths | ||
| so no one re-introduces a DECODE-specific None branch. | ||
| """ | ||
|
|
||
| import pytest | ||
|
|
||
| from dynamo.vllm.health_check import VllmHealthCheckPayload | ||
|
|
||
| pytestmark = [ | ||
| pytest.mark.unit, | ||
| pytest.mark.vllm, | ||
| pytest.mark.pre_merge, | ||
| pytest.mark.gpu_0, | ||
| ] | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("use_text_input", [False, True]) | ||
| def test_vllm_health_check_payload_is_non_none(use_text_input): | ||
| """VllmHealthCheckPayload.to_dict() returns a non-None dict regardless of | ||
| tokenizer mode. Worker code must always register a canary target; decode | ||
| workers rely on the vLLM handler's natural agg-style fallback when | ||
| `prefill_result` is absent (handlers.py::_generate_token_mode).""" | ||
| payload = VllmHealthCheckPayload( | ||
| engine_client=None, use_text_input=use_text_input | ||
| ).to_dict() | ||
|
|
||
| assert payload is not None | ||
| assert isinstance(payload, dict) | ||
| # Payload must contain some form of input the engine can decode. | ||
| assert "token_ids" in payload or "prompt" in payload |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.