Skip to content

[History server] Skip directory markers when listing storage - #5236

Open
win5923 wants to merge 1 commit into
ray-project:masterfrom
win5923:skip-directory-history-server
Open

[History server] Skip directory markers when listing storage#5236
win5923 wants to merge 1 commit into
ray-project:masterfrom
win5923:skip-directory-history-server

Conversation

@win5923

@win5923 win5923 commented Sep 3, 2026

Copy link
Copy Markdown
Member

Why are these changes needed?

When the History Server reads cluster metadata from object storage, directory marker objects created by StorageWriter.CreateDirectory are returned alongside the actual session markers.

S3 bucket
│
├── cluster-metadata/rayjob/default_rayjob-historyserver_rayjob-historyserver-5qj8v/
│   └── 0 bytes  ← directory marker
│
└── cluster-metadata/rayjob/default_rayjob-historyserver_rayjob-historyserver-5qj8v/session_xxx
               └── 0 bytes  ← session marker

The directory marker is a 0-byte object, not a metadata file, but List() passes it to DecodePath(), causing a misleading error:

time="2026-09-03T03:20:04Z" level=error msg="Failed to parse meta file path: cluster-metadata/rayjob/default_rayjob-historyserver_rayjob-historyserver-5qj8v/, error: invalid path segment count for cluster metadata structure: cluster-metadata/rayjob/default_rayjob-historyserver_rayjob-historyserver-5qj8v/"
  • List(): a placeholder has one path segment too few for DecodePath, so every /clusters and /enter_cluster request logged one parse error per cluster in the bucket.

// DecodePath parses the hierarchical path and returns the cluster info.
func DecodePath(filePath string, rootDir string) (utils.ClusterInfo, error) {
prefix := Prefix(rootDir)
cleanFilePath := strings.Trim(filePath, "/")
cleanPrefix := strings.Trim(prefix, "/") + "/"
if !strings.HasPrefix(cleanFilePath, cleanPrefix) {
return utils.ClusterInfo{}, fmt.Errorf("invalid path %q: expected prefix %q", filePath, prefix)
}
relativePath := strings.Trim(strings.TrimPrefix(cleanFilePath, cleanPrefix), "/")
pathSegments := strings.Split(relativePath, "/")
if len(pathSegments) != 3 {
return utils.ClusterInfo{}, fmt.Errorf("invalid path segment count for cluster metadata structure: %s", filePath)
}

  • _listFiles(): a placeholder surfaced as an entry named after its own directory, leaking into /api/v0/logs?glob=** as "events/events".
image

Changes:

  • Add utils.IsDirectoryMarker.
  • Skip markers in the object loops of s3, aliyunoss, in both List() and _listFiles().
  • GCS unchanged: List() already filters them via MatchGlob, and _listFiles() via HasSuffix.
  • Azure Blob unchanged: CreateDirectory is a no-op, so it does not create directory marker objects.

Related issue number

Labels

  • If this PR has user-facing changes that require documentation updates at release time, I have added the doc-updates-required label.
  • If this PR contains breaking changes, I have added the breaking-change label.

Checks

  • I've made sure the tests are passing.
  • Testing Strategy
    • Unit tests
    • Manual tests
    • This PR is not tested :(

Manual test instructions

Signed-off-by: win5923 <ken89@kimo.com>
@win5923

win5923 commented Sep 3, 2026

Copy link
Copy Markdown
Member Author

Should we stop calling CreateDirectory on the writer side? I think object stores like S3 don't require parent directories (a/b) to exist before writing a/b/c.

This would also avoid creating directory marker objects in the first place.

cc @chiayi @KunWuLuan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant