[serve] Update replica ID format (and other random strings) to better…

… match UUIDs (ray-project#41737) Small cosmetic update to our replica ID generation to make it use the same alphabet as UUID (which is standard and we use for request IDs). --------- Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
ShuN6211 · Dec 8, 2023 · f783234 · f783234
1 parent 2487553
commit f783234
Show file tree

Hide file tree

Showing 11 changed files with 32 additions and 28 deletions.
diff --git a/python/ray/serve/_private/controller.py b/python/ray/serve/_private/controller.py
@@ -614,7 +614,7 @@ def shutdown(self):
             and proxy_state_is_shutdown
         ):
             logger.warning(
-                "All resources have shut down, shutting down controller!",
+                "All resources have shut down, controller exiting.",
                 extra={"log_to_stderr": False},
             )
             _controller_actor = ray.get_runtime_context().current_actor

diff --git a/python/ray/serve/_private/deployment_state.py b/python/ray/serve/_private/deployment_state.py
@@ -53,7 +53,7 @@
     check_obj_ref_ready_nowait,
     format_actor_name,
     get_capacity_adjusted_num_replicas,
-    get_random_letters,
+    get_random_string,
     msgpack_deserialize,
     msgpack_serialize,
 )
@@ -1831,7 +1831,7 @@ def _scale_deployment_replicas(
                 )
                 for _ in range(to_add):
                     replica_name = ReplicaName(
-                        self.app_name, self.deployment_name, get_random_letters()
+                        self.app_name, self.deployment_name, get_random_string()
                     )
                     new_deployment_replica = DeploymentReplica(
                         self._controller_name,

diff --git a/python/ray/serve/_private/utils.py b/python/ray/serve/_private/utils.py
@@ -134,8 +134,12 @@ def block_until_http_ready(
         time.sleep(backoff_time_s)
 
 
-def get_random_letters(length=6):
-    return "".join(random.choices(string.ascii_letters, k=length))
+# Match the standard alphabet used for UUIDs.
+RANDOM_STRING_ALPHABET = string.ascii_lowercase + string.digits
+
+
+def get_random_string(length=8):
+    return "".join(random.choices(RANDOM_STRING_ALPHABET, k=length))
 
 
 def format_actor_name(actor_name, controller_name=None, *modifiers):

diff --git a/python/ray/serve/_private/version.py b/python/ray/serve/_private/version.py
@@ -7,7 +7,7 @@
 
 from ray._private.pydantic_compat import BaseModel
 from ray.serve._private.config import DeploymentConfig
-from ray.serve._private.utils import DeploymentOptionUpdateType, get_random_letters
+from ray.serve._private.utils import DeploymentOptionUpdateType, get_random_string
 from ray.serve.generated.serve_pb2 import DeploymentVersion as DeploymentVersionProto
 
 logger = logging.getLogger("ray.serve")
@@ -26,7 +26,7 @@ def __init__(
         if code_version is not None and not isinstance(code_version, str):
             raise TypeError(f"code_version must be str, got {type(code_version)}.")
         if code_version is None:
-            self.code_version = get_random_letters()
+            self.code_version = get_random_string()
         else:
             self.code_version = code_version
 

diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py
@@ -31,7 +31,7 @@
     Default,
     ensure_serialization_context,
     extract_self_if_method_call,
-    get_random_letters,
+    get_random_string,
 )
 from ray.serve.config import (
     AutoscalingConfig,
@@ -533,7 +533,7 @@ def run(
             "name": deployment._name,
             "replica_config": deployment._replica_config,
             "deployment_config": deployment._deployment_config,
-            "version": deployment._version or get_random_letters(),
+            "version": deployment._version or get_random_string(),
             "route_prefix": deployment.route_prefix,
             "url": deployment.url,
             "docs_path": deployment._docs_path,

diff --git a/python/ray/serve/handle.py b/python/ray/serve/handle.py
@@ -15,7 +15,7 @@
 from ray.serve._private.utils import (
     DEFAULT,
     get_current_actor_id,
-    get_random_letters,
+    get_random_string,
     is_running_in_asyncio_loop,
 )
 from ray.util import metrics
@@ -180,7 +180,7 @@ def _create_request_counter(cls, app_name, deployment_name):
             # TODO(zcin): Separate deployment_id into deployment and application tags
             {
                 "handle": cls._gen_handle_tag(
-                    app_name, deployment_name, handle_id=get_random_letters()
+                    app_name, deployment_name, handle_id=get_random_string()
                 ),
                 "deployment": deployment_name,
                 "application": app_name,

diff --git a/python/ray/serve/tests/test_controller_recovery.py b/python/ray/serve/tests/test_controller_recovery.py
@@ -20,7 +20,7 @@
     SERVE_NAMESPACE,
     SERVE_PROXY_NAME,
 )
-from ray.serve._private.utils import get_random_letters
+from ray.serve._private.utils import get_random_string
 from ray.serve.schema import LoggingConfig
 from ray.serve.tests.test_failure import request_with_retries
 from ray.util.state import list_actors
@@ -130,7 +130,7 @@ def call(block=False):
 
         return ret.split("|")[0], ret.split("|")[1]
 
-    signal_name = f"signal#{get_random_letters()}"
+    signal_name = f"signal#{get_random_string()}"
     signal = SignalActor.options(name=signal_name).remote()
 
     @serve.deployment(name=name, version="1", num_replicas=2)

diff --git a/python/ray/serve/tests/test_deploy.py b/python/ray/serve/tests/test_deploy.py
@@ -11,7 +11,7 @@
 from ray import serve
 from ray._private.pydantic_compat import ValidationError
 from ray._private.test_utils import SignalActor
-from ray.serve._private.utils import get_random_letters
+from ray.serve._private.utils import get_random_string
 from ray.serve.exceptions import RayServeException
 
 
@@ -115,7 +115,7 @@ def call(block=False):
 
         return ret.split("|")[0], ret.split("|")[1]
 
-    signal_name = f"signal-{get_random_letters()}"
+    signal_name = f"signal-{get_random_string()}"
     signal = SignalActor.options(name=signal_name).remote()
 
     @serve.deployment(name=name, version="1")
@@ -206,7 +206,7 @@ def call(block=False):
 
         return ret.split("|")[0], ret.split("|")[1]
 
-    signal_name = f"signal-{get_random_letters()}"
+    signal_name = f"signal-{get_random_string()}"
     signal = SignalActor.options(name=signal_name).remote()
 
     @serve.deployment(name=name, version="1", num_replicas=2)
@@ -300,7 +300,7 @@ def call():
 
         return ret.split("|")[0], ret.split("|")[1]
 
-    signal_name = f"signal-{get_random_letters()}"
+    signal_name = f"signal-{get_random_string()}"
     signal = SignalActor.options(name=signal_name).remote()
 
     @serve.deployment(name=name, version="1", num_replicas=2)

diff --git a/python/ray/serve/tests/unit/test_application_state.py b/python/ray/serve/tests/unit/test_application_state.py
@@ -21,7 +21,7 @@
 from ray.serve._private.deploy_utils import deploy_args_to_deployment_info
 from ray.serve._private.deployment_info import DeploymentInfo
 from ray.serve._private.test_utils import MockKVStore
-from ray.serve._private.utils import get_random_letters
+from ray.serve._private.utils import get_random_string
 from ray.serve.exceptions import RayServeException
 from ray.serve.schema import DeploymentSchema, ServeApplicationSchema
 
@@ -151,7 +151,7 @@ def deployment_params(name: str, route_prefix: str = None, docs_path: str = None
     return {
         "deployment_name": name,
         "deployment_config_proto_bytes": DeploymentConfig(
-            num_replicas=1, user_config={}, version=get_random_letters()
+            num_replicas=1, user_config={}, version=get_random_string()
         ).to_proto_bytes(),
         "replica_config_proto_bytes": ReplicaConfig.create(
             lambda x: x

diff --git a/python/ray/serve/tests/unit/test_common.py b/python/ray/serve/tests/unit/test_common.py
@@ -12,7 +12,7 @@
     RunningReplicaInfo,
     StatusOverview,
 )
-from ray.serve._private.utils import get_random_letters
+from ray.serve._private.utils import get_random_string
 from ray.serve.generated.serve_pb2 import (
     ApplicationStatusInfo as ApplicationStatusInfoProto,
 )
@@ -25,15 +25,15 @@
 def test_replica_tag_formatting():
     app_name = "my_app"
     deployment_tag = "DeploymentA"
-    replica_suffix = get_random_letters()
+    replica_suffix = get_random_string()
 
     replica_name = ReplicaName(app_name, deployment_tag, replica_suffix)
     assert replica_name.replica_tag == f"{app_name}#{deployment_tag}#{replica_suffix}"
     assert str(replica_name) == f"{app_name}#{deployment_tag}#{replica_suffix}"
 
 
 def test_replica_name_from_str():
-    replica_suffix = get_random_letters()
+    replica_suffix = get_random_string()
     actor_name = f"{ReplicaName.prefix}DeploymentA#{replica_suffix}"
 
     replica_name = ReplicaName.from_str(actor_name)
@@ -45,7 +45,7 @@ def test_replica_name_from_str():
 
 
 def test_invalid_name_from_str():
-    replica_suffix = get_random_letters()
+    replica_suffix = get_random_string()
 
     replica_tag = f"DeploymentA##{replica_suffix}"
     with pytest.raises(AssertionError):
@@ -58,7 +58,7 @@ def test_invalid_name_from_str():
 
 
 def test_is_replica_name():
-    replica_suffix = get_random_letters()
+    replica_suffix = get_random_string()
 
     assert not ReplicaName.is_replica_name(f"DeploymentA##{replica_suffix}")
     assert not ReplicaName.is_replica_name(f"DeploymentA#{replica_suffix}")

diff --git a/python/ray/serve/tests/unit/test_deployment_state.py b/python/ray/serve/tests/unit/test_deployment_state.py
@@ -38,7 +38,7 @@
 from ray.serve._private.test_utils import MockKVStore, MockTimer
 from ray.serve._private.utils import (
     get_capacity_adjusted_num_replicas,
-    get_random_letters,
+    get_random_string,
 )
 
 
@@ -313,7 +313,7 @@ def deployment_info(
     if version is not None:
         code_version = version
     else:
-        code_version = get_random_letters()
+        code_version = get_random_string()
 
     version = DeploymentVersion(
         code_version, info.deployment_config, info.replica_config.ray_actor_options
@@ -373,7 +373,7 @@ def mock_save_checkpoint_fn(*args, **kwargs):
 
 def replica(version: Optional[DeploymentVersion] = None) -> VersionedReplica:
     if version is None:
-        version = DeploymentVersion(get_random_letters(), DeploymentConfig(), {})
+        version = DeploymentVersion(get_random_string(), DeploymentConfig(), {})
 
     class MockVersionedReplica(VersionedReplica):
         def __init__(self, version: DeploymentVersion):
@@ -2155,7 +2155,7 @@ def test_scale_num_replicas(
     """
 
     # State
-    version = get_random_letters()
+    version = get_random_string()
     deployment_id = DeploymentID("test_deployment", "test_app")
 
     # Create deployment state manager