-
Notifications
You must be signed in to change notification settings - Fork 63
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
Implement cluster API #151
Comments
I'll take a look at this. |
@rby Nice, thanks for taking care of this. Note that this might be a significant chunk of work, so feel free to split it in whatever way you find convenient. You might have to adjust, or separate communication channels here. |
I did some exploration, but got stuck on an issue with the protocol when clustering is enabled:
vs, with a standalone:
so all tests are broken already :-) |
@rby that is somewhat expected as clustering works a bit differently. I'm afraid we'll need to have a separate executor to support that. |
progress so far
I guess the next step is to split a task for a |
I'd propose to have a minimal working version in this PR, i.e. connecting to cluster fully works. Once that's in place, the other commands can go in separate PRs. |
@rby I think it might be worth to check https://github.com/antirez/redis-rb-cluster for some ideas how to implement the new executor or modify the existing one. |
@mijicd yup that's what I've been doing the last couple of hours.
|
@rby Sounds good to me. Perhaps there's a chance to reuse the existing executor, but just build it differently, by passing the CRC stuff while building layer, but this is just me speaking out loud. Regarding the difficulty, I didn't mark it as a good first issue for reason(s) :). However, if we'd manage to have a poc of a single command, that would be amazing. |
@mijicd I made some progress, I'll try to test what I have so far without docker-compose as nodes weren't reachable when topology fetched through |
All right, I have a test that passes with |
@rby wow, nice. Sorry for the late response, I'll take a look at it. Btw. you could open up a draft PR so that everyone can chime in there. |
@rby hello. I wonder, would you continue work on that issue? 2 months have passed since you started, mb you need some help or if you don't want to do that issue anymore, I can replace you? |
@anatolysergeev on the contrary it would be great if you take over. |
partially closed issue zio#151
* prepare code for cluster implementation * add cluster support partially done issue #151 * fix compilation errors * some improvement to redis cluster compose file * fix comments * fix errors after merge * fix comments delete some unrelated changes * delete parentheses from asking and slots method * revert redis config name move redis uri to separate class * revert reorganization of folders * fix banchmark 2.12.16 compilation * fix scala 3 compilatioin errors * fix lint * fix review comments * fix review comments * fix lint errors * add documentation to the cluster methods fix some review comments
I'm think that implementation of the other cluster commands isn't priority now due to their low use by users in business logic. That's why i want focus on other issues, that thinks are more important. Feel free to take this issue, i move away from it for now. |
@anatolysergeev I would start working on this issue. Do you have any in-progress commands, or I can freely choose commands? |
Extracted from this comment authored by @regis-leray.
Commands
Tips
zio.redis.api
.zio.redis.options
.zio.redis.Input
andzio.redis.Output
, respectively.zio.redis.InputSpec
andzio.redis.OutputSpec
, respectively.zio.redis.ApiSpec
.The text was updated successfully, but these errors were encountered: