nextcloud / nextcloud/whiteboard

Support Redis Cluster in the websocket server

Open
#1,317 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
JavaScript
Stars
215
Forks
39
Avg merge
1d 3h
Merged PRs (30d)
32

Description

Describe the feature you'd like

Let REDIS_URL name several seed nodes so the websocket server can use a Redis Cluster, the way redis.cluster already works on the Nextcloud side.

Why

The README recommends Redis for multi-node websocket deployments, and states that "Redis is required for multi-node clusters". On an installation that already runs Redis Cluster, that recommendation currently cannot be followed: RedisAdapter.createRedisClient() builds a single-node client with createClient(), which does not follow MOVED redirections.

The result is not a clean failure at startup. The server connects, the Socket.IO Redis Streams adapter sets up, and then roughly half of all operations fail at run time depending on which slot a key hashes to.

Measured against a 3-master / 3-replica cluster, writing 40 keys through createClient() pointed at one node:

successful: 17/40
failed    : 23/40
example   : MOVED 8308 127.0.0.1:7002

Nextcloud server itself supports Redis Cluster ('redis.cluster' => ['seeds' => [...]]), so an administrator who has already built one reasonably expects the whiteboard backend to use it. Today the only options are to stand up a second, non-clustered Redis just for whiteboard, or to give up multi-node.

We run a Nextcloud installation with a six node Redis Cluster and four websocket nodes behind a load balancer. We ended up deploying a separate single Redis instance for whiteboard, which is now the only non-redundant component in an otherwise redundant setup.

Proposed shape

Reuse REDIS_URL and treat a comma separated value as a list of cluster seed nodes:

STORAGE_STRATEGY=redis
REDIS_URL=redis://node1:6379,redis://node2:6379,redis://node3:6379

createCluster() is already available in the redis package the project depends on, so this is a small change and needs no new dependency. Credentials given on the first seed have to be applied to the discovered nodes as well, since cluster discovery reports them without auth.

One thing worth documenting alongside it: Redis Cluster only has database 0, so the /database_number suffix that works for a single node cannot be used to separate whiteboard's keys from other users of the same cluster.

I have this working and tested against a real cluster and will open a pull request.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at RedisAdapter.createRedisClient() and the redis package's createCluster() support, then trace how REDIS_URL is parsed. Done means comma-separated seed URLs work with discovered-node authentication, while the README documents Redis Cluster's database-0 limitation and the single-node suffix restriction; verify against a cluster.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, redis
Domain
backend, databases
Issue type
Feature
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
74/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.