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

perf: use more optimized way to get user storage info in ocs user info #49476

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

icewind1991
Copy link
Member

Instead of doing a full setup etc. We just get quota directly from the user, and the used space directly from the cache.

This means that we only need to setup the home storage for any user, instead of all storages the user has access to.

This does lead to a minor change in behavior, that for users with unlimited quota, the reported quota no longer falls back to the free space in the data directory. As far as I can tell these fields aren't used in the accounts ui and that behavior isn't something that can be relied on anyway, since s3 based setups (and various others) won't have this behavior anyway.

@icewind1991 icewind1991 added the 3. to review Waiting for reviews label Nov 25, 2024
@icewind1991 icewind1991 added this to the Nextcloud 31 milestone Nov 25, 2024
@icewind1991 icewind1991 requested review from come-nc, a team, artonge and yemkareems and removed request for a team November 25, 2024 16:36
@icewind1991 icewind1991 force-pushed the ocs-user-info-quota-optimize branch 2 times, most recently from 1706c3e to e004ce7 Compare November 25, 2024 16:55
Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

quota_include_external_storage is no longer read, is that a regression?
Also, lastSeenQuotaUsage is no more updated, we could fill it with the computed quota, I think?

apps/provisioning_api/lib/Controller/AUserData.php Outdated Show resolved Hide resolved
@icewind1991 icewind1991 force-pushed the ocs-user-info-quota-optimize branch from e004ce7 to a4ed1b0 Compare December 23, 2024 16:34
@icewind1991
Copy link
Member Author

quota_include_external_storage is no longer read, is that a regression?

made it fallback to the old implementation when quota_include_external_storage is enabled.

@icewind1991 icewind1991 force-pushed the ocs-user-info-quota-optimize branch 3 times, most recently from 906b89b to 5958fcc Compare January 3, 2025 14:07
@icewind1991 icewind1991 requested a review from come-nc January 3, 2025 14:26
Comment on lines 285 to 286
/* In case the Exception left things in a bad state */
\OC_Util::tearDownFS();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That should still happen if $includeExternal is true, no?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is, inside the if ($includeExternal) { block

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if it throws you still end up in this catch block, or did I miss something?

@icewind1991 icewind1991 force-pushed the ocs-user-info-quota-optimize branch from 5958fcc to 1a913a8 Compare January 6, 2025 15:51
@icewind1991 icewind1991 force-pushed the ocs-user-info-quota-optimize branch from 1a913a8 to 118ff88 Compare January 6, 2025 15:56
@icewind1991 icewind1991 enabled auto-merge January 6, 2025 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants