Redis Cluster support #23204
Redis Cluster support #23204
Conversation
| if(isset($config['password']) && $config['password'] !== '') { | ||
| self::$cache->auth($config['password']); | ||
| } | ||
| if ($config = $systemConfig->getValue('redis.cluster', [])) { |
There was a problem hiding this comment.
would it makes sense to subclass here instead?
There was a problem hiding this comment.
Hmm, actually, that's probably a really good idea.
RobinMcCorkell
force-pushed
the
redis-cluster
branch
from
0f5c9fd to
2af4103
Compare
|
looks good. 👍 @DeepDiver1975 ? |
|
Code looks good, would prefer to have some additional testing before merging though |
RobinMcCorkell
force-pushed
the
redis-cluster
branch
from
2af4103 to
a765255
Compare
|
Not sure how we want to take this? Do enterprise customers want support now, even if that means compiling a development version of phpredis? |
|
As an enterprise edition customer I would like to see it implemented rather soon, as transactional file locking is apparently strongly recommended but requires redis. This feature would be needed to maintain the high availability in our clustered environment while using the preferred file locking mechanism. |
|
@Ethendrel Are you sure? That would be surprising to me, since 2.2.7 does not include the cluster code according to GitHub. Could it be that you've got a git-compiled version, but the version code in the extension is still 2.2.7 (they only update the version number at release)? |
|
Closed in favour of #24033 |
|
Is there cluster support in future? #24033 was put in backlog, so this might be of interest again. |
|
@DeepDiver1975 since predis support was deferred to future, we should reopen this and revive @Xenopathic's PR for cluster support |
Implement a completely new interface to just throw it away in the next release (or the one after that) 😕 |
Agreed - I honestly don't see this in the scope of 9.1 - this needs some serious thinking which usually has to be done before a release. @karlitschek I'd like to put this to 9.2 and have a discussion first on where we want to go. |
|
@DeepDiver1975 that sounds like a GREAT idea. This has potentially a serious set of implications... |
|
I'm closing this for now - we can restore the branch any time if necessary |
|
rebasing is pointless -> close and restart |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
This adds the ability to specify a Redis Cluster, rather than a single Redis server, with the new
redis.clusterconfig.php key. There are no unit tests for this since we would need a way to run Docker containers for multiple Redis servers, with the whole kill-a-master-everything-should-work thing.Note that this does NOT bring support for Redis Sentinel-based configurations, or manual master-slave configurations, that is a different architecture.
Also, there seems to be no way to do authentication or DB index selection with a cluster, for those features you'll want the single server mode
Fixes #21448
cc @pako81 @cdamken @MorrisJobke @MTRichards