Skip to content

Commit

Permalink
[serve] Remove aiorwlock dependency (ray-project#42159)
Browse files Browse the repository at this point in the history
This was added long ago to avoid race conditions between reconfigure and user method calls. However, sometime since then the reader lock acquisition seems to have been accidentally removed. We have not seen any issues from this so my preference is to just altogether remove the complexity & dependency.

Users can implement their own coordination if this is a problem for them by adding concurrency control to their reconfigure method.

---------

Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
  • Loading branch information
edoakes authored and vickytsang committed Jan 12, 2024
1 parent 9282726 commit 0068a47
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 20 deletions.
36 changes: 17 additions & 19 deletions python/ray/serve/_private/replica.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
from importlib import import_module
from typing import Any, AsyncGenerator, Callable, Dict, Optional, Tuple, Union

import aiorwlock
import starlette.responses
from starlette.requests import Request
from starlette.types import Message, Receive, Scope, Send
Expand Down Expand Up @@ -708,7 +707,6 @@ def __init__(
self._init_kwargs = init_kwargs
self._is_function = inspect.isfunction(deployment_def)
self._deployment_id = deployment_id
self._rwlock = aiorwlock.RWLock()
self._delete_lock = asyncio.Lock()

# Will be populated in `initialize_callable`.
Expand Down Expand Up @@ -800,24 +798,24 @@ async def call_user_health_check(self):
await self._user_health_check()

async def call_reconfigure(self, user_config: Any):
async with self._rwlock.writer:
if user_config is not None:
if self._is_function:
raise ValueError(
"deployment_def must be a class to use user_config"
)
elif not hasattr(self._callable, RECONFIGURE_METHOD):
raise RayServeException(
"user_config specified but deployment "
+ self._deployment_id
+ " missing "
+ RECONFIGURE_METHOD
+ " method"
)
reconfigure_method = sync_to_async(
getattr(self._callable, RECONFIGURE_METHOD)
# NOTE(edoakes): there is the possibility of a race condition in user code if
# they don't have any form of concurrency control between `reconfigure` and
# other methods. See https://github.com/ray-project/ray/pull/42159.
if user_config is not None:
if self._is_function:
raise ValueError("deployment_def must be a class to use user_config")
elif not hasattr(self._callable, RECONFIGURE_METHOD):
raise RayServeException(
"user_config specified but deployment "
+ self._deployment_id
+ " missing "
+ RECONFIGURE_METHOD
+ " method"
)
await reconfigure_method(user_config)
reconfigure_method = sync_to_async(
getattr(self._callable, RECONFIGURE_METHOD)
)
await reconfigure_method(user_config)

async def call_user_method(
self,
Expand Down
1 change: 0 additions & 1 deletion python/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,6 @@ def get_packages(self):
"requests",
"starlette",
"fastapi",
"aiorwlock",
"watchfiles",
],
"tune": ["pandas", "tensorboardX>=1.9", "requests", pyarrow_dep, "fsspec"],
Expand Down

0 comments on commit 0068a47

Please sign in to comment.