[Serve] Recover PENDING_INITIALIZATION status actor #33890

sihanwang41 · 2023-03-29T21:58:39Z

Why are these changes needed?

Then replica is under initializing state and the controller is dead, user will see

(ServeController pid=458318) ERROR 2023-03-29 08:44:42,547 controller 458318 deployment_state.py:500 - Exception in deployment 'xqHWInctmH'
(ServeController pid=458318) Traceback (most recent call last):
(ServeController pid=458318)   File "/home/ray/anaconda3/lib/python3.7/site-packages/ray/serve/_private/deployment_state.py", line 489, in check_ready
(ServeController pid=458318)     deployment_config, version = ray.get(self._ready_obj_ref)
(ServeController pid=458318)   File "/home/ray/anaconda3/lib/python3.7/site-packages/ray/_private/client_mode_hook.py", line 105, in wrapper
(ServeController pid=458318)     return func(*args, **kwargs)
(ServeController pid=458318)   File "/home/ray/anaconda3/lib/python3.7/site-packages/ray/_private/worker.py", line 2508, in get
(ServeController pid=458318)     raise value.as_instanceof_cause()
(ServeController pid=458318) ray.exceptions.RayTaskError(AttributeError): ray::ServeReplica:xqHWInctmH.get_metadata() (pid=458148, ip=172.31.126.222, repr=<ray.serve._private.replica.ServeReplica:xqHWInctmH object at 0x7f57ae0f0110>)
(ServeController pid=458318)   File "/home/ray/anaconda3/lib/python3.7/concurrent/futures/_base.py", line 428, in result
(ServeController pid=458318)     return self.__get_result()
(ServeController pid=458318)   File "/home/ray/anaconda3/lib/python3.7/concurrent/futures/_base.py", line 384, in __get_result
(ServeController pid=458318)     raise self._exception
(ServeController pid=458318)   File "/home/ray/anaconda3/lib/python3.7/site-packages/ray/serve/_private/replica.py", line 249, in get_metadata
(ServeController pid=458318)     return self.replica.deployment_config, self.replica.version
(ServeController pid=458318) AttributeError: 'NoneType' object has no attribute 'deployment_config'

Fix the NoneType bug when recover happens, no matter the actor is under any state, user would not see the traceback. Instead user will see the

slow startup warning, and then replica will be terminated, and new replica will be provisioned.
If actor succeed in PENDING_INITIALIZATION, no error pops out.

This is observed from long_running_serve_failure: https://console.anyscale-staging.com/o/anyscale-internal/projects/prj_qC3ZfndQWYYjx2cz8KWGNUL4/clusters/ses_u7xeve33e2djg9grgr9qcs9l4x?command-history-section=command_history

