[History Server] Treat suspended RayCluster as dead in isDead - #5069
Open
yjaw wants to merge 3 commits into
Open
[History Server] Treat suspended RayCluster as dead in isDead#5069yjaw wants to merge 3 commits into
yjaw wants to merge 3 commits into
Conversation
A suspended RayCluster keeps its CR but deletes all Pods, so proxying to the head service fails. Check spec.suspend in isDead so sessions fall back to stored historical data instead. Close ray-project#5062
…spend spec.Suspend flips true the instant a user requests suspension, well before Pods are actually terminated. isDead would mark the session dead and cache its snapshot immediately, racing the collector's SIGTERM flush and permanently caching incomplete data. RayClusterSuspended is only set once the controller observes zero running Pods, so the flush window has already passed by the time we treat the session as dead and cacheable.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
Reviewed by Cursor Bugbot for commit 7eb847e. Configure here.
…Session isDead alone wasn't enough: listClusters and resolveSession independently treat any existing RayCluster CR as live, bypassing isDead entirely. Since resolveSession backs the primary /enter_cluster entry point (including the default "latest" resolution), a suspended cluster was still shown as live and still proxied to a head Pod that no longer exists. Extract isRayClusterSuspended as a shared helper and apply it in both places so a suspended cluster falls back to stored session data instead.
Member
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
2 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.
Why are these changes needed?
When a RayCluster is suspended (
spec.suspend = true), KubeRay deletes all Pods (including the head Pod) but keeps the RayCluster CR. The history server had multiple places that treated "the RayCluster CR exists" as "the cluster is live," none of which accounted for suspension, so it kept trying to proxy dashboard requests to a head Pod that no longer exists — failing with a DNS lookup error — and neither the live nor the dead session for that cluster could be viewed.SessionProcessor.isDeadOnly checked whether the CR existed. Now also treats a suspended RayCluster as dead so its sessions fall back to reading stored historical data.
We check the
RayClusterSuspendedstatus condition rather thanspec.Suspenddirectly:spec.Suspendflipstruethe instant a user requests suspension, well before Pods are actually deleted. Marking the session dead (and caching its snapshot — dead-session snapshots are cached indefinitely) at that point would race the collector's SIGTERM-triggered flush of the final logs/events, permanently caching an incomplete snapshot.RayClusterSuspendedis only set by the controller once it observes zero running Pods (raycluster_controller.go#L2064-L2083), which is after the flush window has passed. During the briefRayClusterSuspendingtransition, the session is still treated as live and a proxy attempt can transiently fail, butSessionStatusLiveis never cached, so the next request re-checks and resolves automatically — no permanent breakage.listClustersandresolveSessionisDeadalone wasn't sufficient:listClusters(used to build the cluster list) andresolveSession(used to resolve/enter_cluster/...requests, including the default"latest"resolution — the primary path a user hits when opening a cluster) each independently call into the K8s client and treat any existing RayCluster CR as live, bypassingisDeadentirely. A suspended cluster was still shown as a live entry in the list, and entering it still resolved to"live"and proxied to a Pod that no longer exists.Extracted
isRayClusterSuspendedas a shared helper (used byisDeadtoo) and applied it in both places so a suspended cluster is excluded from the live list and falls back to stored session data when resolved.Related issue number
Closes #5062
Labels
doc-updates-requiredlabel.breaking-changelabel.Checks