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.

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

[core] remove PythonGcsClient as well as Python gRPC stubs to GCS. #48964

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rynewang
Copy link
Contributor

After the effort to add Async GcsClient binding to Python, we allow both old and new code paths to live for a while and now it seems to be pretty stable. We never received any issues from customers about the NewGcsClient. Hence we remove the old one.

@rynewang rynewang marked this pull request as ready for review November 27, 2024 23:09
@rynewang rynewang requested a review from a team as a code owner November 27, 2024 23:09
GCS_AIO_CLIENT_DEFAULT_THREAD_COUNT = env_integer(
"GCS_AIO_CLIENT_DEFAULT_THREAD_COUNT", 5
)
from ray._raylet import NewGcsClient
Copy link
Collaborator

Choose a reason for hiding this comment

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

NewGcsClient -> GcsClient?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah problem is we have NewGcsClient the binding and the GcsClient the one with call frequency counter. Not sure about the naming here...

Copy link
Collaborator

Choose a reason for hiding this comment

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

Got it.

Should it be GcsClient, GcsSyncClient and GcsAsyncClient?

Comment on lines +13 to +15
Historical note: there was a `ray::gcs::PythonGcsClient` C++ binding which has only
sync API and in Python we wrap it with ThreadPoolExecutor. It's been removed in
favor of `ray::gcs::GcsClient` which contains async API.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need this lol

GCS_AIO_CLIENT_DEFAULT_THREAD_COUNT = env_integer(
"GCS_AIO_CLIENT_DEFAULT_THREAD_COUNT", 5
)
from ray._raylet import NewGcsClient
Copy link
Collaborator

Choose a reason for hiding this comment

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

Got it.

Should it be GcsClient, GcsSyncClient and GcsAsyncClient?

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.

2 participants