Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[core] Add ClusterID token to GCS server [3/n] #36535

Merged
merged 5 commits into from
Jun 30, 2023
Merged

[core] Add ClusterID token to GCS server [3/n] #36535

merged 5 commits into from
Jun 30, 2023

Conversation

vitsai
Copy link
Contributor

@vitsai vitsai commented Jun 17, 2023 •
edited
Loading

Add support for a GetClusterId RPC call in the GCS server that clients can use to obtain the cluster ID. In particular, GCS server will retrieve the cluster id from the persistent store if it exists, or otherwise generate a new one and store it.

Previous PR (GRPC client): #36526
Next PR (GCS client): #35014

Part 3 of breaking down #35014 into more digestible parts.

Why are these changes needed?

Related issue number

#34763

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
    • I've added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@vitsai vitsai mentioned this pull request Jun 17, 2023
Merged
8 tasks
@vitsai vitsai force-pushed the gcs-version-client-small branch 2 times, most recently from c002e55 to 41d8dbd Compare June 17, 2023 06:48
@vitsai vitsai force-pushed the gcs-version-server branch from 62b3e3c to f74167c Compare June 21, 2023 18:20
fishbone
fishbone approved these changes Jun 30, 2023
View reviewed changes
Copy link
Contributor

@fishbone fishbone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix the comments.

jjyao
jjyao approved these changes Jun 30, 2023
View reviewed changes
Copy link
Collaborator

@jjyao jjyao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

