[History Server] Anchor the GCS GetContent search at the configured root dir - #5077
Open
stantheman0128 wants to merge 1 commit into
Open
[History Server] Anchor the GCS GetContent search at the configured root dir#5077stantheman0128 wants to merge 1 commit into
stantheman0128 wants to merge 1 commit into
Conversation
…oot dir GetContent searched with the glob "**/<cluster>*/**/<file>". Per the GCS docs, "**" matches zero or more characters including "/", so a leading "**/" places no constraint on the prefix at all. The search therefore ignores RootDir, while ListFiles and List in the same handler both build their prefixes with path.Join(h.RootDir, ...). On a bucket holding more than one root dir, for example two history server deployments sharing it, the search can match an object under a different root dir and GetContent returns whichever one the iterator yields first. Anchor the pattern at RootDir. An empty RootDir keeps the previous unanchored pattern, so deployments that do not set one are unaffected. The pattern is built by a small helper so it can be asserted directly: fake-gcs-server does not implement matchGlob, so no test in this package can observe what a pattern actually selects. That also means the existing TestGetContent never exercised the glob; it passes because its bucket holds a single object.
Collaborator
|
cc @chiayi |
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.
Summary
The GCS history-server backend searched for log objects with an unanchored
MatchGlob(**/<cluster>*/**/<file>). GCS treats**as crossing/, so the pattern was not rooted atRootDireven thoughListFilesandListin the same handler already join paths underh.RootDir.This change extracts
contentMatchGloband anchors the search at the configured root directory, matching the s3/azureblob/aliyunoss backends.AI disclosure
AI-assisted patch; human-reviewed by Stan Shih (stantheman0128) before publish.
Verification / Evidence
What was not tested
MatchGlobbehaviour against fake-gcs-server: v1.53.1 does not implementmatchGlob(grep shows zero hits), so the glob contract is asserted oncontentMatchGlobdirectly instead of an integration test that would fail for the wrong reason.Related context: #5075 adds s3/azureblob
GetContentpath tests; this PR fixes the GCS production path.