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] impl RedisStoreClient for GCS Service #7675

Merged
merged 46 commits into from
Apr 1, 2020

Conversation

micafan
Copy link
Contributor

@micafan micafan commented Mar 20, 2020

This PR is for GCS Fault Tolerance:

GCS Service use Persistent Storage to store key information. This PR implement the RedisStoreClient which use Redis as Persistent Storage.

Some complex functions are not implemented and will be completed in subsequent PRs.

Related issue number

Checks

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@micafan micafan linked an issue Mar 20, 2020 that may be closed by this pull request
@micafan micafan changed the title [GCS] Add RedisStoreClient for GCS Service [GCS] impl RedisStoreClient for GCS Service Mar 20, 2020
@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/23695/
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/23723/
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/23724/
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/23725/
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/23922/
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/23941/
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/23939/
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/23943/
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/23945/
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/23982/
Test FAILed.

@raulchen raulchen merged commit 780c1c3 into ray-project:master Apr 1, 2020
@raulchen raulchen deleted the redis_impl_kv_store_step_one branch April 1, 2020 13:18
@mehrdadn
Copy link
Contributor

mehrdadn commented Apr 2, 2020

Hi folks! Unfortunate news—it seems this PR broke something on Windows (which we don't have tests running for yet), and it's not clear to me what's going on. Would someone be able to help me figure out what's going on? I'm not really familiar with XADD (is it a recent Redis feature?), so it's not clear to me how to handle this. If we're now depending on Redis 5 features or if this is otherwise difficult to fix, can we revert this PR and hold off on it until we can get it working on Windows?

(cc @raulchen @micafan)

F0402 05:45:47.700804 23788 redis_context.cc:70]  Check failed: false Got an error in redis reply: ERR unknown command `XADD`, with args beginning with: `ef0a6c220100`, `*`, `signal`, `ACTOR_DIED_SIGNAL`,
*** Check failure stack trace: ***
    @   00007FF79A0A6D3D  google::LogMessage::Fail
    @   00007FF79A0A60E7  google::LogMessage::SendToLog
    @   00007FF79A0A6984  google::LogMessage::Flush
    @   00007FF79A0A67F6  google::LogMessage::~LogMessage
    @   00007FF799E0911A  ray::RayLog::~RayLog
    @   00007FF799E6FC84  ray::gcs::CallbackReply::CallbackReply
    @   00007FF799E7EFF9  std::_Ref_count_obj<ray::gcs::CallbackReply>::_Ref_count_obj<redisReply *&>
    @   00007FF799E74D02  std::make_shared<ray::gcs::CallbackReply,redisReply *&>
    @   00007FF799E71286  ray::gcs::GlobalRedisCallback
    @   00007FF79A0CD7CF  __redisRunCallback
    @   00007FF79A0CD1D4  redisProcessCallbacks
    @   00007FF79A0CD8B6  redisAsyncHandleRead
    @   00007FF79A0CF3A2  ray::gcs::RedisAsyncContext::RedisAsyncHandleRead
    @   00007FF79A58ED77  RedisAsioClient::handle_read

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.

GCS Service
6 participants