Skip to content
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

Redis prefix causes ever-growing redis cache size #32203

Open
juliusknorr opened this issue Apr 28, 2022 · 3 comments
Open

Redis prefix causes ever-growing redis cache size #32203

juliusknorr opened this issue Apr 28, 2022 · 3 comments
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement feature: caching Related to our caching system: scssCacher, jsCombiner...

Comments

@juliusknorr
Copy link
Member

juliusknorr commented Apr 28, 2022

Whenever Nextcloud or an app is updated the Memcache prefix is changing and old stored keys without an expiry would just stay around forever. Since the prefix is also hashed and not easily genera table due to the app version list in place there is also currently no good way to clean them up manually.

if (!$config->getSystemValueBool('log_query')) {
$v = \OC_App::getAppVersions();
} else {
// If the log_query is enabled, we can not get the app versions
// as that does a query, which will be logged and the logging
// depends on redis and here we are back again in the same function.
$v = [
'log_query' => 'enabled',
];
}
$v['core'] = implode(',', \OC_Util::getVersion());
$version = implode(',', $v);
$instanceId = \OC_Util::getInstanceId();
$path = \OC::$SERVERROOT;
$prefix = md5($instanceId . '-' . $version . '-' . $path);

Maybe we could consider having a predictable global prefix, so we could also introduce a nightly job to delete outdated keys?

On the other hand deleting keys by prefix is a rather heavy operation as it requires a KEYS and then a DEL for each key on the Redis side, so maybe a default expiry of some weeks would also be something that we could consider.

cc @nextcloud/server-backend

@juliusknorr juliusknorr added enhancement 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Apr 28, 2022
@come-nc
Copy link
Contributor

come-nc commented Apr 28, 2022

maybe a default expiry of some weeks would also be something that we could consider.

Instinctively I would go for this, what is the usecase for non-expiring entries in redis?

@juliusknorr
Copy link
Member Author

what is the usecase for non-expiring entries in redis?

At least I'm not aware that we have any, at least not if we consider an expiry > 1 day. In the end I think our memcache abstraction was always considered volatile.

@juliusknorr
Copy link
Member Author

Note that one may configure redis to delete keys, but that only happens if the redis server reaches the configured memory limit then: https://redis.io/docs/manual/eviction/#eviction-policies

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement feature: caching Related to our caching system: scssCacher, jsCombiner...
Projects
None yet
Development

No branches or pull requests

4 participants