[nataliekung]

[core][gcs] Do not FATAL when an actor is missing its task spec on GCS init

Why are these changes needed?

When GCS fault tolerance is enabled with external Redis,
GcsActorManager::Initialize() iterates the actor table and, for every actor it
decides to load, looks up its task spec with map_find_or_die(). If the spec is
missing, that helper aborts the whole gcs_server:

[C] (gcs_server) container_util.h:163: Key <actor_id_hex> doesn't exist
RuntimeError: Failed to start GCS.

The head then CrashLoopBackOffs, and because GCS owns cluster-level metadata,
the entire shared cluster is down until an operator manually resets GCS
state. A single dangling actor record takes down the whole cluster with no
self-heal.

Root cause

The task spec (@ACTOR_TASK_SPEC) is always persisted before the actor-table
row (@ACTOR), so the normal write ordering never leaves a loadable actor
without a spec. The only way an actor becomes non-DEAD with a missing spec is
Initialize() itself:

• When an actor should not be loaded (for example, its owning job or root
  detached actor is dead), Initialize() de-selects it, appends it to
  dead_actors, and later ActorTaskSpecTable().BatchDelete(dead_actors)
  removes its task spec.
• Previously the actor-table row was left in its original non-DEAD state
  (e.g. DEPENDENCIES_UNREADY, ALIVE).
• On the next restart that row is reloaded as non-DEAD, but its task spec is
  already gone, so map_find_or_die() aborts GCS.

What is changed?

1. OnInitializeActorShouldLoad() looks up the task spec with find() and
   returns false when it is missing, so Initialize() isolates such a row
   instead of aborting the whole GCS with a fatal lookup.
2. When Initialize() de-selects an actor that is not already permanently DEAD,
   it now marks the actor DEAD, persists the row, publishes the death, and
   only then deletes its task spec — from inside the ActorTable().Put()
   completion callback, mirroring DestroyActor(). Ordering the spec delete
   after the DEAD row is durable means a crash in between can leave at most a
   DEAD row (survivable; it converges on the next restart), never a non-DEAD row
   without a spec. The trigger is simply !actor_is_dead_and_not_restartable,
   so every de-selected actor about to lose its spec is made consistent first.
   Already-permanently-dead de-selected actors keep the existing behavior: their
   expected-absent spec is cleaned up by the end-of-loop batch delete.

Effects:

• GCS starts successfully; only the de-selected actors are isolated, all other
  actors load normally.
• The dangling actor-table row is persisted as DEAD before its spec is deleted,
  so it reloads as DEAD (never as a non-DEAD row with a missing spec) on later
  restarts.
• Any client still waiting on the actor (its owner may still be alive after a
  GCS failover) is unblocked with a death cause instead of hanging.

Risk

Low and localized:

• Only actors that are de-selected at init (owner dead, or a pre-existing
  dangling non-DEAD row with no spec) change behavior; the normal load path
  (spec present, owner alive) is unchanged.
• A de-selected actor cannot be reconstructed anyway, so marking it DEAD is the
  correct terminal state — strictly better than aborting GCS or leaving an
  inconsistent row.
• Routes through the existing "should not load" branch; no new state-machine
  paths are introduced.

Testing

Added C++ unit tests in gcs_actor_manager_test.cc:

• TestInitializeMarksDeadWhenOwnerJobDeadAndReloads — the real root-cause
  path: an actor whose owning job is dead but whose task spec is still present.
  Asserts Initialize() marks it DEAD, persists the DEAD row, and deletes its
  task spec from storage; then a fresh GcsActorManager initialized from the
  persisted state does not abort, still reports the actor DEAD, and does not
  re-register it.
• TestInitializeIsolatesActorWithMissingTaskSpec — the find() guard: a
  non-DEAD actor whose task spec is already gone does not abort Initialize()
  and is isolated as DEAD (reloadable from storage).

Run them with:

bazel test //src/ray/gcs/actor/tests:gcs_actor_manager_test --test_output=all

To confirm the guard actually prevents the crash, revert only the
gcs_actor_manager.cc hunk (keep the tests) and re-run: Initialize() aborts
with container_util.h: Key <actor_id> doesn't exist. With the fix applied the
tests pass.

[gemini-code-assist]

Code Review

