Do not persist user/group/project quota zap objects when unneeded #14721
Conversation
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
behlendorf
added
Type: Performance
behlendorf
reviewed
Apr 10, 2023
There was a problem hiding this comment.
This makes sense to me as an optimization for those who enable quotas and then decide to disable them. What's pretty surprising through is how much overhead there is in zap_lookup() since it should all be cached in the ARC. It seems like there's some potential for additional optimization here in a follow up PR.
behlendorf
added
the
Status: Revision Needed
In the zfs_id_over*quota functions, there is a short-circuit to skip the zap_lookup when the quota zap does not exist. If quotas are never used in a zpool, then the quota zap will never exist. But if user/group/project quotas are ever used, the zap objects will be created and will persist even if the quotas are deleted. The quota zap_lookup in the write path can become a bottleneck for write-heavy small I/O workloads. Before this commit, it was not possible to remove this lookup without creating a new zpool. Signed-off-by: Sam Atkinson <[email protected]>
atkinsam
force-pushed
the
remove-quota-zap
branch
from
36aea9f to
5948c15
Compare
behlendorf
removed
the
Status: Revision Needed
behlendorf
approved these changes
Oct 20, 2023
behlendorf
added
Status: Accepted
|
It may be a coincidence, but in my #15428 I hit such an unexpected panic: |
|
Hmm, it doesn't appear to be a coincidence. I just saw we hit the same issue in other PRs which were recently rebased. Let me see about reverting this change. |
|
Thanks for taking care of the revert, I'll take a look at this. |
lundman
pushed a commit
to openzfsonwindows/openzfs
that referenced
this pull request
Dec 12, 2023
In the zfs_id_over*quota functions, there is a short-circuit to skip the zap_lookup when the quota zap does not exist. If quotas are never used in a zpool, then the quota zap will never exist. But if user/group/project quotas are ever used, the zap objects will be created and will persist even if the quotas are deleted. The quota zap_lookup in the write path can become a bottleneck for write-heavy small I/O workloads. Before this commit, it was not possible to remove this lookup without creating a new zpool. Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Sam Atkinson <[email protected]> Closes openzfs#14721
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Motivation and Context
This change provides up to a ~300% performance improvement in the case where a user creates a zpool and adds a user/group/project quota, and then removes all user/group/project quotas. In this case, today the write path will still do a zap_lookup on the quota object(s). Even when all quotas are deleted.
Description
In the zfs_id_over*quota functions, there is a short-circuit to skip the zap_lookup when the quota zap does not exist. If quotas are never used in a zpool, then the quota zap will never exist. But if user/group/project quotas are ever used, the zap objects will be created and will persist even if the quotas are deleted.
The quota zap_lookup in the write path can become a bottleneck for write-heavy small I/O workloads. Before this commit, it was not possible to remove this lookup without creating a new zpool.
During a write-heavy workload I was testing, I realized that these zap objects are a huge contributor to the total usage of zap_lookup:
And a big contributor to CPU. In one of my workloads, ~90% of CPU time is spent in zfs_id_overblockquota:
How Has This Been Tested?
Created some user/group quotas and did some writes. Then verified that when I set my user/group quotas back to 0, the quota zap object is removed.
A few test cases run with ior, comparing performance before/after this change when I added and then removed quotas on my file system:
Types of changes
Checklist:
Signed-off-by.