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

[GCS]Add abstract interface of actor to GCS Client #6269

Merged

Conversation

micafan
Copy link
Contributor

@micafan micafan commented Nov 25, 2019

Add abstract interface for reading or writing or subscribing actors to GCS Client. Through the abstract interface, GCS can be extended to support other storage systems.

Related issue number

Closes #5058

Checks

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/18878/
Test FAILed.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/18876/
Test PASSed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/18931/
Test FAILed.

@micafan micafan requested review from edoakes and pcmoritz November 27, 2019 03:06
@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/18978/
Test PASSed.

src/ray/gcs/gcs_client_interface.h Outdated Show resolved Hide resolved
src/ray/gcs/redis_gcs_client.h Outdated Show resolved Hide resolved
@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/19159/
Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/19160/
Test FAILed.

Copy link
Contributor

@edoakes edoakes left a comment

Choose a reason for hiding this comment

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

LGTM, just a few more nits

src/ray/gcs/actor_state_accessor.h Outdated Show resolved Hide resolved
src/ray/gcs/redis_actor_state_accessor.cc Outdated Show resolved Hide resolved
src/ray/gcs/redis_actor_state_accessor.h Outdated Show resolved Hide resolved
src/ray/gcs/redis_actor_state_accessor.cc Outdated Show resolved Hide resolved
Copy link
Contributor

@edoakes edoakes left a comment

Choose a reason for hiding this comment

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

Ah sorry, meant to approve....

Copy link
Contributor

@raulchen raulchen left a comment

Choose a reason for hiding this comment

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

Regarding the file structure, it seems that we'll have 5 files (x_info_accessor.h, redis_x_info_accessor.h/cc, and service_based_x_info_accessor.h/cc). That's a bit too complicated. What about just put everything in 2 files x_info_accessor.h/cc?

src/ray/gcs/redis_actor_state_accessor.h Outdated Show resolved Hide resolved
src/ray/gcs/actor_state_accessor.h Outdated Show resolved Hide resolved
@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/19187/
Test PASSed.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/19193/
Test PASSed.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/19191/
Test PASSed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/19192/
Test FAILed.

@raulchen raulchen merged commit 668ce47 into ray-project:master Dec 5, 2019
@raulchen raulchen deleted the add_gcs_actor_accessor_interface branch December 5, 2019 05:38
@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/19219/
Test FAILed.

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.

5 participants