-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[10.0] Add Redis Cluster support #26407
Conversation
@DeepDiver1975, thanks for your PR! By analyzing the history of the files in this pull request, we identified @icewind1991, @butonic and @nickvergessen to be potential reviewers. |
throw new \Exception('Redis Cluster support is not available'); | ||
} | ||
// cluster config | ||
$timeout = isset($config['timeout']) ? $config['timeout'] : null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we are not using the ternary operator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ternary has no isset support, so it can not be used in such cases
Some notes for testing this:
with v2.2.8 and a small fix, see upstream doc example the PR works as expected! 👍 |
a4c677c
to
59c8de9
Compare
@Ethendrel care to review? Then we can finally merge redis cluster support! |
33e174d
to
a863dec
Compare
@owncloud/qa quick manual test and create plan to have automated tests for this ? |
our plans for the dockerized integration tests could help a lot here |
Which kind of cluster? :) |
redis 😉 |
Never used a redis cluster, so you want some sentinel based cluster? |
Nope, just as in the redis cluster tutorial: https://redis.io/topics/cluster-tutorial
3 master, 3 slaves. The failover and promotion should be handled by this setup and the redis client, AFAICT. The client side does come with a fail over strategy config option: https://github.com/phpredis/phpredis/blob/feature/redis_cluster/cluster.markdown#automatic-slave-failover--distribution
|
we can test this in 10.0.0beta with a customer, if we merge it for the beta release. |
Please cover with dokcer reds cluster infrastructure for testing @tboerger |
Ok then 👍 @DeepDiver1975 please add to the feature list in the wiki |
I will coordinate this along with the beta. |
QA ticket owncloud/QA#411 |
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. |
Description
Adding support for redis clustering
Restart of #23204
Related Issue
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: