-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Add dbindex to redis.config.php #2236
base: master
Are you sure you want to change the base?
Conversation
add dbindex for redis as comment. referenced from a6f2a3c commit Signed-off-by: Arjun Santhosh <[email protected]>
That's not a comment. :-) Also, this would have to be handled in the PHP session handler too otherwise the result might not be what people expect... Lines 108 to 133 in 064069b
|
@@ -6,6 +6,7 @@ | |||
'redis' => array( | |||
'host' => getenv('REDIS_HOST'), | |||
'password' => getenv('REDIS_HOST_PASSWORD_FILE') && file_exists(getenv('REDIS_HOST_PASSWORD_FILE')) ? trim(file_get_contents(getenv('REDIS_HOST_PASSWORD_FILE'))) : (string) getenv('REDIS_HOST_PASSWORD'), | |||
'dbindex' => (int) getenv('REDIS_DB_INDEX'), |
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.
'dbindex' => (int) getenv('REDIS_DB_INDEX'), | |
// 'dbindex' => (int) getenv('REDIS_DB_INDEX'), |
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.
having this in the autoconfig but commented out is pretty useless IMHO. Why did they add it like this in AIO?
kind of duplictae of #1286 Can you elaborate the use case? |
Having an external redis master-master cluster and deploying nextcloud only on designated node. I am aware that prefix hash is calculated by concatenating instanceid, path and version, so in theory there should be no conflicting prefixes, however being able to separate it in separate dbindices in case of needing to cleanup after update etc. is a bonus, especially if you're managing mutliple instances connecting to single cluster. One of the solutions would be having option to use consistent prefix for redis entries. |
Signed-off-by: Arjun Santhosh <[email protected]>
If this will get pulled into main, has the session lock configuration been adjusted aswell? I've tried an example here https://github.com/nextcloud/docker/pull/2288/files |
But Redis Cluster doesn't support dbindex / multiple databases. |
add dbindex for redis as comment. referenced from
a6f2a3c
commit.