-
Notifications
You must be signed in to change notification settings - Fork 7.9k
[core] [1/2] Topology aware scheduling public API #63479
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
Changes from 5 commits
048f2ae
b39025b
1b96c6f
141bdaf
9e7d9af
ba7f43f
b0e8f85
5b8979f
972ca0e
88364be
734fcbf
6d3a742
867b20b
016361c
7c6a2b9
17f8a35
627688d
0db8536
e5afe1f
27cba4f
e49ae4f
dc20d13
1f675bb
e60213e
dda9bab
e8c530d
093469f
2aeea5e
fb280df
7a52b43
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 |
|---|---|---|
|
|
@@ -723,6 +723,17 @@ cdef int prepare_actor_concurrency_groups( | |
| return 1 | ||
|
|
||
|
|
||
| cdef CPlacementStrategy prepare_c_strategy(c_string strategy) except *: | ||
| if strategy == b"PACK": | ||
| return PLACEMENT_STRATEGY_PACK | ||
| elif strategy == b"SPREAD": | ||
| return PLACEMENT_STRATEGY_SPREAD | ||
| elif strategy == b"STRICT_PACK": | ||
| return PLACEMENT_STRATEGY_STRICT_PACK | ||
| else: | ||
| return PLACEMENT_STRATEGY_STRICT_SPREAD | ||
|
|
||
|
|
||
| def raise_sys_exit_with_custom_error_message( | ||
| ray_terminate_msg: str, | ||
| exit_code: int = 0) -> None: | ||
|
|
@@ -3693,23 +3704,22 @@ cdef class CoreWorker: | |
| c_string strategy, | ||
| c_bool is_detached, | ||
| soft_target_node_id, | ||
| c_vector[unordered_map[c_string, c_string]] bundle_label_selector): | ||
| c_vector[unordered_map[c_string, c_string]] bundle_label_selector, | ||
| topology_strategy): | ||
|
Contributor
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. Better to make the
Contributor
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. Done. |
||
| cdef: | ||
| CPlacementGroupID c_placement_group_id | ||
| CPlacementStrategy c_strategy | ||
| CNodeID c_soft_target_node_id = CNodeID.Nil() | ||
| c_vector[unordered_map[c_string, CPlacementStrategy]] c_topology_strategy | ||
| unordered_map[c_string, CPlacementStrategy] c_level | ||
|
|
||
| if strategy == b"PACK": | ||
| c_strategy = PLACEMENT_STRATEGY_PACK | ||
| elif strategy == b"SPREAD": | ||
| c_strategy = PLACEMENT_STRATEGY_SPREAD | ||
| elif strategy == b"STRICT_PACK": | ||
| c_strategy = PLACEMENT_STRATEGY_STRICT_PACK | ||
| else: | ||
| if strategy == b"STRICT_SPREAD": | ||
| c_strategy = PLACEMENT_STRATEGY_STRICT_SPREAD | ||
| else: | ||
| raise TypeError(strategy) | ||
| c_strategy = prepare_c_strategy(strategy) | ||
|
|
||
| for level in topology_strategy: | ||
| c_level.clear() | ||
| for label, level_strategy in level.items(): | ||
| c_level[label] = prepare_c_strategy(level_strategy) | ||
| c_topology_strategy.push_back(c_level) | ||
|
|
||
| if soft_target_node_id is not None: | ||
| c_soft_target_node_id = CNodeID.FromHex(soft_target_node_id) | ||
|
|
@@ -3724,7 +3734,8 @@ cdef class CoreWorker: | |
| bundles, | ||
| is_detached, | ||
| c_soft_target_node_id, | ||
| bundle_label_selector), | ||
| bundle_label_selector, | ||
| c_topology_strategy), | ||
| &c_placement_group_id)) | ||
|
|
||
| return PlacementGroupID(c_placement_group_id.Binary()) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,119 @@ | ||
| import os | ||
| import sys | ||
|
|
||
| import pytest | ||
|
|
||
| import ray | ||
| from ray.util.placement_group import placement_group, placement_group_table | ||
|
|
||
| NODE_ID_LABEL = "ray.io/node-id" | ||
| RACK_LABEL = "rack_id" | ||
|
Contributor
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. nit: I'd rather keep the rack label examples as "ray.io/gpu-domain" to be consistent with prior docs
Contributor
Author
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. done |
||
| ONE = "1" | ||
| TWO = "2" | ||
| rack1_labels = {RACK_LABEL: ONE} | ||
| rack2_labels = {RACK_LABEL: TWO} | ||
|
|
||
|
|
||
| def test_topology_strategy_feasible_after_rack_kill(ray_start_cluster): | ||
| """Verify topology-aware rescheduling after total rack failure. | ||
|
|
||
| Creates a PG on rack 1 (the only available rack at the time). After | ||
| removing one rack 1 node, the PG enters RESCHEDULING but stays pinned to | ||
| rack 1, so it remains infeasible even though rack 2 has capacity. Once | ||
| all rack 1 nodes are removed (total failure), the topology assignment | ||
| is cleared and the PG reschedules onto rack 2. | ||
| """ | ||
| cluster = ray_start_cluster | ||
| cluster.add_node(num_cpus=0) | ||
| ray.init(address=cluster.address) | ||
|
|
||
| rack1_nodes = [cluster.add_node(num_cpus=1, labels=rack1_labels) for _ in range(4)] | ||
|
|
||
| def assert_pg_nodes_label_value(cluster_nodes, pg, label, value): | ||
| node_id_to_labels = {node["NodeID"]: node["Labels"] for node in cluster_nodes} | ||
| for node_id in placement_group_table(pg)["bundles_to_node_id"].values(): | ||
| assert node_id_to_labels[node_id].get(label) == value | ||
|
|
||
| bundles = [{"CPU": 1}] * 4 | ||
|
|
||
| pg = placement_group( | ||
| bundles=bundles, | ||
| topology_strategy=[{RACK_LABEL: "STRICT_PACK"}], | ||
| ) | ||
| ray.get(pg.ready(), timeout=30) | ||
| assert placement_group_table(pg)["state"] == "CREATED" | ||
| assert_pg_nodes_label_value(ray.nodes(), pg, RACK_LABEL, ONE) | ||
|
|
||
| # Bring rack 2 online; PG should still be pinned to rack 1. | ||
| for _ in range(4): | ||
| cluster.add_node(num_cpus=1, labels=rack2_labels) | ||
|
|
||
| # Drop one rack 1 node -> partial failure -> RESCHEDULING + infeasible. | ||
| cluster.remove_node(rack1_nodes[0]) | ||
| with pytest.raises(ray.exceptions.GetTimeoutError): | ||
| ray.get(pg.ready(), timeout=5) | ||
| assert placement_group_table(pg)["state"] == "RESCHEDULING" | ||
|
|
||
| # Total rack 1 failure -> clears assignment -> reschedules onto rack 2. | ||
| for node in rack1_nodes[1:]: | ||
| cluster.remove_node(node) | ||
|
|
||
| ray.get(pg.ready(), timeout=30) | ||
| assert placement_group_table(pg)["state"] == "CREATED" | ||
| assert_pg_nodes_label_value(ray.nodes(), pg, RACK_LABEL, TWO) | ||
|
|
||
|
|
||
| def test_topology_strategy_strict_pack(ray_start_cluster): | ||
| """Testing STRICT_PACK on the node level and STRICT_PACK on the rack level""" | ||
| cluster = ray_start_cluster | ||
| cluster.add_node(num_cpus=0) | ||
| ray.init(address=cluster.address) | ||
|
|
||
| cluster.add_node(num_cpus=4, labels=rack1_labels) | ||
|
|
||
| def assert_pg_nodes_label_value(cluster_nodes, pg, label, value): | ||
|
Contributor
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. this is only validating the rack level right, can we do validation for the node level as well?
Contributor
Author
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. done |
||
| node_id_to_labels = {node["NodeID"]: node["Labels"] for node in cluster_nodes} | ||
| for node_id in placement_group_table(pg)["bundles_to_node_id"].values(): | ||
| assert node_id_to_labels[node_id].get(label) == value | ||
|
|
||
| bundles = [{"CPU": 1}] * 4 | ||
|
|
||
| pg = placement_group( | ||
| bundles=bundles, | ||
| topology_strategy=[{NODE_ID_LABEL: "STRICT_PACK", RACK_LABEL: "STRICT_PACK"}], | ||
| ) | ||
| ray.get(pg.ready(), timeout=30) | ||
| assert placement_group_table(pg)["state"] == "CREATED" | ||
| assert_pg_nodes_label_value(ray.nodes(), pg, RACK_LABEL, ONE) | ||
|
|
||
|
|
||
| def test_topology_strategy_strict_spread(ray_start_cluster): | ||
| """Testing STRICT_SPREAD on the node level and STRICT_PACK on the rack level""" | ||
|
Contributor
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. same here, let's validate that STRICT_SPREAD actually works
Contributor
Author
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. done |
||
| cluster = ray_start_cluster | ||
| cluster.add_node(num_cpus=0) | ||
| ray.init(address=cluster.address) | ||
|
|
||
| for _ in range(4): | ||
| cluster.add_node(num_cpus=1, labels=rack1_labels) | ||
|
Contributor
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. each node should have more than one cpu. Otherwise the behavior of STRICT_SPREAD vs PACK (the default behavior) is equivalent
Contributor
Author
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. good catch |
||
|
|
||
| def assert_pg_nodes_label_value(cluster_nodes, pg, label, value): | ||
| node_id_to_labels = {node["NodeID"]: node["Labels"] for node in cluster_nodes} | ||
| for node_id in placement_group_table(pg)["bundles_to_node_id"].values(): | ||
| assert node_id_to_labels[node_id].get(label) == value | ||
|
|
||
| bundles = [{"CPU": 1}] * 4 | ||
|
|
||
| pg = placement_group( | ||
| bundles=bundles, | ||
| topology_strategy=[{NODE_ID_LABEL: "STRICT_SPREAD", RACK_LABEL: "STRICT_PACK"}], | ||
| ) | ||
| ray.get(pg.ready(), timeout=30) | ||
| assert placement_group_table(pg)["state"] == "CREATED" | ||
| assert_pg_nodes_label_value(ray.nodes(), pg, RACK_LABEL, ONE) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| if os.environ.get("PARALLEL_CI"): | ||
| sys.exit(pytest.main(["-n", "auto", "--boxed", "-vs", __file__])) | ||
| else: | ||
| sys.exit(pytest.main(["-sv", __file__])) | ||
Uh oh!
There was an error while loading. Please reload this page.