Cache the user backend info for 5mins#25440
Merged
Conversation
nickvergessen
added
bug
2. developing
nickvergessen
commented
Feb 2, 2021
Member
|
Master is Nextcloud 22 now. |
ChristophWurst
approved these changes
Feb 5, 2021
Member
|
Guess it is time to shape this up. And then merge it early for 22? |
Member
Author
|
Yeah, I'm thinking about increasing the caching to 5 minutes. we just influence the order of user backends we check. |
nickvergessen
force-pushed
the
bugfix/noid/cache-the-user-backend-info-for-30s
branch
from
003becf to
f45570b
Compare
nickvergessen
changed the title
nickvergessen
force-pushed
the
bugfix/noid/cache-the-user-backend-info-for-30s
branch
from
f45570b to
607a975
Compare
Signed-off-by: Joas Schilling <coding@schilljs.com>
nickvergessen
force-pushed
the
bugfix/noid/cache-the-user-backend-info-for-30s
branch
from
607a975 to
645f831
Compare
ChristophWurst
approved these changes
Feb 15, 2021
rullzer
approved these changes
Feb 17, 2021
Member
rullzer
left a comment
rullzer
left a comment
There was a problem hiding this comment.
So we assume the nr of backends doesn't change here. Well else we fall trough so all good I guess.
Member
|
/backport to stable21 |
MichaIng
added a commit
that referenced
this pull request
Jun 26, 2024
but fallback to NullCache. This can be the case if APCu is used without apc.enable_cli=1. APCu (and ArrayCache) however runs within the PHP instance and hence cannot be shared between CLI and web or used as distributed cache. The CLI call can hence only create or invalildate entries for itself. For short-living CLI calls, this is theoretically a downsides regarding performance and resource usage, and Nextcloud must not have any issues using the dummy NullCache instead of an isolated freshly created and destroyed APCu or ArrayCache instance. This partly reverts #25770. The fallback was removed, because CLI calls started to hang after #25440. The reason however was not that a cache is generally required for CLI calls, but because the previously logged warning invoked the user backend, which invoked again the cacking backend, causing a loop. This commit re-adds the fallback without logging a warning. For mentioned reasons, it is okay to fallback to NullCache silently. The motivation is to make apc.enable_cli=1 optional, and that hence the documentation about it can be removed. We should not enforce admins to enable APCu for CLI calls, which is reasonably disabled by default. This also reduces requirements for hosting providers to support Nextcloud. Signed-off-by: MichaIng <micha@dietpi.com>
5 tasks
MichaIng
added a commit
that referenced
this pull request
Jun 30, 2024
but fallback to NullCache. This can be the case if APCu is used without apc.enable_cli=1. APCu however runs within the PHP instance and hence cannot be shared between CLI and web or used as distributed cache. The CLI call can hence only create or invalidate entries for itself. For short-living CLI calls, this is theoretically a downsides regarding performance and resource usage, and Nextcloud must not have any issues using the dummy NullCache instead of an isolated freshly created and destroyed APCu instance. This partly reverts #25770. The fallback was removed, because CLI calls started to hang after #25440. The reason however was not that a cache is generally required for CLI calls, but because the previously logged warning invoked the user backend, which invoked again the caching backend, causing a loop. This commit re-adds the fallback without logging a warning, and for APCu only. For mentioned reasons, it is okay to fallback to NullCache silently. If Redis or memcached are configured but not available, then the web UI would fail as well, and it makes sense to abort and inform CLI calls as well then. The motivation is to make apc.enable_cli=1 optional, and that hence the documentation about it can be removed. We should not enforce admins to enable APCu for CLI calls, which is reasonably disabled by default. This also reduces requirements for hosting providers to support Nextcloud. Signed-off-by: MichaIng <micha@dietpi.com>
MichaIng
added a commit
that referenced
this pull request
Jun 30, 2024
but fallback to NullCache. This can be the case if APCu is used without apc.enable_cli=1. APCu however runs within the PHP instance and hence cannot be shared between CLI and web or used as distributed cache. The CLI call can hence only create or invalidate entries for itself. For short-living CLI calls, this is theoretically a downsides regarding performance and resource usage, and Nextcloud must not have any issues using the dummy NullCache instead of an isolated freshly created and destroyed APCu instance. This partly reverts #25770. The fallback was removed, because CLI calls started to hang after #25440. The reason however was not that a cache is generally required for CLI calls, but because the previously logged warning invoked the user backend, which invoked again the caching backend, causing a loop. This commit re-adds the fallback without logging a warning, and for APCu only. For mentioned reasons, it is okay to fallback to NullCache silently. If Redis or memcached are configured but not available, then the web UI would fail as well, and it makes sense to abort and inform CLI calls as well then. The motivation is to make apc.enable_cli=1 optional, and that hence the documentation about it can be removed. We should not enforce admins to enable APCu for CLI calls, which is reasonably disabled by default. This also reduces requirements for hosting providers to support Nextcloud. Signed-off-by: MichaIng <micha@dietpi.com>
MichaIng
added a commit
that referenced
this pull request
Jul 3, 2024
but fallback to NullCache. This can be the case if APCu is used without apc.enable_cli=1. APCu however runs within the PHP instance and hence cannot be shared between CLI and web or used as distributed cache. The CLI call can hence only create or invalidate entries for itself. For short-living CLI calls, this is theoretically a downsides regarding performance and resource usage, and Nextcloud must not have any issues using the dummy NullCache instead of an isolated freshly created and destroyed APCu instance. This partly reverts #25770. The fallback was removed, because CLI calls started to hang after #25440. The reason however was not that a cache is generally required for CLI calls, but because the previously logged warning invoked the user backend, which invoked again the caching backend, causing a loop. This commit re-adds the fallback without logging a warning, and for APCu only. For mentioned reasons, it is okay to fallback to NullCache silently. If Redis or memcached are configured but not available, then the web UI would fail as well, and it makes sense to abort and inform CLI calls as well then. The motivation is to make apc.enable_cli=1 optional, and that hence the documentation about it can be removed. We should not enforce admins to enable APCu for CLI calls, which is reasonably disabled by default. This also reduces requirements for hosting providers to support Nextcloud. Signed-off-by: MichaIng <micha@dietpi.com>
AndyScherzinger
pushed a commit
that referenced
this pull request
Aug 1, 2024
but fallback to NullCache. This can be the case if APCu is used without apc.enable_cli=1. APCu however runs within the PHP instance and hence cannot be shared between CLI and web or used as distributed cache. The CLI call can hence only create or invalidate entries for itself. For short-living CLI calls, this is theoretically a downsides regarding performance and resource usage, and Nextcloud must not have any issues using the dummy NullCache instead of an isolated freshly created and destroyed APCu instance. This partly reverts #25770. The fallback was removed, because CLI calls started to hang after #25440. The reason however was not that a cache is generally required for CLI calls, but because the previously logged warning invoked the user backend, which invoked again the caching backend, causing a loop. This commit re-adds the fallback without logging a warning, and for APCu only. For mentioned reasons, it is okay to fallback to NullCache silently. If Redis or memcached are configured but not available, then the web UI would fail as well, and it makes sense to abort and inform CLI calls as well then. The motivation is to make apc.enable_cli=1 optional, and that hence the documentation about it can be removed. We should not enforce admins to enable APCu for CLI calls, which is reasonably disabled by default. This also reduces requirements for hosting providers to support Nextcloud. Signed-off-by: MichaIng <micha@dietpi.com>
Member
|
I am missing cache invalidation after delete. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Joas Schilling coding@schilljs.com