-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Ruiyang Wang <[email protected]>
GCS_AIO_CLIENT_DEFAULT_THREAD_COUNT = env_integer( | ||
"GCS_AIO_CLIENT_DEFAULT_THREAD_COUNT", 5 | ||
) | ||
from ray._raylet import NewGcsClient |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NewGcsClient -> GcsClient?
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
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
?
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. |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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
?
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.