[karticam]

This is PR 7 of a series of PRs to implement move semantics for plasma objects in core.

The complete design doc is here: https://docs.google.com/document/d/1ZJhgRmwVyKfNCR21CWf6xMcc0B6HJwSPQsfebTRvnnE/edit?tab=t.0#heading=h.7wcod1f9afpg

After #67093 and #67133, the consumer's node pins a moved object as its new primary copy and reports it to the owner. With this PR the owner frees the previous primary's copy when it handles that report, so a move ends with one copy.

Changes

1. The owner frees the previous primary's copy. The new ReferenceCounter::UpdateObjectPrimaryMoved records the new primary and returns the previous primary's node. HandleUpdateObjectLocationBatch passes that node to FreeObjectOnNodesAsync, which sends a FreeLocalObjects request to its raylet, and that raylet unpins the copy and deletes it, spill file included. Nothing is freed when:

   • there is no previous primary: recovery is in flight;
   • the previous primary is the new node: the owner handles the same update twice, which happens when the raylet resends a location batch after a lost reply;
   • the new node is already dead: the previous primary was never freed, so it still pins the object and stays the primary.

2. Recovery skips a copy that is being freed. If the consumer's node dies while the free is on its way to the producer's node, recovery could pin the producer's copy just before the free deletes it. The owner would then record a primary that no longer exists, and the object would be lost without an error. Each Reference now has locations_being_freed, and the new GetObjectLocationsForRecovery returns the locations minus those nodes. Only recovery's lookup uses it; every other reader still sees all locations. A node leaves the set when it reports the copy removed or when it dies, and the set is cleared when the object goes out of scope. The batched free has no per-object reply and fails only when the node is dead, so node death also covers a failed free.

3. Spill reports of a moved object. For a while both the producer's and the consumer's nodes pin a moved object, and both may spill it, but the owner keeps one spill location. If the producer's node spills the object just before the free reaches it, its spill report can reach the owner after the move. The owner would then record a spill file that the free is about to delete. So:

   • HandleObjectSpilled takes the reporting node, which the owner knows from the batch even for a spill to external storage, and records a moved object's spill only from the node that holds its primary copy. While recovery is in flight, it ignores only a node whose copy is being freed.
   • The primary-moved update is handled before a spill report in the same update, since the consumer's node can spill the object before its update leaves.
   • When the previous primary is freed, its recorded spill is dropped, since the free deletes the spill file too.

   Objects that are never moved are unaffected.

[gemini-code-assist]

Code Review

This pull request implements a mechanism to free the previous primary copy of an object once its primary copy has moved to a consumer node. It introduces tracking of locations_being_freed to prevent recovery from pinning copies that are being deleted, and ensures that object spilling is only recorded from the node holding the primary copy. The review feedback identifies a valid issue where a node might remain in locations_being_freed if an object is moved back to a previous primary node before its removal report is processed, and suggests explicitly erasing the node from locations_being_freed when it becomes the primary copy again.

[gemini-code-assist]

If an object is moved back to a node that was previously a primary (e.g., in a sequence of moves like A -> B -> A), the node node_id might still reside in locations_being_freed if the removal report from the first move was delayed or lost. If node_id remains in locations_being_freed while it is the active primary, GetObjectLocationsForRecovery will incorrectly filter it out, potentially leaving recovery with no valid locations.\n\nTo prevent this, we should explicitly erase node_id from locations_being_freed when it becomes the primary copy again.

  const std::optional<NodeID> previous_primary = it->second.pinned_at_node_id_;\n  UpdateOwnedObjectCounters(object_id, it->second, /*decrement=*/true);\n  it->second.pinned_at_node_id_ = node_id;\n  it->second.locations_being_freed.erase(node_id);\n  std::optional<NodeID> node_to_free;

[cursor]

Stale Bugbot comment from a previous run.

[cursor]

Missing Doxygen on modified function

Low Severity

⚠️ Document functions and classes with Doxygen /** ... */ block comments using @ tags (@brief, and @param/@return where applicable).

 

^{Triggered by project rule: Bugbot Rules}

^{Reviewed by Cursor Bugbot for commit 200a5a9. Configure here.}

[cursor]

Stale Bugbot comment from a previous run.

[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).

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 7bdb8ca. Configure here.}

[cursor]

Freed spill node stuck forever

Medium Severity

A node enters locations_being_freed on move and leaves only on a later plasma REMOVED, node death, or the object going out of scope. A producer copy that was already spilled has already reported REMOVED, and deleting the spill file does not send another one. That node then stays excluded from GetObjectLocationsForRecovery for the rest of the object's life, so a later copy there cannot be pinned if the new primary is lost.

Additional Locations (1)

• src/ray/core_worker/reference_counter.cc#L1571-L1575

 

^{Reviewed by Cursor Bugbot for commit 7bdb8ca. Configure here.}