[Serve] Reject dedicated_cpu option (ray-project#39955)

Serve offers the option to set the Serve controller's `num_cpus` to 1 by setting `disable_cpu` to `True`. However, the Serve controller should always set `num_cpus` to 0, so it doesn't consume any logical CPU resources. This change removes all the internal logic handling `disable_cpu` when it's `True`, and it raises and error if the user tries to set it to anything other than `True`. Co-authored-by: Edward Oakes <[email protected]>
Mark2000 · Sep 29, 2023 · d13c530 · d13c530
1 parent c1ec1cc
commit d13c530
Show file tree

Hide file tree

Showing 10 changed files with 14 additions and 48 deletions.
diff --git a/java/serve/src/main/java/io/ray/serve/api/Serve.java b/java/serve/src/main/java/io/ray/serve/api/Serve.java
@@ -39,13 +39,10 @@ public class Serve {
   /**
    * Initialize a serve instance.
    *
-   * @param dedicatedCpu Whether to reserve a CPU core for the internal Serve controller actor.
-   *     Defaults to False.
    * @param config Configuration options for Serve.
    * @return
    */
-  public static synchronized ServeControllerClient start(
-      boolean dedicatedCpu, Map<String, String> config) {
+  public static synchronized ServeControllerClient start(Map<String, String> config) {
     // Initialize ray if needed.
     if (!Ray.isInitialized()) {
       System.setProperty("ray.job.namespace", Constants.SERVE_NAMESPACE);
@@ -69,7 +66,6 @@ public static synchronized ServeControllerClient start(
         Ray.actor(
                 PyActorClass.of("ray.serve._private.controller", "ServeControllerAvatar"),
                 Constants.SERVE_CONTROLLER_NAME,
-                dedicatedCpu,
                 httpPort)
             .setName(Constants.SERVE_CONTROLLER_NAME + "_AVATAR")
             .setLifetime(ActorLifetime.DETACHED)

diff --git a/java/serve/src/test/java/io/ray/serve/BaseServeTest.java b/java/serve/src/test/java/io/ray/serve/BaseServeTest.java
@@ -22,7 +22,7 @@ public static void startServe() {
     Map<String, String> config = Maps.newHashMap();
     // The default port 8000 is occupied by other processes on the ci platform.
     config.put(RayServeConfig.PROXY_HTTP_PORT, "8341"); // TODO(liuyang-my) Get an available port.
-    client = Serve.start(false, config);
+    client = Serve.start(config);
   }
 
   @AfterMethod(alwaysRun = true)

diff --git a/java/serve/src/test/java/io/ray/serve/api/ServeTest.java b/java/serve/src/test/java/io/ray/serve/api/ServeTest.java
@@ -55,7 +55,7 @@ public void startTest() {
       // The default port 8000 is occupied by other processes on the ci platform.
       Map<String, String> config = Maps.newHashMap();
       config.put(RayServeConfig.PROXY_HTTP_PORT, "8341");
-      Serve.start(false, config);
+      Serve.start(config);
 
       Optional<PyActorHandle> controller = Ray.getActor(Constants.SERVE_CONTROLLER_NAME);
       Assert.assertTrue(controller.isPresent());

diff --git a/java/serve/src/test/java/io/ray/serve/docdemo/HttpStrategyCalcOnRayServe.java b/java/serve/src/test/java/io/ray/serve/docdemo/HttpStrategyCalcOnRayServe.java
@@ -19,7 +19,7 @@
 public class HttpStrategyCalcOnRayServe {
 
   public void deploy() {
-    Serve.start(false, null);
+    Serve.start(null);
 
     Deployment deployment =
         Serve.deployment()

diff --git a/java/serve/src/test/java/io/ray/serve/docdemo/ManagePythonDeployment.java b/java/serve/src/test/java/io/ray/serve/docdemo/ManagePythonDeployment.java
@@ -14,7 +14,7 @@ public static void main(String[] args) {
         "ray.job.code-search-path",
         System.getProperty("java.class.path") + File.pathSeparator + "/path/to/code/");
 
-    Serve.start(false, null);
+    Serve.start(null);
 
     Deployment deployment =
         Serve.deployment()

diff --git a/java/serve/src/test/java/io/ray/serve/docdemo/StrategyCalcOnRayServe.java b/java/serve/src/test/java/io/ray/serve/docdemo/StrategyCalcOnRayServe.java
@@ -14,7 +14,7 @@ public class StrategyCalcOnRayServe {
 
   // docs-deploy-start
   public void deploy() {
-    Serve.start(false, null);
+    Serve.start(null);
 
     Deployment deployment =
         Serve.deployment()

diff --git a/python/ray/serve/_private/api.py b/python/ray/serve/_private/api.py
@@ -110,7 +110,6 @@ def _check_http_options(
 
 def _start_controller(
     http_options: Union[None, dict, HTTPOptions] = None,
-    dedicated_cpu: bool = False,
     grpc_options: Union[None, dict, gRPCOptions] = None,
     **kwargs,
 ) -> Tuple[ActorHandle, str]:
@@ -130,7 +129,7 @@ def _start_controller(
         ray.init(namespace=SERVE_NAMESPACE)
 
     controller_actor_options = {
-        "num_cpus": 1 if dedicated_cpu else 0,
+        "num_cpus": 0,
         "name": SERVE_CONTROLLER_NAME,
         "lifetime": "detached",
         "max_restarts": -1,
@@ -179,7 +178,6 @@ def _start_controller(
 
 async def serve_start_async(
     http_options: Union[None, dict, HTTPOptions] = None,
-    dedicated_cpu: bool = False,
     grpc_options: Union[None, dict, gRPCOptions] = None,
     **kwargs,
 ) -> ServeControllerClient:
@@ -210,7 +208,7 @@ async def serve_start_async(
     controller, controller_name = (
         await ray.remote(_start_controller)
         .options(num_cpus=0)
-        .remote(http_options, dedicated_cpu, grpc_options, **kwargs)
+        .remote(http_options, grpc_options, **kwargs)
     )
 
     client = ServeControllerClient(
@@ -224,7 +222,6 @@ async def serve_start_async(
 
 def serve_start(
     http_options: Union[None, dict, HTTPOptions] = None,
-    dedicated_cpu: bool = False,
     grpc_options: Union[None, dict, gRPCOptions] = None,
     **kwargs,
 ) -> ServeControllerClient:
@@ -259,8 +256,6 @@ def serve_start(
                 - "NoServer" or None: disable HTTP server.
             - num_cpus (int): The number of CPU cores to reserve for each
               internal Serve HTTP proxy actor.  Defaults to 0.
-        dedicated_cpu: Whether to reserve a CPU core for the internal
-          Serve controller actor.  Defaults to False.
         grpc_options: [Experimental] Configuration options for gRPC proxy.
           You can pass in a gRPCOptions object with fields:
 
@@ -285,7 +280,7 @@ def serve_start(
         pass
 
     controller, controller_name = _start_controller(
-        http_options, dedicated_cpu, grpc_options, **kwargs
+        http_options, grpc_options, **kwargs
     )
 
     client = ServeControllerClient(

diff --git a/python/ray/serve/_private/controller.py b/python/ray/serve/_private/controller.py
@@ -1033,7 +1033,6 @@ class ServeControllerAvatar:
     def __init__(
         self,
         controller_name: str,
-        dedicated_cpu: bool = False,
         http_proxy_port: int = 8000,
     ):
         try:
@@ -1044,7 +1043,7 @@ def __init__(
             http_config = HTTPOptions()
             http_config.port = http_proxy_port
             self._controller = ServeController.options(
-                num_cpus=1 if dedicated_cpu else 0,
+                num_cpus=0,
                 name=controller_name,
                 lifetime="detached",
                 max_restarts=-1,

diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py
@@ -89,8 +89,6 @@ def start(
         http_options: HTTP config options for the proxies. These can be passed as an
           unstructured dictionary or the structured `HTTPOptions` class. See
           `HTTPOptions` for supported options.
-        dedicated_cpu: [DEPRECATED] Whether to reserve a CPU core for the
-          Serve controller actor.
         grpc_options: [EXPERIMENTAL] gRPC config options for the proxies. These can
           be passed as an unstructured dictionary or the structured `gRPCOptions`
           class See `gRPCOptions` for supported options.
@@ -102,10 +100,10 @@ class See `gRPCOptions` for supported options.
             "In a future release, it will be removed altogether."
         )
 
-    if dedicated_cpu:
-        warnings.warn(
-            "Setting `dedicated_cpu=True` in `serve.start` is deprecated and will be "
-            "removed in a future version."
+    if dedicated_cpu is not False:
+        raise ValueError(
+            "`dedicated_cpu` is no longer supported. "
+            "In a future release, it will be removed altogether."
         )
 
     if proxy_location is None:
@@ -124,7 +122,6 @@ class See `gRPCOptions` for supported options.
 
     _private_api.serve_start(
         http_options=http_options,
-        dedicated_cpu=dedicated_cpu,
         grpc_options=grpc_options,
         **kwargs,
     )

diff --git a/python/ray/serve/tests/test_standalone.py b/python/ray/serve/tests/test_standalone.py
@@ -301,27 +301,6 @@ def connect_in_deployment(*args):
     assert "deployment-ception" in serve.list_deployments()
 
 
-@pytest.mark.parametrize("controller_cpu", [True, False])
-@pytest.mark.parametrize("num_proxy_cpus", [0, 1, 2])
-def test_dedicated_cpu(controller_cpu, num_proxy_cpus, ray_cluster):
-    cluster = ray_cluster
-    num_cluster_cpus = 8
-    head_node = cluster.add_node(num_cpus=num_cluster_cpus)
-
-    ray.init(head_node.address)
-    wait_for_condition(lambda: ray.cluster_resources().get("CPU") == num_cluster_cpus)
-
-    num_cpus_used = int(controller_cpu) + num_proxy_cpus
-
-    serve.start(
-        dedicated_cpu=controller_cpu, http_options=HTTPOptions(num_cpus=num_proxy_cpus)
-    )
-    available_cpus = num_cluster_cpus - num_cpus_used
-    wait_for_condition(lambda: (ray.available_resources().get("CPU") == available_cpus))
-    serve.shutdown()
-    ray.shutdown()
-
-
 def test_set_socket_reuse_port():
     sock = socket.socket()
     if hasattr(socket, "SO_REUSEPORT"):