[CORE] Filters bad subscriber messages from taking down GCS publisher#60252
Merged
Conversation
Signed-off-by: davik <davik@anyscale.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request effectively addresses a critical issue where an invalid channel in a subscription request could take down the GCS publisher. By changing RegisterSubscription to return a Status, the error can be gracefully handled by the GCS and propagated to the subscriber. The subscriber is correctly updated to treat this as a fatal error, which is appropriate for an internal system inconsistency. The changes are well-contained, and the addition of new unit tests ensures the new error-handling logic is robust. Overall, this is a solid improvement for cluster stability.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Kunchen (David) Dai <54918178+Kunchd@users.noreply.github.com>
Kunchd
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ZacAttack
reviewed
Jan 20, 2026
…er pubsub Signed-off-by: davik <davik@anyscale.com>
Signed-off-by: davik <davik@anyscale.com>
ZacAttack
reviewed
Jan 26, 2026
ZacAttack
reviewed
Jan 26, 2026
ZacAttack
reviewed
Jan 26, 2026
ZacAttack
reviewed
Jan 26, 2026
Signed-off-by: davik <davik@anyscale.com>
ZacAttack
reviewed
Jan 26, 2026
| return status; | ||
| } | ||
|
|
||
| RAY_CHECK(sub_message.has_worker_object_eviction_message() || |
Contributor
There was a problem hiding this comment.
mmmm... My gut is telling me that it'd be sure nice if we had a utility function or something for this. Idea being, if we add more sub_message types in the future it would 'hopefully?' be obvious that we should update the list of message types? idk. Admittedly this is no worse then what we had. So maybe this is ok. What do you think?
Contributor
Author
There was a problem hiding this comment.
We chatted offline, and concluded this should go into a separate undertaking that defines a distinct proto API per pubsub system.
ZacAttack
approved these changes
Jan 26, 2026
ZacAttack
left a comment
ZacAttack
left a comment
Contributor
There was a problem hiding this comment.
Overall this looks good to me. Left one optional comment.
Contributor
Author
|
@dayshah Could you merge this for me when you get the chance? Thanks |
dayshah
reviewed
Jan 27, 2026
dayshah
left a comment
dayshah
left a comment
Contributor
There was a problem hiding this comment.
wait what other ways are there for a bad subscriber message to come in other than version mismatch?
Also the status invalid will get returned in the reply, but how does the receiver handle that? I feel like this could be a real invariant in the system and the cluster should die if this happens
israbbani
reviewed
Jan 27, 2026
Signed-off-by: davik <davik@anyscale.com>
ZacAttack
reviewed
Jan 29, 2026
ZacAttack
reviewed
Jan 29, 2026
ZacAttack
reviewed
Jan 29, 2026
ZacAttack
approved these changes
Jan 29, 2026
ZacAttack
left a comment
ZacAttack
left a comment
Contributor
There was a problem hiding this comment.
Ok, aside from some nits, looks good.
Signed-off-by: davik <davik@anyscale.com>
Signed-off-by: davik <davik@anyscale.com>
elliot-barn
pushed a commit
that referenced
this pull request
Feb 9, 2026
…#60252) ## Description Issue #58964 reported that it's possible that the GCS publisher may receive an invalid channel causing the cluster to be taken down. From investigation, we suspect this mostly likely sprung up due to a misconfiguration in the local environment of the subscriber (e.g. ray version mismatch). This PR Modify publisher reject and reply to messages with bad arguments. The subscriber will fail upon receiving a bad argument reply as pubsub should be internal and bad arguments indicate potential bugs in the system. However, we do not take down the cluster in this case in case of a local environment misconfiguration. ## Related issues #58964 ## Additional information --------- Signed-off-by: davik <davik@anyscale.com> Signed-off-by: Kunchen (David) Dai <54918178+Kunchd@users.noreply.github.com> Co-authored-by: davik <davik@anyscale.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
elliot-barn
pushed a commit
that referenced
this pull request
Feb 9, 2026
…#60252) ## Description Issue #58964 reported that it's possible that the GCS publisher may receive an invalid channel causing the cluster to be taken down. From investigation, we suspect this mostly likely sprung up due to a misconfiguration in the local environment of the subscriber (e.g. ray version mismatch). This PR Modify publisher reject and reply to messages with bad arguments. The subscriber will fail upon receiving a bad argument reply as pubsub should be internal and bad arguments indicate potential bugs in the system. However, we do not take down the cluster in this case in case of a local environment misconfiguration. ## Related issues #58964 ## Additional information --------- Signed-off-by: davik <davik@anyscale.com> Signed-off-by: Kunchen (David) Dai <54918178+Kunchd@users.noreply.github.com> Co-authored-by: davik <davik@anyscale.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
ans9868
pushed a commit
to ans9868/ray
that referenced
this pull request
Feb 18, 2026
…ray-project#60252) ## Description Issue ray-project#58964 reported that it's possible that the GCS publisher may receive an invalid channel causing the cluster to be taken down. From investigation, we suspect this mostly likely sprung up due to a misconfiguration in the local environment of the subscriber (e.g. ray version mismatch). This PR Modify publisher reject and reply to messages with bad arguments. The subscriber will fail upon receiving a bad argument reply as pubsub should be internal and bad arguments indicate potential bugs in the system. However, we do not take down the cluster in this case in case of a local environment misconfiguration. ## Related issues ray-project#58964 ## Additional information --------- Signed-off-by: davik <davik@anyscale.com> Signed-off-by: Kunchen (David) Dai <54918178+Kunchd@users.noreply.github.com> Co-authored-by: davik <davik@anyscale.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Adel Nour <ans9868@nyu.edu>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Issue #58964 reported that it's possible that the GCS publisher may receive an invalid channel causing the cluster to be taken down. From investigation, we suspect this mostly likely sprung up due to a misconfiguration in the local environment of the subscriber (e.g. ray version mismatch).
This PR Modify publisher reject and reply to messages with bad arguments. The subscriber will fail upon receiving a bad argument reply as pubsub should be internal and bad arguments indicate potential bugs in the system. However, we do not take down the cluster in this case in case of a local environment misconfiguration.
Related issues
#58964
Additional information