zio-redis icon indicating copy to clipboard operation
zio-redis copied to clipboard

Implement cluster API

Open mijicd opened this issue 4 years ago • 16 comments

Extracted from this comment authored by @regis-leray.

Commands

  • [x] CLUSTER ASK When a cluster client receives an -ASK redirect, command is sent to the target node followed by the command which was redirected.
  • [ ] CLUSTER ADDSLOTS slot [slot ...] Assign new hash slots to receiving node
  • [ ] CLUSTER BUMPEPOCH Advance the cluster config epoch
  • [ ] CLUSTER COUNT-FAILURE-REPORTS node-id Return the number of failure reports active for a given node
  • [ ] CLUSTER COUNTKEYSINSLOT slot Return the number of local keys in the specified hash slot
  • [ ] CLUSTER DELSLOTS slot [slot ...] Set hash slots as unbound in receiving node
  • [ ] CLUSTER FAILOVER [FORCE|TAKEOVER] Forces a replica to perform a manual failover of its master.
  • [ ] CLUSTER FLUSHSLOTS Delete a node's own slots information
  • [ ] CLUSTER FORGET node-id Remove a node from the nodes table
  • [ ] CLUSTER GETKEYSINSLOT slot count Return local key names in the specified hash slot
  • [ ] CLUSTER INFO Provides info about Redis Cluster node state
  • [ ] CLUSTER KEYSLOT key Returns the hash slot of the specified ke- [ ]
  • [ ] CLUSTER MEET ip port Force a node cluster to handshake with another node
  • [ ] CLUSTER MYID Return the node id
  • [ ] CLUSTER NODES Get Cluster config for the node
  • [ ] CLUSTER REPLICATE node-id Reconfigure a node as a replica of the specified master node
  • [ ] CLUSTER RESET [HARD|SOFT] Reset a Redis Cluster node
  • [ ] CLUSTER SAVECONFIG Forces the node to save cluster state on disk
  • [ ] CLUSTER SET-CONFIG-EPOCH config-epoch Set the configuration epoch in a new node
  • [x] CLUSTER SETSLOT slot IMPORTING|MIGRATING|STABLE|NODE [node-id] Bind a hash slot to a specific node
  • [ ] CLUSTER SLAVES node-id List replica nodes of the specified master node
  • [ ] CLUSTER REPLICAS node-id List replica nodes of the specified master node
  • [x] CLUSTER SLOTS Get array of Cluster slot to node mappings- [ ]
  • [ ] READONLY Enables read queries for a connection to a cluster replica node
  • [ ] READWRITE Disables read queries for a connection to a cluster replica node

Tips

  • Define the API in zio.redis.api.
  • Command-specific options should be defined in zio.redis.options.
  • Additional inputs and outputs can be defined in zio.redis.Input and zio.redis.Output, respectively.
  • Redis API evolves, therefore the commands listed above might be slightly changed. Double check the official docs before starting with implementation.
  • Use telnet to understand command protocol details.
  • Write inputs and outputs tests in zio.redis.InputSpec and zio.redis.OutputSpec, respectively.
  • Write integration tests by expanding the zio.redis.ApiSpec.
  • If you feel that the pull request size is growing out of control, feel free to split it but make sure to link this issue in each of the related PRs.

mijicd avatar Nov 20 '20 10:11 mijicd

I'll take a look at this.

rby avatar Nov 21 '20 10:11 rby

@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.

mijicd avatar Nov 21 '20 10:11 mijicd

I did some exploration, but got stuck on an issue with the protocol when clustering is enabled: basically this is a telnet session with a cluster:

Escape character is '^]'.
set a 1
-MOVED 15495 :0

vs, with a standalone:

Escape character is '^]'.
set a 1
+OK

so all tests are broken already :-)

rby avatar Nov 21 '20 16:11 rby

@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.

mijicd avatar Nov 21 '20 16:11 mijicd

progress so far

  • the docker-compose-cluster.yml should help with tests/integration

I guess the next step is to split a task for a RedisClusterExecutor or similar?

rby avatar Nov 21 '20 16:11 rby

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.

mijicd avatar Nov 21 '20 16:11 mijicd

@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 avatar Nov 21 '20 18:11 mijicd

@mijicd yup that's what I've been doing the last couple of hours. My guess is:

  • implement cluster nodes command and make it available in the RedisExecutor
  • a Layer to build some kind of ClusterRedisExecutor from a non empty list of RedisExecutor (startup nodes)
  • the layer needs to call cluster nodes on the first available node
  • build the pool of nodes if needed
  • then from there try to understand the slot hashing and figure out how to redirect calls.
  • figure out the next bridge to build wdyt? It's a heavy issue indeed :-)

rby avatar Nov 21 '20 20:11 rby

@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 avatar Nov 21 '20 20:11 mijicd

@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 cluster info command.

rby avatar Nov 22 '20 18:11 rby

All right, I have a test that passes with sAdd, sCard :-)

rby avatar Nov 22 '20 18:11 rby

@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.

mijicd avatar Nov 23 '20 19:11 mijicd

@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? Sorry, no offences. I need this functionality for my projects at work.)) With appreciation)

anatolysergeev avatar Jan 11 '21 22:01 anatolysergeev

@anatolysergeev on the contrary it would be great if you take over. best luck 👍

rby avatar Jan 11 '21 23:01 rby

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 avatar Nov 12 '22 20:11 anatolysergeev

@anatolysergeev I would start working on this issue. Do you have any in-progress commands, or I can freely choose commands?

drmarjanovic avatar Apr 06 '23 07:04 drmarjanovic