This pull request prevents GCS startup crashes caused by missing actor task specifications (e.g., due to torn writes or head crashes during actor teardown). Instead of calling map_find_or_die and aborting, OnInitializeActorShouldLoad now safely handles missing task specs by logging an error and treating the actor as dead (not loadable). A regression test has been added to verify this behavior. I have no feedback to provide.

[cursor]

Stale Bugbot comment from a previous run.

[cursor]

PR needs clearer description

Low Severity

⚠️ This PR needs a clearer title and/or description.

This violates the Clear PR Descriptions and Titles rule. The description is blank and does not explain what problem is present or how it is fixed.

To help reviewers, please ensure your PR includes:

• Title: A concise summary of the change
• Description:
  • What problem does this solve?
  • How does this PR solve it?
  • Any relevant context for reviewers such as:
    • Why is the problem important to solve?
    • Why was this approach chosen over others?

See this list of PRs as examples for PRs that have gone above and beyond:

• [Core] Introduce local port service discovery  #59613
• [Core] Improve Large-Scale Resource View Synchronization Through Sync Message Batching #57641
• Remove node observability information from hot path of core components #56474
• [core][rdt] Support out-of-order actors by extracting metadata when creating #59610
• [core] fix open leak for plasma store memory (shm/fallback) by workers #52622

 

^{Triggered by project rule: Bugbot Rules}

^{Reviewed by Cursor Bugbot for commit 994bb95. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

^{Reviewed by Cursor Bugbot for commit 1051b31. Configure here.}

[nataliekung]

Thanks for the review. Addressing the Bugbot "medium" finding (isolated actors remained non-DEAD):

Previously, an actor whose task spec was missing on GCS init was skipped from loading and parked in the destroyed-actor cache with its original non-DEAD state, and no death was published. If the owner was still alive after a GCS failover, GetActorInfo kept reporting the actor as e.g. DEPENDENCIES_UNREADY, and callers waiting on the actor could hang indefinitely.

The updated commit now actively resolves the actor during Initialize instead of only skipping it:

• It marks the actor DEAD and attaches an ActorDeathCause (ActorDiedErrorContext) whose message explains that the task spec is missing because the actor table and actor task spec table were left inconsistent (torn write or a crash during teardown).
• It persists the DEAD state to the actor table, so the dangling row self-heals across restarts, and publishes the death via gcs_publisher_->PublishActor(...) so any client still waiting on the actor unblocks instead of hanging.
• The row is cached as a destroyed actor with the DEAD state, so GetActorInfo now reports it as dead.

The regression test was updated accordingly: after Initialize it now asserts the actor is reported DEAD via the actor-info path (with a populated death cause), not merely absent from the live set. The RAY_LOG(ERROR) recording the inconsistency is retained.

[rueian]

Hi @nataliekung, could you sign off on your commit to make the DCO check pass?

[rueian]

Does this actually reload state from the storage?

[nataliekung]

Good catch — it didn't. The old assertion only read the in-memory observability cache. I've extended the test to drain the io_context (flushing the async ActorTable().Put) and then read the row back via gcs_table_storage_->ActorTable().Get, asserting it reloads as DEAD with the death cause. That verifies the DEAD state is actually persisted, which is what lets the dangling row self-heal on later restarts.

[rueian]

Why don't we also make it return false here if we don't find the task spec?

[nataliekung]

Done. OnInitializeActorShouldLoad now looks the task spec up with find() and returns false when it's missing, instead of map_find_or_die(), so the helper decides "should this actor load?" on its own and can never FATAL.

Initialize then uses that false to route the actor into the not-loaded branch, where — if the spec is missing for an otherwise-loadable actor — it marks the row DEAD, persists it, and publishes the death (so a client waiting on it whose owner is still alive after a GCS failover unblocks). I dropped the earlier top-of-loop pre-check, so the missing-spec handling now lives in one place, driven by the helper's return value.

[rueian]

I think your fix is correct, but this comment is not true currently.

Yes, the actor table and the actor task spec table are written as separate, non-atomic entries, but I think we currently avoid the missing task spec by writing the task spec first, then writing the actor table. The only exception is the above dead_actors.push_back(actor_id) during the GCS Initialization. That is why your fix, marking dead actors as actually DEAD in the table, works. Could you make the comments accurate? In the logs and error messages, we can just not mention "so a torn write across the two or a head crash during actor teardown..."

[nataliekung]

You're right, thanks — the comment, log, and death cause were inaccurate, and I've corrected them.

The task spec is always written before the actor-table row, so the normal write ordering never leaves a loadable actor without its spec. The only way an actor becomes non-DEAD with a missing spec is this de-selection path in Initialize() itself: when an actor should not be loaded (for example, its owner is dead), it is appended to dead_actors and its task spec is later BatchDeleted, but the actor-table row was previously left in its non-DEAD state. On the next restart that row is reloaded as non-DEAD with its spec already gone, and map_find_or_die() aborts GCS — which is exactly why marking the row DEAD fixes it.

I removed the "torn write" / "crash during actor teardown" wording from the code comment, the log message, the death-cause error_message (also renamed the helper to GenActorNotLoadableAtInitCause), the commit message, and the PR description, and rewrote them to describe this de-selection mechanism.

[rueian]

do we need the !actor_task_specs.contains(actor_id) check? Isn't !actor_is_dead_and_not_restartable enough?

[nataliekung]

Agreed — dropped the !actor_task_specs.contains(actor_id) check. The trigger is now just !actor_is_dead_and_not_restartable.

Every de-selected actor that is not already permanently DEAD is about to have its task spec BatchDeleted, so leaving its row non-DEAD is exactly what creates the future FATAL. Marking all such actors DEAD, persisting the row, and publishing the death before the spec is deleted proactively prevents the landmine instead of only reacting once the spec is already gone.

[rueian]

@nataliekung, please fix these build errors:

[2026-09-21T19:10:16Z] # Execution platform: @@internal_platforms_do_not_use//host:host
--
  | [2026-09-21T19:10:16Z] src/ray/gcs/actor/gcs_actor_manager.cc:1872:21: error: 'actor_id' in capture list does not name a variable
  | [2026-09-21T19:10:16Z]             {[this, actor_id, observability_data](Status status) {
  | [2026-09-21T19:10:16Z]                     ^
  | [2026-09-21T19:10:16Z] src/ray/gcs/actor/gcs_actor_manager.cc:1874:20: error: reference to local binding 'actor_id' declared in enclosing function 'ray::gcs::GcsActorManager::Initialize'
  | [2026-09-21T19:10:16Z]                    actor_id, GenActorDataOnlyWithStates(observability_data));
  | [2026-09-21T19:10:16Z]                    ^
  | [2026-09-21T19:10:16Z] src/ray/gcs/actor/gcs_actor_manager.cc:1784:21: note: 'actor_id' declared here
  | [2026-09-21T19:10:16Z]   for (const auto &[actor_id, actor_table_data] : gcs_init_data.Actors()) {
  | [2026-09-21T19:10:16Z]                     ^
  | [2026-09-21T19:10:16Z] src/ray/gcs/actor/gcs_actor_manager.cc:1878:20: error: reference to local binding 'actor_id' declared in enclosing function 'ray::gcs::GcsActorManager::Initialize'
  | [2026-09-21T19:10:16Z]                    actor_id, {[](auto) {}, io_context_});
  | [2026-09-21T19:10:16Z]                    ^
  | [2026-09-21T19:10:16Z] src/ray/gcs/actor/gcs_actor_manager.cc:1784:21: note: 'actor_id' declared here
  | [2026-09-21T19:10:16Z]   for (const auto &[actor_id, actor_table_data] : gcs_init_data.Actors()) {
  | [2026-09-21T19:10:16Z]                     ^
  | [2026-09-21T19:10:16Z] 3 errors generated.

[nataliekung]

Thanks for catching this. Root cause: in GcsActorManager::Initialize, actor_id comes from the range-for structured binding for (const auto &[actor_id, actor_table_data] : ...), and I captured it by name in the ActorTable().Put(...) completion callback. Capturing a structured binding in a lambda is a GCC extension that clang (the CI --build-type clang toolchain) rejects, which is why the GCC build was green but premerge failed.

Fixed by switching that lambda to an init-capture that copies real variables — [this, actor_id = actor_id, observability_data = observability_data] — so nothing captures the binding directly; the nested task-spec Delete callback captures nothing. No behavior change.

Verified the target now builds and the test passes under clang + libc++ (--config=llvm). Repushed as ee79093d.