Note: If user constructor initialization just needs very long time to finish, it is recommended to increase the SERVE_SLOW_STARTUP_WARNING_S, (if the user doesn't change, the deployment manager will terminate the old replica and start new replica after the timeout.)

Related issue number

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 :(

…ecover bug Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

python/ray/serve/tests/test_controller_recovery.py

zcin · 2023-03-31T17:51:55Z

python/ray/serve/tests/test_controller_recovery.py

+        for actor in all_current_actors:
+            if name in actor["name"]:
+                return actor["name"]
+        return None


Suggested change

return None

zcin · 2023-03-31T18:29:19Z

python/ray/serve/tests/test_controller_recovery.py

+    class V1:
+        def __init__(self):
+            signal2.send.remote()
+            ray.get(signal.wait.remote())


Suggested change

ray.get(signal.wait.remote())

await signal.wait.remote()

I think it make no difference for this test case.

Is the controller able to call the replica during recovery while the replica is stuck on ray.get(), since it's a blocking call? I think if the test wants to test the buggy code path, it needs to be able to call into the replica actor while the replica is waiting on signal.wait.remote()

python/ray/serve/tests/test_controller_recovery.py

zcin · 2023-03-31T18:32:53Z

python/ray/serve/_private/replica.py

+            # under initialization. Relying on DeploymentStateManager to
+            # timeout the replica.
+            while self.replica is None:
+                await asyncio.sleep(0.2)


Should this be a constant?

edoakes · 2023-04-03T17:10:51Z

python/ray/serve/_private/deployment_state.py

@@ -497,7 +497,10 @@ def check_ready(self) -> Tuple[ReplicaStartupStatus, Optional[DeploymentVersion]
                    self._allocated_obj_ref
                )
            except Exception:
-                logger.exception(f"Exception in deployment '{self._deployment_name}'")
+                logger.exception(
+                    f"Exception in replica '{self._replica_tag}',"


Suggested change

f"Exception in replica '{self._replica_tag}',"

f"Exception in replica '{self._replica_tag}', "

edoakes · 2023-04-03T17:12:05Z

python/ray/serve/_private/replica.py

+            # When the self.replica variable is None, potentially replica is still
+            # under initialization. Relying on DeploymentStateManager to
+            # timeout the replica.


I'm a little lost at why this change is needed. Is it because the constructor might still be running while we are recovering?

If so, can we use an explicit asyncio.Event instead of implementing our own sleeping logic here?

e.g., self._initialized = asyncio.Event()

I'm a little lost at why this change is needed. Is it because the constructor might still be running while we are recovering?

yes

super, I forget this Async variable! updated.

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

shrekris-anyscale

Nice work! Could you give some more detail on why this error appears when the controller is dead and a replica is initializing? I don't understand why that causes self.replica to be None.

python/ray/serve/_private/deployment_state.py

python/ray/serve/_private/replica.py

python/ray/serve/tests/test_controller_recovery.py

python/ray/serve/_private/replica.py

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

Co-authored-by: shrekris-anyscale <92341594+shrekris-anyscale@users.noreply.github.com> Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

python/ray/serve/_private/replica.py

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

shrekris-anyscale

Great work on the changes!

edoakes · 2023-04-04T15:38:09Z

@sihanwang41 a serve AIR integration test is failing, maybe related?

sihanwang41 · 2023-04-04T16:24:34Z

@edoakes it is not related, Cindy's pr #34039 also fails with same reason. looks like it is a separate issue.

sihanwang41 · 2023-04-04T16:58:21Z

^^ @zhe-thoughts observe this bug from the release test, here is the fix.

sihanwang41 · 2023-04-04T18:16:07Z

master is unfrozen, no need to have Zhe approval right now. ready to merge

Then replica is under initializing state and the controller is dead, user will see ``` (ServeController pid=458318) ERROR 2023-03-29 08:44:42,547 controller 458318 deployment_state.py:500 - Exception in deployment 'xqHWInctmH' (ServeController pid=458318) Traceback (most recent call last): (ServeController pid=458318) File "/home/ray/anaconda3/lib/python3.7/site-packages/ray/serve/_private/deployment_state.py", line 489, in check_ready (ServeController pid=458318) deployment_config, version = ray.get(self._ready_obj_ref) (ServeController pid=458318) File "/home/ray/anaconda3/lib/python3.7/site-packages/ray/_private/client_mode_hook.py", line 105, in wrapper (ServeController pid=458318) return func(*args, **kwargs) (ServeController pid=458318) File "/home/ray/anaconda3/lib/python3.7/site-packages/ray/_private/worker.py", line 2508, in get (ServeController pid=458318) raise value.as_instanceof_cause() (ServeController pid=458318) ray.exceptions.RayTaskError(AttributeError): ray::ServeReplica:xqHWInctmH.get_metadata() (pid=458148, ip=172.31.126.222, repr=<ray.serve._private.replica.ServeReplica:xqHWInctmH object at 0x7f57ae0f0110>) (ServeController pid=458318) File "/home/ray/anaconda3/lib/python3.7/concurrent/futures/_base.py", line 428, in result (ServeController pid=458318) return self.__get_result() (ServeController pid=458318) File "/home/ray/anaconda3/lib/python3.7/concurrent/futures/_base.py", line 384, in __get_result (ServeController pid=458318) raise self._exception (ServeController pid=458318) File "/home/ray/anaconda3/lib/python3.7/site-packages/ray/serve/_private/replica.py", line 249, in get_metadata (ServeController pid=458318) return self.replica.deployment_config, self.replica.version (ServeController pid=458318) AttributeError: 'NoneType' object has no attribute 'deployment_config' ``` - Fix the NoneType bug when recover happens, no matter the actor is under any state, user would not see the traceback. Instead user will see the 1. slow startup warning, and then replica will be terminated, and new replica will be provisioned. 2. If actor succeed in `PENDING_INITIALIZATION`, no error pops out. This is observed from long_running_serve_failure: https://console.anyscale-staging.com/o/anyscale-internal/projects/prj_qC3ZfndQWYYjx2cz8KWGNUL4/clusters/ses_u7xeve33e2djg9grgr9qcs9l4x?command-history-section=command_history **Note**: If user constructor initialization just needs very long time to finish, it is recommended to increase the `SERVE_SLOW_STARTUP_WARNING_S`, (if the user doesn't change, the deployment manager will terminate the old replica and start new replica after the timeout.) Co-authored-by: shrekris-anyscale <92341594+shrekris-anyscale@users.noreply.github.com>

Then replica is under initializing state and the controller is dead, user will see ``` (ServeController pid=458318) ERROR 2023-03-29 08:44:42,547 controller 458318 deployment_state.py:500 - Exception in deployment 'xqHWInctmH' (ServeController pid=458318) Traceback (most recent call last): (ServeController pid=458318) File "/home/ray/anaconda3/lib/python3.7/site-packages/ray/serve/_private/deployment_state.py", line 489, in check_ready (ServeController pid=458318) deployment_config, version = ray.get(self._ready_obj_ref) (ServeController pid=458318) File "/home/ray/anaconda3/lib/python3.7/site-packages/ray/_private/client_mode_hook.py", line 105, in wrapper (ServeController pid=458318) return func(*args, **kwargs) (ServeController pid=458318) File "/home/ray/anaconda3/lib/python3.7/site-packages/ray/_private/worker.py", line 2508, in get (ServeController pid=458318) raise value.as_instanceof_cause() (ServeController pid=458318) ray.exceptions.RayTaskError(AttributeError): ray::ServeReplica:xqHWInctmH.get_metadata() (pid=458148, ip=172.31.126.222, repr=<ray.serve._private.replica.ServeReplica:xqHWInctmH object at 0x7f57ae0f0110>) (ServeController pid=458318) File "/home/ray/anaconda3/lib/python3.7/concurrent/futures/_base.py", line 428, in result (ServeController pid=458318) return self.__get_result() (ServeController pid=458318) File "/home/ray/anaconda3/lib/python3.7/concurrent/futures/_base.py", line 384, in __get_result (ServeController pid=458318) raise self._exception (ServeController pid=458318) File "/home/ray/anaconda3/lib/python3.7/site-packages/ray/serve/_private/replica.py", line 249, in get_metadata (ServeController pid=458318) return self.replica.deployment_config, self.replica.version (ServeController pid=458318) AttributeError: 'NoneType' object has no attribute 'deployment_config' ``` - Fix the NoneType bug when recover happens, no matter the actor is under any state, user would not see the traceback. Instead user will see the 1. slow startup warning, and then replica will be terminated, and new replica will be provisioned. 2. If actor succeed in `PENDING_INITIALIZATION`, no error pops out. This is observed from long_running_serve_failure: https://console.anyscale-staging.com/o/anyscale-internal/projects/prj_qC3ZfndQWYYjx2cz8KWGNUL4/clusters/ses_u7xeve33e2djg9grgr9qcs9l4x?command-history-section=command_history **Note**: If user constructor initialization just needs very long time to finish, it is recommended to increase the `SERVE_SLOW_STARTUP_WARNING_S`, (if the user doesn't change, the deployment manager will terminate the old replica and start new replica after the timeout.) Co-authored-by: shrekris-anyscale <92341594+shrekris-anyscale@users.noreply.github.com> Signed-off-by: elliottower <elliot@elliottower.com>

Then replica is under initializing state and the controller is dead, user will see ``` (ServeController pid=458318) ERROR 2023-03-29 08:44:42,547 controller 458318 deployment_state.py:500 - Exception in deployment 'xqHWInctmH' (ServeController pid=458318) Traceback (most recent call last): (ServeController pid=458318) File "/home/ray/anaconda3/lib/python3.7/site-packages/ray/serve/_private/deployment_state.py", line 489, in check_ready (ServeController pid=458318) deployment_config, version = ray.get(self._ready_obj_ref) (ServeController pid=458318) File "/home/ray/anaconda3/lib/python3.7/site-packages/ray/_private/client_mode_hook.py", line 105, in wrapper (ServeController pid=458318) return func(*args, **kwargs) (ServeController pid=458318) File "/home/ray/anaconda3/lib/python3.7/site-packages/ray/_private/worker.py", line 2508, in get (ServeController pid=458318) raise value.as_instanceof_cause() (ServeController pid=458318) ray.exceptions.RayTaskError(AttributeError): ray::ServeReplica:xqHWInctmH.get_metadata() (pid=458148, ip=172.31.126.222, repr=<ray.serve._private.replica.ServeReplica:xqHWInctmH object at 0x7f57ae0f0110>) (ServeController pid=458318) File "/home/ray/anaconda3/lib/python3.7/concurrent/futures/_base.py", line 428, in result (ServeController pid=458318) return self.__get_result() (ServeController pid=458318) File "/home/ray/anaconda3/lib/python3.7/concurrent/futures/_base.py", line 384, in __get_result (ServeController pid=458318) raise self._exception (ServeController pid=458318) File "/home/ray/anaconda3/lib/python3.7/site-packages/ray/serve/_private/replica.py", line 249, in get_metadata (ServeController pid=458318) return self.replica.deployment_config, self.replica.version (ServeController pid=458318) AttributeError: 'NoneType' object has no attribute 'deployment_config' ``` - Fix the NoneType bug when recover happens, no matter the actor is under any state, user would not see the traceback. Instead user will see the 1. slow startup warning, and then replica will be terminated, and new replica will be provisioned. 2. If actor succeed in `PENDING_INITIALIZATION`, no error pops out. This is observed from long_running_serve_failure: https://console.anyscale-staging.com/o/anyscale-internal/projects/prj_qC3ZfndQWYYjx2cz8KWGNUL4/clusters/ses_u7xeve33e2djg9grgr9qcs9l4x?command-history-section=command_history **Note**: If user constructor initialization just needs very long time to finish, it is recommended to increase the `SERVE_SLOW_STARTUP_WARNING_S`, (if the user doesn't change, the deployment manager will terminate the old replica and start new replica after the timeout.) Co-authored-by: shrekris-anyscale <92341594+shrekris-anyscale@users.noreply.github.com> Signed-off-by: Jack He <jackhe2345@gmail.com>

sihanwang41 force-pushed the recover_failure branch from 869c691 to 46b32f7 Compare March 29, 2023 22:51

sihanwang41 changed the title ~~[Serve] Controller recover initializing replica~~ [Serve] Raise proper exception when replica recover from initializing Mar 29, 2023

sihanwang41 force-pushed the recover_failure branch 3 times, most recently from 0c7e38b to 7034d32 Compare March 30, 2023 03:24

[Serve] Recover PENDING_INITIALIZATION status actor & Fix the actor r…

7221899

…ecover bug Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

sihanwang41 force-pushed the recover_failure branch from 7034d32 to 7221899 Compare March 30, 2023 04:45

sihanwang41 changed the title ~~[Serve] Raise proper exception when replica recover from initializing~~ [Serve] Recover PENDING_INITIALIZATION status actor Mar 30, 2023

sihanwang41 added 4 commits March 29, 2023 21:54

Fix

acab5fb

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

Fix lint

22e131d

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

Fix

60ed64a

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

Fix

35fbe30

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

sihanwang41 marked this pull request as ready for review March 30, 2023 18:24

sihanwang41 assigned edoakes, zcin and shrekris-anyscale Mar 30, 2023

sihanwang41 mentioned this pull request Mar 31, 2023

[serve] release test failure: long_running_serve_failure #33868

Closed

zcin reviewed Mar 31, 2023

View reviewed changes

edoakes reviewed Apr 3, 2023

View reviewed changes

sihanwang41 added 2 commits April 3, 2023 11:08

Address comments

9ae9b89

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

Fix

10d6300

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

shrekris-anyscale reviewed Apr 3, 2023

View reviewed changes

sihanwang41 and others added 7 commits April 3, 2023 12:04

update test

f8d090c

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

Update python/ray/serve/_private/replica.py

58e1588

Co-authored-by: shrekris-anyscale <92341594+shrekris-anyscale@users.noreply.github.com> Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

Update python/ray/serve/tests/test_controller_recovery.py

8eb29df

Co-authored-by: shrekris-anyscale <92341594+shrekris-anyscale@users.noreply.github.com> Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

Update python/ray/serve/_private/replica.py

1d68fe2

Co-authored-by: shrekris-anyscale <92341594+shrekris-anyscale@users.noreply.github.com> Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

Update python/ray/serve/tests/test_controller_recovery.py

02c1ce9

Co-authored-by: shrekris-anyscale <92341594+shrekris-anyscale@users.noreply.github.com> Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

Address comments

04f907d

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

Address comments

b692548

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

edoakes reviewed Apr 3, 2023

View reviewed changes

python/ray/serve/_private/replica.py Outdated Show resolved Hide resolved

edoakes approved these changes Apr 3, 2023

View reviewed changes

Address comments

940f8e3

Signed-off-by: Sihan Wang <sihanwang41@gmail.com>

shrekris-anyscale approved these changes Apr 4, 2023

View reviewed changes

zcin approved these changes Apr 4, 2023

View reviewed changes

sihanwang41 closed this Apr 4, 2023

sihanwang41 reopened this Apr 4, 2023

edoakes merged commit b9f3b8d into ray-project:master Apr 4, 2023

sihanwang41 mentioned this pull request Apr 4, 2023

[CherryPick][Serve] Recover PENDING_INITIALIZATION status actor (#33890) #34063

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Serve] Recover PENDING_INITIALIZATION status actor #33890

[Serve] Recover PENDING_INITIALIZATION status actor #33890

sihanwang41 commented Mar 29, 2023 •

edited

Loading

zcin Mar 31, 2023

zcin Mar 31, 2023

sihanwang41 Apr 3, 2023

zcin Apr 3, 2023 •

edited

Loading

zcin Mar 31, 2023

sihanwang41 Apr 3, 2023

edoakes Apr 3, 2023

edoakes Apr 3, 2023

edoakes Apr 3, 2023

edoakes Apr 3, 2023

sihanwang41 Apr 3, 2023 •

edited

Loading

shrekris-anyscale left a comment

shrekris-anyscale left a comment

edoakes commented Apr 4, 2023

sihanwang41 commented Apr 4, 2023

sihanwang41 commented Apr 4, 2023

sihanwang41 commented Apr 4, 2023

	f"Exception in replica '{self._replica_tag}',"
	f"Exception in replica '{self._replica_tag}', "

[Serve] Recover PENDING_INITIALIZATION status actor #33890

[Serve] Recover PENDING_INITIALIZATION status actor #33890

Conversation

sihanwang41 commented Mar 29, 2023 • edited Loading

Why are these changes needed?

Related issue number

Checks

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zcin Apr 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sihanwang41 Apr 3, 2023 • edited Loading

Choose a reason for hiding this comment

shrekris-anyscale left a comment

Choose a reason for hiding this comment

shrekris-anyscale left a comment

Choose a reason for hiding this comment

edoakes commented Apr 4, 2023

sihanwang41 commented Apr 4, 2023

sihanwang41 commented Apr 4, 2023

sihanwang41 commented Apr 4, 2023

sihanwang41 commented Mar 29, 2023 •

edited

Loading

zcin Apr 3, 2023 •

edited

Loading

sihanwang41 Apr 3, 2023 •

edited

Loading