Support for redis-cluster

[pat-s]

Feature Description

Using Gitea with redis-cluster seems not possible as of right now.
When attempting so, the following error is thrown by Gitea

gitea 2023/04/01 23:31:36 ...ers/web/auth/auth.go:312:handleSignInFull() [E] [6428bed8] RegenerateSession: regenerate session: CROSSSLOT Keys in request don't hash to the same slot

This is a unique behavior of redis-cluster compared to a single redis installation and must be accounted for on the client side as described in https://dzone.com/articles/resolved-crossslot-keys-error-in-redis-cluster-mod.

When creating keys that could be used by multi-key operations, use hashtags ({…}) to force the keys into the same hash slot. When the key contains a “{…}” pattern, only the substring between the braces, “{” and “}”, is considered to calculate the hash slot.
For example, the keys {user1}:mykey1 and {user1}:mykey2 are hashed to the same hash-slot because only the string inside the braces “{” and “}”, that is, “user1”, is used to compute the hash-slot.

I can't infer if this would be a simple addition or a breaking change but wanted to drop the information about redis-cluster anyhow as I couldn't find a prior discussion about it.

Screenshots

No response

[lunny]

Could you paste your configuration and redis cluster information here?

[pat-s]

I just used the default config of the bitnami chart. Important: I used the latest v6 redis (chart: release 7.6.4) as the v7 has some compatibility issues with the underlying Debian image and runs into another issue (unrelated to Gitea).

[wxiaoguang]

You can modify your redis conn string in your Gitea config , add a "prefix" with slot, then p.c.Rename(ctx, poldsid, psid) in code won't report the CROSSSLOT error. I guess: redis://..../?...&prefix={session}:

[lunny]

See the possible configuration connection string https://github.com/go-gitea/gitea/blob/main/modules/nosql/redis.go#L18-L22

[wxiaoguang]

The "prefix" option is not documented, but cluster needs it. Make it prefix={session}: , then all sessions keys become {session}:random-session-id, it could make the redis cluster happy.

[silverwind]

Tangentially related: There is also redis-sentinel, a kind of "proxy" daemon that may need special support inside our code. I think it can be used in addition to redis-cluster. It is the premier HA option for redis.

[wxiaoguang]

To fix the problem, either:

1. Let users do "?prefix={session}:" trick

• OR -

2. Do not use Rename in redis session provider , just Get and Set, then no cross-cluster problem.

[pat-s]

I came across this while testing redis-cluster support for the helm chart. Happy to try again once there is a built-in fix for this which does not require manual modifications. This way we can have a "clean" start when adding support for redis-cluster.

[wxiaoguang]

Can you try this PR: Use Get/Set instead of Rename when Regenerate session id #23975 ?

[pat-s]

Not as a PR as I would need an image to test it with the Helm chart.

[wxiaoguang]

Approve and merge, then get a dev image?

[lunny]

It should be a bug rather than a feature because we have already supports redis cluster.

[pat-s]

This PR together with #24114 resulted in a successful connection to a redis-cluster instance 🎉