[core] Resolve the race condition between reference counting GC and actor creation

[kevin85421]

Why are these changes needed?

Problem statement

import ray

@ray.remote
class Actor:
    pass

ray.init()

for i in range(0, 10000):
    print(i)
    Actor.options(
        name="actor",
        get_if_exists=True,
        max_restarts=-1,
    ).remote()

In the above script, Actor.remote() with get_if_exists=True is called multiple times, and the actor handle goes out of scope immediately. When get_if_exists is set to True, the process can be briefly divided into three steps. See this function for more details:

• Step 1: Ray calls get_actor to retrieve the actor if it exists.
• Step 2: If the actor does not exist, Ray attempts to call _remote to create a new actor.
• Step 3: If step 2 fails to create a new actor, Ray calls get_actor again.

Next, we need to understand how get_actor and actor creation work.

• get_actor: The most important function is ActorManager::GetNamedActorHandle. First, the actor manager checks whether the named actor's information exists in the local cache. If not, it calls the SyncGetByName function to query the GCS for information about the named actor. If the named actor is not found in either the actor manager's cache or the GCS, an error status is returned, and an exception is thrown in Python.

• actor creation: The most important function is GcsActorManager::RegisterActor.

The bug happens in the following case:

• (Step 1-1) The core worker cannot find the named actor in the cached_actor_name_to_ids_ cache because when the actor handle goes out of scope and the reference count of the actor reaches 0, the named actor is removed from cached_actor_name_to_ids_ by a callback function.
• (Step 1-2) Because the core worker can't find the named actor in the cached_actor_name_to_ids_, it will call SyncGetByName to ask GCS.
• (Step 1-3) GCS uses GcsActorManager::HandleGetNamedActorInfo to handle SyncGetByName. GCS returns Status::NotFound to the core worker under the following conditions:
  • The named actor exists in registered_actors_ and named_actors, but its ActorState is rpc::ActorTableData::DEAD.
• (Step 1-4) The core worker receives Status::NotFound from the GCS and returns Status::NotFound, causing an exception to be raised in Python.
• (Step 2-1) actor.py catches the exception, and then calls self._remote to create an actor.
• (Step 2-2) GcsActorManager::RegisterActor: The request attempts to create a new actor with the same name and namespace, but a different actor ID is generated. As a result, the old named actor's actor ID remains in registered_actors_, and this request does not enter the if statement.
• (Step 2-3) GcsActorManager::RegisterActor: Next, name and namespace are used to look up named_actors, and an error is returned because the old / new named actors share the same name and namespace. Hence, the actor creation failed.
• (Step 3) Call get_actor again, but it will fail.
• The GCS finally receives the notification about the actor going out of scope and then calls DestroyActor to remove the old named actor from registered_actors_ and named_actors.

To summarize, the bug occurs because the GCS has a different understanding of whether an actor exists during "get actor" and create actor operations.

• get_actor: The GCS considers an actor nonexistent if it is not in registered_actors_, not in named_actors, or if the actor's state is DEAD. See this line for more details.
• actor creation: The GCS considers an actor to exist if it is present in named_actors.

The differing definitions of "exist" cause the bug when the GCS fails to detect an actor going out of scope in time and calls DestroyActor to update registered_actors_ and named_actors.

Solution

The goal of the solution is to ensure the GCS has a consistent understanding of whether an actor "exists," regardless of when users attempt to "get" or "create" an actor.

The solution is that the GCS must ensure registered_actors_ and named_actors are updated if it plans to return an error status to the core worker in (Step 1-3). In this PR, we call DestroyActor to update registered_actors_ and named_actors before returning the status to the core worker.

Related issue number

Closes #48856

Checks

pytest -vs ray/tests/test_get_or_create_actor.py::test_get_or_create_named_actor

• Without this PR, it failed all 10 times out of 10 runs.
• With this PR, it passed all 10 times out of 10 runs.

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

[dentiny]

Curious how can I reproduce the TSAN failure?

[kevin85421]

@dentiny I wrote a detailed PR description. We can chat in-person if you have further questions.

[rynewang]

I think this relates to a similar actor refcnt issue, that I found 2023, that actually dates back to a very old unresolved issue at ~2019... but I can't find that issue. @jjyao do you have memories on actor refcount issues?

[jjyao]

After #49480 an actor has three states:

1. ALIVE
2. DEAD and non-restartable: not in registered_actors_ and named_actors_
3. DEAD and restartable: in registered_actors_ and named_actors_

The semantic is clear for get_if_exist if the actor is in state 1 or 2. What we need to figure out is the behavior when the actor is in state 3.

[jjyao]

One option is to bring it back to state 1.