test(router): add GMS placement E2E coverage#10484
Conversation
17939aa to
0878c27
Compare
00c46ef to
fef342b
Compare
2d02bbf to
5b40a9c
Compare
|
|
||
| from tests.gpu_memory_service.common.gms import GMSServer | ||
|
|
||
| devices = os.environ.get("DYNAMO_ROUTER_E2E_GMS_DEVICES", "0,1") |
There was a problem hiding this comment.
When GMS is enabled, the harness starts servers on devices 0,1 by default even for gpu_1/single-GPU tests, so those tests can fail by trying to open GPU 1 on a one-GPU runner. Fix: default the GMS device list to the resolved single test GPU unless DYNAMO_ROUTER_E2E_GMS_DEVICES is explicitly set.
🤖 AI Fix
In tests/router/e2e_harness.py, update maybe_router_gms_servers so the unset DYNAMO_ROUTER_E2E_GMS_DEVICES default is str(int(os.environ.get("DYNAMO_ROUTER_E2E_GPU_START_INDEX", "0"))) instead of "0,1", leaving explicit multi-device coverage controlled by DYNAMO_ROUTER_E2E_GMS_DEVICES.
| ), | ||
| "context_length": _env_int("DYNAMO_ROUTER_E2E_SGLANG_CONTEXT_LENGTH", 1024), | ||
| "disable_cuda_graph": _env_bool( | ||
| "DYNAMO_ROUTER_E2E_SGLANG_DISABLE_CUDA_GRAPH", False |
There was a problem hiding this comment.
The SGLang default now leaves CUDA and piecewise CUDA graphs enabled, but the same-GPU two-worker tests rely on disabling them to avoid the documented capture-time VRAM spike. Fix: keep the default disabled and allow opt-in via the env override.
🤖 AI Fix
In tests/router/test_router_e2e_with_sglang.py, change the SGLANG_ARGS["disable_cuda_graph"] default passed to _env_bool("DYNAMO_ROUTER_E2E_SGLANG_DISABLE_CUDA_GRAPH", ...) from False to True.
fef342b to
46b3e7e
Compare
I think you accidentally included all of the stacked PRs. |
grahamking
left a comment
There was a problem hiding this comment.
Include everything, I think this is a mistake needs rebase maybe. 60k lines.
5b40a9c to
8bc2ae7
Compare
8bc2ae7
into
review/gms-v2-latehost-09-3-router-event-plane-gms-d2d-worker-publish
46b3e7e to
1df43fa
Compare
|
Historical review record preserved. GitHub cannot retarget this merged stacked PR; its current replacement is #11053 (position 10/18). No historical comments or reviews are being deleted. |
Stacked PR in the GMS-managed KV review train.
Base:
review/gms-v2-latehost-09-3-router-event-plane-gms-d2d-worker-publishHead:
review/gms-v2-latehost-09-4-router-event-plane-gms-d2d-e2eRefs #10457 #10458 #10450
This PR is intentionally scoped to one commit in the train so reviewers can inspect the GMS KV ownership, engine integration, Bulwark, host-tier, router, and docs work incrementally.