Documentation should make it clear that redis cluster is required for redis mode

[paulchoi]

I tried configuring Redis caching and the error message says ERR This instance has cluster support disabled. I brought up Redis instances using redis:7.0.4-alpine Docker container with no custom configuration.

INFO: 2022/07/28 00:29:42 main.go:46: chproxy ver. 1.16.3, rev. fab6ecdb24f30afdafde7b6d4cec4f03eebb5d6a, built at 2022-07-21T11:32:34Z
INFO: 2022/07/28 00:29:42 main.go:47: Loading config: /config/config.yaml
FATAL: 2022/07/28 00:29:42 main.go:53: error while applying config: failed to reach redis: ERR This instance has cluster support disabled

[mga-chka]

I don't see why redis caching requires a redis cluster. I'll flag this issue as a bug and we'll see if we either change the doc or fix the root cause

[paulchoi]

Figured out what is triggering the issue.

If just 1 Redis instance is configured under addresses, the error is not triggered. When 2 or more Redis instances are configured under addresses, then the error triggers.

So the workaround is to use just 1 Redis address for now.

[mga-chka]

Thanks for the update. Just to understand (to see if it's worth to mention your trick in the doc), if you had a non clustered Redis, why did you ended-up with multiple addresses in the conf?

[paulchoi]

(Sorry, was out of town for a while)

Sometimes you can add multiple Redis instances to a configuration if the application supports a hashing algorithm that utilizes multiple Redis instances - consistent hashing, for example. Since the configuration allows for multiple Redis instances, and there was no mention of requiring Redis Cluster, I wanted to see if chproxy would work.

I think it'd be sufficient to just state in the documentation that one should specify 2 or more Redis instances only if they are part of a Redis Cluster.

[mga-chka]

FYI the following PR https://github.com/ContentSquare/chproxy/pull/215 will add more description on the documentation on the pb you faced. Once it's merge I'll close this PR (unless you think the description added is not enough)

[mga-chka]

The PR was merged