src/ray/gcs/gcs_server/gcs_server.cc Outdated
continuation = std::move(continuation)](std::optional<std::string> token) mutable {
if (!token.has_value()) {
ClusterID cluster_id = ClusterID::FromRandom();
RAY_LOG(INFO) << "No existing server token found. Generating new token: "
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

token -> cluster id

src/ray/gcs/gcs_server/gcs_server.cc
});
} else {
ClusterID cluster_id = ClusterID::FromBinary(token.value());
RAY_LOG(INFO) << "Found existing server token: " << cluster_id;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

token -> cluster id

src/ray/gcs/gcs_server/gcs_server.cc Outdated
kTokenNamespace,
kClusterIdKey,
[this,
continuation = std::move(continuation)](std::optional<std::string> token) mutable {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

token -> cluster_id_binary?

@vitsai
nits
4642cb3 
Signed-off-by: vitsai <victoria@anyscale.com>
jjyao
jjyao reviewed Jun 30, 2023
View reviewed changes
src/ray/gcs/gcs_server/gcs_server.cc Outdated
continuation(cluster_id);
});
} else {
ClusterID cluster_id = ClusterID::FromBinary(cluster_id.value());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ClusterID cluster_id = ClusterID::FromBinary(cluster_id.value());
ClusterID cluster_id = ClusterID::FromBinary(provided_cluster_id.value());

@vitsai
comment
fe5f608 
Signed-off-by: vitsai <victoria@anyscale.com>
@fishbone fishbone merged commit 00d633a into master Jun 30, 2023
@fishbone fishbone deleted the gcs-version-server branch June 30, 2023 18:02
@vitsai vitsai mentioned this pull request Jul 5, 2023
Merged
8 tasks
fishbone pushed a commit that referenced this pull request Jul 10, 2023
@vitsai
[core] Retrieve the token from GCS server [4/n] (#37003)
4d8c2c9 
Retrieve the token from the GCS server in the GCS client while connecting, to attach to metadata in requests.

Previous PR (GCS server): #36535
Next PR (auth): #36073
Bhav00 pushed a commit to Bhav00/ray that referenced this pull request Jul 11, 2023
@vitsai @Bhav00
[core] Retrieve the token from GCS server [4/n] (ray-project#37003)
da64972 
Retrieve the token from the GCS server in the GCS client while connecting, to attach to metadata in requests.

Previous PR (GCS server): ray-project#36535
Next PR (auth): ray-project#36073

Signed-off-by: Bhavpreet Singh <singh.bhavpreet00@gmail.com>
vitsai added a commit that referenced this pull request Jul 11, 2023
@vitsai
[core] Retrieve the token from GCS server [4/n] (#37003)
0f09444 
Retrieve the token from the GCS server in the GCS client while connecting, to attach to metadata in requests.

Previous PR (GCS server): #36535
Next PR (auth): #36073
@vitsai vitsai mentioned this pull request Jul 11, 2023
Merged
8 tasks
vitsai added a commit that referenced this pull request Jul 11, 2023
@vitsai
[core] Retrieve the token from GCS server [4/n] (#37003) (#37294)
456f532 
Retrieve the token from the GCS server in the GCS client while connecting, to attach to metadata in requests.

Previous PR (GCS server): #36535
Next PR (auth): #36073
@akshay-anyscale akshay-anyscale mentioned this pull request Jul 21, 2023
Closed
8 tasks
Bhav00 pushed a commit to Bhav00/ray that referenced this pull request Jul 28, 2023
@vitsai @Bhav00
[core] Retrieve the token from GCS server [4/n] (ray-project#37003)
dce17b7 
Retrieve the token from the GCS server in the GCS client while connecting, to attach to metadata in requests.

Previous PR (GCS server): ray-project#36535
Next PR (auth): ray-project#36073

Signed-off-by: Bhavpreet Singh <singh.bhavpreet00@gmail.com>
Bhav00 pushed a commit to Bhav00/ray that referenced this pull request Jul 28, 2023
@vitsai @Bhav00
[core] Retrieve the token from GCS server [4/n] (ray-project#37003)
8abe3f3 
Retrieve the token from the GCS server in the GCS client while connecting, to attach to metadata in requests.

Previous PR (GCS server): ray-project#36535
Next PR (auth): ray-project#36073

Signed-off-by: Bhavpreet Singh <singh.bhavpreet00@gmail.com>
NripeshN pushed a commit to NripeshN/ray that referenced this pull request Aug 15, 2023
@vitsai @NripeshN
[core] Retrieve the token from GCS server [4/n] (ray-project#37003)
4e2c8aa 
Retrieve the token from the GCS server in the GCS client while connecting, to attach to metadata in requests.

Previous PR (GCS server): ray-project#36535
Next PR (auth): ray-project#36073

Signed-off-by: NripeshN <nn2012@hw.ac.uk>
arvind-chandra pushed a commit to lmco/ray that referenced this pull request Aug 31, 2023
@vitsai @arvind-chandra
[core] Add ClusterID to ClientCallManager [2/n] (ray-project#36526)
2af1e4c 
This change adds a Cluster ID to GRPC client (client call, client call manager), and attaches it to the metadata of each ClientCall provided it is non-nil.

Previous PR (GRPC server): ray-project#36517
Next PR (GCS server): ray-project#36535

Part 2 of breaking down ray-project#35014 into more digestible parts.

Related issue number
ray-project#34763

Signed-off-by: e428265 <arvind.chandramouli@lmco.com>
arvind-chandra pushed a commit to lmco/ray that referenced this pull request Aug 31, 2023
@vitsai @arvind-chandra
[core] Add ClusterID token to GCS server [3/n] (ray-project#36535)
22944ba 
Add support for a GetClusterId RPC call in the GCS server that clients can use to obtain the cluster ID. In particular, GCS server will retrieve the cluster id from the persistent store if it exists, or otherwise generate a new one and store it.

Previous PR (GRPC client): ray-project#36526
Next PR (GCS client): ray-project#35014

Part 3 of breaking down ray-project#35014 into more digestible parts.

Signed-off-by: e428265 <arvind.chandramouli@lmco.com>
arvind-chandra pushed a commit to lmco/ray that referenced this pull request Aug 31, 2023
@vitsai @arvind-chandra
[core] Retrieve the token from GCS server [4/n] (ray-project#37003)
cfc8b0e 
Retrieve the token from the GCS server in the GCS client while connecting, to attach to metadata in requests.

Previous PR (GCS server): ray-project#36535
Next PR (auth): ray-project#36073

Signed-off-by: e428265 <arvind.chandramouli@lmco.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jjyao jjyao jjyao approved these changes

@fishbone fishbone fishbone approved these changes

@scv119 scv119 Awaiting requested review from scv119

Assignees

@jjyao jjyao

@fishbone fishbone

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vitsai @fishbone @jjyao