Skip to content
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

[Serve] Optimize ServeController.get_app_config() #45878

Merged
5 changes: 0 additions & 5 deletions python/ray/serve/_private/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -379,11 +379,6 @@ def get_deployment_info(
deployment_route.route if deployment_route.route != "" else None,
)

@_ensure_connected
def get_app_config(self, name: str = SERVE_DEFAULT_APP_NAME) -> Dict:
"""Returns the most recently requested Serve config."""
return ray.get(self._controller.get_app_config.remote(name))
Comment on lines -382 to -385
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be unused, so I went ahead and removed it instead of refactoring it. But I'm happy to provide an equivalent method in this PR if desired.


@_ensure_connected
def get_serve_status(self, name: str = SERVE_DEFAULT_APP_NAME) -> StatusOverview:
proto = StatusOverviewProto.FromString(
Expand Down
30 changes: 21 additions & 9 deletions python/ray/serve/_private/controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -877,10 +877,18 @@ def get_serve_instance_details(self) -> Dict:
grpc_config = self.get_grpc_config()
applications = {}

app_statuses = self.application_state_manager.list_app_statuses()

# If there are no app statuses, there's no point getting the app configs.
# Moreover, there might be no app statuses because the GCS is down,
# in which case getting the app configs would fail anyway,
# since they're stored in the checkpoint in the GCS.
app_configs = self.get_app_configs() if app_statuses else {}

for (
app_name,
app_status_info,
) in self.application_state_manager.list_app_statuses().items():
) in app_statuses.items():
applications[app_name] = ApplicationDetails(
name=app_name,
route_prefix=self.application_state_manager.get_route_prefix(app_name),
Expand All @@ -889,8 +897,9 @@ def get_serve_instance_details(self) -> Dict:
message=app_status_info.message,
last_deployed_time_s=app_status_info.deployment_timestamp,
# This can be none if the app was deployed through
# serve.run, or if the app is in deleting state
deployed_app_config=self.get_app_config(app_name),
# serve.run, the app is in deleting state,
# or a checkpoint hasn't been set yet
deployed_app_config=app_configs.get(app_name),
deployments=self.application_state_manager.list_deployment_details(
app_name
),
Expand Down Expand Up @@ -945,13 +954,16 @@ def list_serve_statuses(self) -> List[bytes]:
statuses.append(self.get_serve_status(name))
return statuses

def get_app_config(self, name: str = SERVE_DEFAULT_APP_NAME) -> Optional[Dict]:
def get_app_configs(self) -> Dict[str, ServeApplicationSchema]:
checkpoint = self.kv_store.get(CONFIG_CHECKPOINT_KEY)
if checkpoint is not None:
_, _, _, config_checkpoints_dict = pickle.loads(checkpoint)
if name in config_checkpoints_dict:
config = config_checkpoints_dict[name]
return ServeApplicationSchema.parse_obj(config).dict(exclude_unset=True)
if checkpoint is None:
return {}

_, _, _, config_checkpoints_dict = pickle.loads(checkpoint)
return {
app: ServeApplicationSchema.parse_obj(config)
shrekris-anyscale marked this conversation as resolved.
Show resolved Hide resolved
for app, config in config_checkpoints_dict.items()
}

def get_all_deployment_statuses(self) -> List[bytes]:
"""Gets deployment status bytes for all live deployments."""
Expand Down
Loading