Add cluster option to result backend

[stinovlas]

This is a draft that adds redis cluster option for result backend.

This PR contains set up of redis cluster in CI, which can be used in further PRs (such as adding cluster mode support to broker).

[stinovlas]

I'd like to ask for CI (GitHub actions) approval. Otherwise it's really hard to fine tune the tests.

[s3rius]

Sure. Anyway. I have a question regarding self.redis_cls.from_url. Does this thing create a connection pool inside or it maintains a single connection?

[stinovlas]

Sure. Anyway. I have a question regarding self.redis_cls.from_url. Does this thing create a connection pool inside or it maintains a single connection?

AFAIK RedisCluster maintains connection pool internally and it's important to create just single instance of it (not using it as a context manager, which would destroy the connection pool after each __aexit__). It's not possible to provide RedisCluster with external connetion pool upon instantiation. I suspect this is because cluster constructs the connection pool from fetched replicas addresses that are not known when calling RedisCluster.__init__.

I used the same approach for single Redis instance as well. The other possible approach would be to create separate result backend class for cluster mode. I'm not really sure which of these two approaches is better though. I'd appreciate input. If we added a new class, the original result backend could stay as it is.

[s3rius]

@stinovlas, let's create a completely separate backend for now. It might be called like RedisClusterResultBackend. Also it would be easier to maintain.

[stinovlas]

@stinovlas, let's create a completely separate backend for now. It might be called like RedisClusterResultBackend. Also it would be easier to maintain.

I separated the cluster backend to a new class. There's quite a lot of duplication (also in tests), but let's call it a proof of concept for now. I believe that tests could be deduplicated, at least to an extent.

[s3rius]

For tests. Let's create a docker-compose with cluster and non-cluster redis. with correct ports exposed. Because github services is a bit limited.

[stinovlas]

For tests. Let's create a docker-compose with cluster and non-cluster redis. with correct ports exposed. Because github services is a bit limited.

I was able to put together single-node cluster without docker-compose. It seems to work. I can setup full-fledget six node cluster, but I'm not sure it's worth it for test (after all, we test interaction with the cluster, not the cluster itself).

If you insist on docker-compose, I'll need a bit of help, since I'm not that familiar with GitHub actions workflow. I can put together a docker-compose.yml, but then what? Can I call docker-compose up -d directly from CI, or is there some other setup that needs to be done?

[s3rius]

I insist on docker-compose for two main reasons.

1. Local testing is the same as testing in CI.
2. It's easy to test everything locally;

Because currently there's no setup of a test deployment locally. One redis container is fine, but cluster setup is a bit of a pain to setup.

[stinovlas]

I insist on docker-compose for two main reasons.

1. Local testing is the same as testing in CI.
2. It's easy to test everything locally;

Because currently there's no setup of a test deployment locally. One redis container is fine, but cluster setup is a bit of a pain to setup.

Fair enough. I added docker-compose.yml file that sets up:

• single Redis instance on localhost:7000
• six-node (3 masters + 3 replicas) Redis cluster with one node accessible on localhost:7001; I could expose all nodes, but it seemed unnecessary

I hesitated whether to change port for single redis instance from 6379 to 7000. In the end, I decided to do so, so the developers don't have to shutdown their local redis instance. But, I can also see some strong arguments against this (mainly breaking the current test workflow – but since we added cluster and docker-compose, it has changed in any case) and I'm not dead set on this. I can revert the default port for single redis instance to 6379 (and use 7000 for cluster node).

[s3rius]

Looks good to me. I may change docker-compose file a bit later. But anyway. Thanks for your contribution. I really appreciate this.

[stinovlas]

Looks good to me. I may change docker-compose file a bit later. But anyway. Thanks for your contribution. I really appreciate this.

Thanks for communicating so promptly :-). I'll probably take a stab at ListQueueBroker which could provide cluster alternative as well, since it doesn't use potentially problematic PUB/SUB.

[s3rius]

Yes. Seems like it's possible. But I'm not sure how well does redis cluster handles lists in terms of atomacity.