[Kunchd]

Description

Through a recent investigation in object fetch time out errors, we have uncovered a scenario where lineage reconstruction may fail to trigger due to a race between node death notification and object spilled notification.

Currently when objects are spilled, a spill notification is sent to the owner. However, an out of date spill notification for a dead node can actually corrupt the owner's reference counting state, leading to lineage reconstruction failing to trigger when the object is lost further down the line.

Consider the following diagram:

At a high level, the following sequence occurs.

1. A produced object is spilled on the first executing node, and a message is sent to the object owner to notify it about the spill. This message is delayed.
2. The node shortly after goes down, and the GCS picks up on this.
3. The GCS then sends a signal and is received by the object owner. The owner upon learning about the node death, clears the object's tracking metadata and trigger lineage reconstruction.
4. Reconstruction executes on another node and eventually completes.
5. At this point the spill notification finally arrives. The mechanisms that currently protects the reference counter from stale spill messages actually sets spilled to true even though the node is dead. Additionally, the new pinned object location is also cleared via UnsetObjectPrimaryCopy, corrupting the owner's state. (Eluded in the diagram for cleanliness, but at this point another reconstruction attempt is triggered. This reconstruction attempt sees that the spilled flag is true and assumes the object is still within the cluster so it stops).
6. The reconstructed object is lost again via node death.
7. The GCS again notify the owner about the node death. However, this time, when the owner attempts to trigger reconstruction this time around, because its state is corrupted (no pinned primary copy or spilled node id), no reconstruction is ever triggered.

This ultimately results in ObjectFetchTimeOut error.

What this PR does

This PR introduces a check within HandleObjectSpilled that make sure we only update the spilled object metadata if the node we spilled is actually still alive, avoiding corrupting the owner state with the spill information of a dead node.

Related issues

Additional information

[gemini-code-assist]

Code Review

This pull request addresses an issue where a stale spill report from a dead node could poison an object's location state and prevent future reconstructions. It ensures that the spilled flag is only set if the spilled location is alive, and only clears the pinned copy if the spilled node matches the pinned node. A regression test has been added to verify this behavior. The reviewer suggested a performance improvement to avoid redundant recovery attempts by only pushing the object to objects_to_recover_ when the pinned copy is actually cleared.

[cursor]

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

^{Reviewed by Cursor Bugbot for commit 139da33. Configure here.}

[github-actions]

This pull request has been automatically marked as stale because it has not had
any activity for 14 days. It will be closed in another 14 days if no further activity occurs.
Thank you for your contributions.

You can always ask for help on our discussion forum or Ray's public slack channel.

If you'd like to keep this open, just leave any comment, and the stale label will be removed.