Capped cache for cache info in UserMountCache - #25056
Conversation
|
By analyzing the blame information on this pull request, we identified @icewind1991, @nickvergessen and @LukasReschke to be potential reviewers |
|
I think using it for |
|
I mean this one: https://github.com/owncloud/core/blob/master/lib/private/Files/Config/UserMountCache.php#L165 (linking to an invisible row didn't work) |
|
I don't see how that doesn't work with that |
|
@PVince81 well ... the code you linked to in line 165 does what @icewind1991 says. It fetches the mounts from the db if they are not in the user cache. The CappedMemoryCache implements \ArrayAccess so treating it like an array works as expected. Time for the weekend? |
|
oh ... thats a 👍 by me |
|
Hmmm right... seems I didn't parse the I'll adjust to PR to also cap the other one. |
b50be5e to
58a05da
Compare
|
I guess it's 👍 |
|
Okay, both are capped now. Let's do a bit of regression testing... |
|
Did some quick tests with received shares and they still worked fine. |
|
👍 for cacheInfoCache as well Since the default number of entries in the cappedmemorycache is 512 and it will just remove the first key (FIFO) a performance comparison for 1000 or 10000 would be interesting. Then again, only if there are cod paths that would cause iterating over all of the mounts or files (currently I am only aware of files:scan commands but who knows). |
| $this->userManager = $userManager; | ||
| $this->logger = $logger; | ||
| $this->cacheInfoCache = new CappedMemoryCache(); | ||
| $this->mountsForUsers = new CappedMemoryCache(); |
There was a problem hiding this comment.
Can you adjust the doc blocks and remove the default values of the members?
58a05da to
46cc720
Compare
There was a problem hiding this comment.
Cannot use OCP\ICache as ICache because the name is already in use in /var/lib/jenkins/jenkins-slave/workspace/core-ci-linux-php5.4/database/sqlite/label/SLAVE/lib/private/Files/Config/UserMountCache.php
46cc720 to
86d7241
Compare
|
stable9: #25118 |
|
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. |
6 participants
@owncloud/filesystem
I don't think we can use the capped cache for
$mountsForUsersbecause the logic doesn't allow so. If we did, it would limit the number of users to 512... possibly 512 received shares or so, which isn't good as it could cause unpredictable effects when exceeding that value.Or the logic needs to be rewritten to be able to re-resolve the "cache miss" values.
@icewind1991 what do you think ?
CC @butonic