[serve] Better surfacing of errors in serve status #34773

zcin · 2023-04-26T02:27:36Z

Why are these changes needed?

Surface tracebacks from constructor failures.
Output from serve status:

name: default
app_status:
  status: DEPLOY_FAILED
  message: |-
    Deploying app 'default' failed:
    ray::deploy_serve_application() (pid=15878, ip=192.168.1.14)
      File "/Users/cindyz/ray/python/ray/serve/controller.py", line 947, in deploy_serve_application
        serve.run(app, name=name, route_prefix=route_prefix)
      File "/Users/cindyz/ray/python/ray/serve/api.py", line 539, in run
        client.deploy_application(
      File "/Users/cindyz/ray/python/ray/serve/_private/client.py", line 43, in check
        return f(self, *args, **kwargs)
      File "/Users/cindyz/ray/python/ray/serve/_private/client.py", line 299, in deploy_application
        self._wait_for_deployment_healthy(deployment_name)
      File "/Users/cindyz/ray/python/ray/serve/_private/client.py", line 183, in _wait_for_deployment_healthy
        raise RuntimeError(
    RuntimeError: Deployment default_Fail is UNHEALTHY: The Deployment failed to start 3 times in a row. This may be due to a problem with the deployment constructor or the initial health check failing. See controller logs for details. Retrying after 1 seconds. Error:
    ray::ServeReplica:default_Fail.is_initialized() (pid=15919, ip=192.168.1.14, repr=<ray.serve._private.replica.ServeReplica:default_Fail object at 0x1257772e0>)
      File "/Users/cindyz/miniforge3/envs/ray/lib/python3.8/concurrent/futures/_base.py", line 437, in result
        return self.__get_result()
      File "/Users/cindyz/miniforge3/envs/ray/lib/python3.8/concurrent/futures/_base.py", line 389, in __get_result
        raise self._exception
      File "/Users/cindyz/ray/python/ray/serve/_private/replica.py", line 234, in is_initialized
        await self._initialize_replica()
      File "/Users/cindyz/ray/python/ray/serve/_private/replica.py", line 150, in initialize_replica
        await sync_to_async(_callable.__init__)(*init_args, **init_kwargs)
      File "/Users/cindyz/Desktop/constructor_fail.py", line 16, in __init__
        raise Exception("I need to know about this!")
    Exception: I need to know about this!
  deployment_timestamp: 1682476137.8513532
deployment_statuses:
- name: default_Fail
  status: UNHEALTHY
  message: |-
    The Deployment failed to start 3 times in a row. This may be due to a problem with the deployment constructor or the initial health check failing. See controller logs for details. Retrying after 1 seconds. Error:
    ray::ServeReplica:default_Fail.is_initialized() (pid=15919, ip=192.168.1.14, repr=<ray.serve._private.replica.ServeReplica:default_Fail object at 0x1257772e0>)
      File "/Users/cindyz/miniforge3/envs/ray/lib/python3.8/concurrent/futures/_base.py", line 437, in result
        return self.__get_result()
      File "/Users/cindyz/miniforge3/envs/ray/lib/python3.8/concurrent/futures/_base.py", line 389, in __get_result
        raise self._exception
      File "/Users/cindyz/ray/python/ray/serve/_private/replica.py", line 234, in is_initialized
        await self._initialize_replica()
      File "/Users/cindyz/ray/python/ray/serve/_private/replica.py", line 150, in initialize_replica
        await sync_to_async(_callable.__init__)(*init_args, **init_kwargs)
      File "/Users/cindyz/Desktop/constructor_fail.py", line 16, in __init__
        raise Exception("I need to know about this!")
    Exception: I need to know about this!

Serializes exceptions from the replica actor, so that they are displayed properly when surfaced through the controller.

Related issue number

Closes #34712
Closes #34683

Checks

I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
I've run scripts/format.sh to lint the changes in this PR.
I've included any doc changes needed for https://docs.ray.io/en/master/.
- I've added any new APIs to the API Reference. For example, if I added a
  method in Tune, I've added it in doc/source/tune/api/ under the
  corresponding .rst file.
I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
Testing Strategy
- Unit tests
- Release tests
- This PR is not tested :(

Signed-off-by: Cindy Zhang <[email protected]>

sihanwang41 · 2023-04-26T17:28:00Z

python/ray/serve/_private/deployment_state.py

@@ -446,7 +446,9 @@ def recover(self):
        else:
            self._ready_obj_ref = self._actor_handle.get_metadata.remote()

-    def check_ready(self) -> Tuple[ReplicaStartupStatus, Optional[DeploymentVersion]]:
+    def check_ready(


nit: Update the docstring for return value.

zcin · 2023-04-26T18:29:52Z

FYI: Preferably #34430 is merged first, since this touches the same code and would prefer to resolve merge conflicts here.

Signed-off-by: Cindy Zhang <[email protected]>

edoakes · 2023-04-26T19:57:06Z

@zcin other PR is merged please fix conflicts

edoakes · 2023-04-26T19:58:06Z

@zcin should we fix #34683 at the same time by doing the str(e) inside the replica and returning that somehow to controller?

Signed-off-by: Cindy Zhang <[email protected]>

ollie-iterators · 2023-04-28T15:01:37Z

I think the doc tests will be fixed by merging in recent changes from the main branch

Signed-off-by: Cindy Zhang <[email protected]>

ollie-iterators · 2023-04-28T18:45:35Z

Lint Error: python/ray/serve/tests/test_config_files/sqlalchemy.py:5:1: E302 expected 2 blank lines, found 1

Signed-off-by: Cindy Zhang <[email protected]>

zcin · 2023-04-29T00:19:52Z

@zcin should we fix #34683 at the same time by doing the str(e) inside the replica and returning that somehow to controller?

I've added the fix into this PR!

edoakes · 2023-05-01T21:22:21Z

python/ray/serve/_private/deployment_state.py

+                # here because the exception may be a RayTaskError, in which case the
+                # full details of the error is not displayed properly with
+                # traceback.format_exc().
+                return ReplicaStartupStatus.FAILED, str(e)


I think we might want to use repr(e) instead of str(e), but there's likely no difference in practice with common exception types

For RayTaskErrors specifically, repr(e) seems to cause weird formatting:

Got it. Btw, can we unwrap the RayTaskError so we only show the user their underlying exception instead? This can be done with e.as_instanceof_cause()

Signed-off-by: Cindy Zhang <[email protected]>

edoakes · 2023-05-03T16:17:14Z

python/ray/serve/_private/deployment_state.py

+                # NOTE(zcin): we should use str(e) instead of traceback.format_exc()
+                # here because the full details of the error is not displayed properly
+                # with traceback.format_exc().
+                return ReplicaStartupStatus.FAILED, str(e.as_instanceof_cause())


Signed-off-by: Cindy Zhang <[email protected]>

zcin · 2023-05-04T18:34:41Z

@edoakes Not sure whats happening with the GPU test but I pulled an instance with repro-ci and confirmed the serve doc tests work. Should be ready to merge.

1. Surface tracebacks from constructor failures. Output from `serve status`: ``` name: default app_status: status: DEPLOY_FAILED message: |- Deploying app 'default' failed: ray::deploy_serve_application() (pid=15878, ip=192.168.1.14) File "/Users/cindyz/ray/python/ray/serve/controller.py", line 947, in deploy_serve_application serve.run(app, name=name, route_prefix=route_prefix) File "/Users/cindyz/ray/python/ray/serve/api.py", line 539, in run client.deploy_application( File "/Users/cindyz/ray/python/ray/serve/_private/client.py", line 43, in check return f(self, *args, **kwargs) File "/Users/cindyz/ray/python/ray/serve/_private/client.py", line 299, in deploy_application self._wait_for_deployment_healthy(deployment_name) File "/Users/cindyz/ray/python/ray/serve/_private/client.py", line 183, in _wait_for_deployment_healthy raise RuntimeError( RuntimeError: Deployment default_Fail is UNHEALTHY: The Deployment failed to start 3 times in a row. This may be due to a problem with the deployment constructor or the initial health check failing. See controller logs for details. Retrying after 1 seconds. Error: ray::ServeReplica:default_Fail.is_initialized() (pid=15919, ip=192.168.1.14, repr=<ray.serve._private.replica.ServeReplica:default_Fail object at 0x1257772e0>) File "/Users/cindyz/miniforge3/envs/ray/lib/python3.8/concurrent/futures/_base.py", line 437, in result return self.__get_result() File "/Users/cindyz/miniforge3/envs/ray/lib/python3.8/concurrent/futures/_base.py", line 389, in __get_result raise self._exception File "/Users/cindyz/ray/python/ray/serve/_private/replica.py", line 234, in is_initialized await self._initialize_replica() File "/Users/cindyz/ray/python/ray/serve/_private/replica.py", line 150, in initialize_replica await sync_to_async(_callable.__init__)(*init_args, **init_kwargs) File "/Users/cindyz/Desktop/constructor_fail.py", line 16, in __init__ raise Exception("I need to know about this!") Exception: I need to know about this! deployment_timestamp: 1682476137.8513532 deployment_statuses: - name: default_Fail status: UNHEALTHY message: |- The Deployment failed to start 3 times in a row. This may be due to a problem with the deployment constructor or the initial health check failing. See controller logs for details. Retrying after 1 seconds. Error: ray::ServeReplica:default_Fail.is_initialized() (pid=15919, ip=192.168.1.14, repr=<ray.serve._private.replica.ServeReplica:default_Fail object at 0x1257772e0>) File "/Users/cindyz/miniforge3/envs/ray/lib/python3.8/concurrent/futures/_base.py", line 437, in result return self.__get_result() File "/Users/cindyz/miniforge3/envs/ray/lib/python3.8/concurrent/futures/_base.py", line 389, in __get_result raise self._exception File "/Users/cindyz/ray/python/ray/serve/_private/replica.py", line 234, in is_initialized await self._initialize_replica() File "/Users/cindyz/ray/python/ray/serve/_private/replica.py", line 150, in initialize_replica await sync_to_async(_callable.__init__)(*init_args, **init_kwargs) File "/Users/cindyz/Desktop/constructor_fail.py", line 16, in __init__ raise Exception("I need to know about this!") Exception: I need to know about this! ``` 2. Serializes exceptions from the replica actor, so that they are displayed properly when surfaced through the controller.

zcin added 2 commits April 25, 2023 19:26

surface constructor errors

d2c419b

Signed-off-by: Cindy Zhang <[email protected]>

fix tests

ff8e255

Signed-off-by: Cindy Zhang <[email protected]>

zcin changed the title ~~surface constructor errors~~ [serve] Surface errors from constructor failure Apr 26, 2023

zcin marked this pull request as ready for review April 26, 2023 17:16

zcin assigned sihanwang41, edoakes and shrekris-anyscale Apr 26, 2023

sihanwang41 approved these changes Apr 26, 2023

View reviewed changes

update docstring

7ed8d5a

Signed-off-by: Cindy Zhang <[email protected]>

zcin force-pushed the status-message branch 2 times, most recently from d0c7078 to 887d07d Compare April 26, 2023 20:03

Merge branch 'master' into status-message

ad02f88

Signed-off-by: Cindy Zhang <[email protected]>

zcin force-pushed the status-message branch from 887d07d to ad02f88 Compare April 26, 2023 20:03

fix

65665d3

Signed-off-by: Cindy Zhang <[email protected]>

ollie-iterators mentioned this pull request Apr 27, 2023

[ci/docker] Fix arm64 docker image builds by downgrading miniconda #34825

Merged

8 tasks

zcin added 2 commits April 28, 2023 08:54

Merge branch 'master' into status-message

5e1824d

Signed-off-by: Cindy Zhang <[email protected]>

serialize exceptions from replica

036c6b9

Signed-off-by: Cindy Zhang <[email protected]>

lint

075da8f

Signed-off-by: Cindy Zhang <[email protected]>

zcin changed the title ~~[serve] Surface errors from constructor failure~~ [serve] Better surfacing of errors in serve status Apr 29, 2023

edoakes approved these changes May 1, 2023

View reviewed changes

zcin added 3 commits May 1, 2023 17:51

Merge branch 'master' into status-message

41da937

Signed-off-by: Cindy Zhang <[email protected]>

use as_instanceof_cause for raytaskerrors

a88c16f

Signed-off-by: Cindy Zhang <[email protected]>

Merge branch 'master' into status-message

ae69e17

Signed-off-by: Cindy Zhang <[email protected]>

edoakes reviewed May 3, 2023

View reviewed changes

zcin added 2 commits May 3, 2023 09:59

Merge branch 'master' into status-message

3afdd97

Signed-off-by: Cindy Zhang <[email protected]>

Merge branch 'master' into status-message

be8a954

Signed-off-by: Cindy Zhang <[email protected]>

edoakes merged commit adae990 into ray-project:master May 4, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[serve] Better surfacing of errors in serve status #34773

[serve] Better surfacing of errors in serve status #34773

zcin commented Apr 26, 2023 •

edited

Loading

sihanwang41 Apr 26, 2023

zcin commented Apr 26, 2023

edoakes commented Apr 26, 2023

edoakes commented Apr 26, 2023

ollie-iterators commented Apr 28, 2023

ollie-iterators commented Apr 28, 2023

zcin commented Apr 29, 2023

edoakes May 1, 2023

zcin May 2, 2023 •

edited

Loading

edoakes May 2, 2023

edoakes May 3, 2023

zcin commented May 4, 2023

[serve] Better surfacing of errors in serve status #34773

[serve] Better surfacing of errors in serve status #34773

Conversation

zcin commented Apr 26, 2023 • edited Loading

Why are these changes needed?

Related issue number

Checks

sihanwang41 Apr 26, 2023

Choose a reason for hiding this comment

zcin commented Apr 26, 2023

edoakes commented Apr 26, 2023

edoakes commented Apr 26, 2023

ollie-iterators commented Apr 28, 2023

ollie-iterators commented Apr 28, 2023

zcin commented Apr 29, 2023

edoakes May 1, 2023

Choose a reason for hiding this comment

zcin May 2, 2023 • edited Loading

Choose a reason for hiding this comment

edoakes May 2, 2023

Choose a reason for hiding this comment

edoakes May 3, 2023

Choose a reason for hiding this comment

zcin commented May 4, 2023

zcin commented Apr 26, 2023 •

edited

Loading

zcin May 2, 2023 •

edited

Loading