[History server] Add per-session meta.json for session lifecycle tracking#4955
[History server] Add per-session meta.json for session lifecycle tracking#4955Lucas61000 wants to merge 5 commits into
Conversation
…king - Add SessionStatus type, MetaJson struct, and ClusterInfo extensions (types.go) - Add MetaJsonPath helper in cluster-metadata scheme (clustermetadata.go) - Extend StorageWriter/StorageReader with WriteMeta/ReadMeta (interface.go) - Implement WriteMeta/ReadMeta across all 4 storage backends (S3, GCS, Azure, OSS) - Filter .meta.json files from List() to avoid duplicate session entries - Collector: write in_progress on new session, completed + RayStatus on shutdown (head only) - History Server: read-only crash detection — in_progress sessions whose cluster is gone → terminated - Update localtest mocks for WriteMeta/ReadMeta - Fix existing mock implementations for new interface methods - Add tests: MetaJsonPath, List enrichment, crash detection, ReadMeta not found Signed-off-by: Lucas <54979663+lucas61000@users.noreply.github.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
close(ShutdownChan) before updateMetaJsonOnShutdown so WatchSessionLatestLoops cannot fire a late fsnotify event that overwrites completed back to in_progress. Signed-off-by: Lucas <54979663+lucas61000@users.noreply.github.com>
…tor start On startup the new collector finalizes any same-cluster sessions still marked in_progress (left behind by a killed previous collector). Runs at most once per collector lifetime to avoid redundant storage List() calls. Signed-off-by: Lucas <54979663+lucas61000@users.noreply.github.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 3 potential issues.
Reviewed by Cursor Bugbot for commit 3c069d4. Configure here.
| ClusterNamespace: c.Namespace, | ||
| StartTime: c.CreateTimeStamp, | ||
| EndTime: time.Now().Unix(), | ||
| Status: utils.SessionStatusCompleted, |
There was a problem hiding this comment.
Stale sessions marked completed
Medium Severity
When a new session starts, finalizePreviousSessions writes completed for older in_progress meta files left after SIGKILL or collector loss. Those sessions did not shut down gracefully, so persisted status contradicts the PR’s terminated/crash story and bypasses History Server read-time terminated inference for in_progress rows.
Reviewed by Cursor Bugbot for commit 3c069d4. Configure here.
There was a problem hiding this comment.
My understanding is completed means the session ended (cluster moved to a new session), while terminated is reserved for when the RayCluster CR itself disappears. Since the cluster is still alive here, completed seems more appropriate.
| clusters := r.Reader.List() | ||
| for _, c := range clusters { | ||
| if c.Namespace != r.RayClusterNamespace || c.Name != r.RayClusterName { | ||
| continue |
There was a problem hiding this comment.
Owner not matched when finalizing
Medium Severity
finalizePreviousSessions treats sessions as the same cluster when only Namespace and Name match. Metadata paths also distinguish OwnerKind and OwnerName, so a standalone RayCluster and a RayJob-owned cluster can share namespace and Ray cluster name while storing different sessions.
Reviewed by Cursor Bugbot for commit 3c069d4. Configure here.
There was a problem hiding this comment.
As far as I understand, K8s enforces RayCluster name uniqueness per namespace, so two owners cannot hold the same name simultaneously.
…sters Prevents crash detection from reasoning over an incomplete live set when some K8s clients are unreachable. Signed-off-by: Lucas <54979663+lucas61000@users.noreply.github.com>
Signed-off-by: Lucas <54979663+lucas61000@users.noreply.github.com>
|
@chiayi The PR is ready for review, really appreciate your time! |
1 participant
Why are these changes needed?
Add per-session
meta.jsonfiles to the cluster-metadata directory so the History Server can track session lifecycle status (in_progress, completed, terminated).Currently, the History Server lists sessions from cluster-metadata markers but has no way to tell whether a session is active, finished gracefully, or crashed. This matters for operators debugging failed Ray clusters — they need to quickly identify which sessions exited abnormally.
Related issue number
Closes #4885
Checks
/clustersshowscompletedwithendTime.Summary
Collector (head only, write):
in_progressmeta.json when a new session directory is detectedcompleted+ best-effort Ray dashboard cluster_status (3s timeout)in_progress— crash detection handles itHistory Server (read-only inference):
/clusterslisting, cross-references stored sessions against live RayCluster CRsin_progresssession whose K8s CR no longer exists →terminatedListRayClustersreturns error)Storage (4 backends: S3, GCS, Azure Blob, Aliyun OSS):
WriteMeta/ReadMetaonStorageWriter/StorageReaderList()filters.meta.jsonto avoid duplicate session entries, enrichesClusterInfowithStatus+EndTime