Report space metrics per allocation class#18238
Report space metrics per allocation class#18238ryan-moeller wants to merge 2 commits intoopenzfs:masterfrom
Conversation
1567a9b to
2dbe6f5
Compare
|
|
@ryan-moeller it's good to see you back. I've just merged #18222 so you can rebase this and drop that commit from your stack. |
2dbe6f5 to
3f8b2db
Compare
|
Thanks!
|
|
For additional context, here is an early test script and some output demonstrating the new properties: Snippets of new zpool properties after exercising each allocation class |
|
I've created another PR (#18245) adding properties about dedup, so depending which PR will go first, we'll both need to update our ABI files in respective order to make checkstyle CI happy. |
3f8b2db to
940048e
Compare
|
940048e to
76d21a1
Compare
|
|
I'll let #18245 land first, then I can update ABI files and remove the draft status from this PR. |
|
I also decided to add the pool-wide |
|
@ryan-moeller You may rebase. |
76d21a1 to
4a8d523
Compare
I'll grab the updated ABI file from the CI, then push again and remove the draft status. |
4a8d523 to
2ec51ab
Compare
(There are a bunch of unrelated changes in the ABI that I've left out.) |
tests/zfs-tests/tests/functional/cli_root/zpool_get/zpool_get_006_pos.ksh
Outdated
Show resolved
Hide resolved
tests/zfs-tests/tests/functional/cli_root/zpool_get/zpool_get_006_pos.ksh
Show resolved
Hide resolved
tests/zfs-tests/tests/functional/cli_root/zpool_get/zpool_get_005_pos.ksh
Outdated
Show resolved
Hide resolved
|
Also seeing zpool_get_005_pos: |
|
That's what I'm currently looking into. Apparently "dedupused" that was added in this latest rebase conflicts with "dedup_used" in the property lookup logic in libzfs? That's a bit of a problem, as both names were chosen to align with existing names. Maybe I could name the properties like "dedup_class_used" instead? Though they're long enough names already, I'll look at whether there is another way to solve this. |
If you pulled the ABI files from the CI there shouldn't be unrelated. What kind of changes are you seeing here? |
+ |
I think whether we could trade abbreviation of "embedded_log" to "elog", as the longest part, for the addition of "_class". I am not sure we can/should avoid the last in the long run, while the first should make the lengths more uniform. |
2ec51ab to
29d55c7
Compare
|
I was thinking about |
I suppose it still leaves collision on |
|
Ah that was it, the |
29d55c7 to
1a66ac4
Compare
|
|
Any other comments, primarily on property names? Otherwise this looks good to me. |
|
What do you think about moving the |
Capacity is reported as a percentage not a size. Sponsored-by: Klara, Inc. Signed-off-by: Ryan Moeller <ryan.moeller@klarasystems.com>
The existing zpool properties accounting pool space (size, allocated, fragmentation, expandsize, free, capacity) are based on the normal metaslab class or are cumulative properties of several classes combined. Add properties reporting the space accounting metrics for each metaslab class individually. Also introduce pool-wide AVAIL, USABLE, and USED properties reporting values corresponding to FREE, SIZE, and ALLOC deflated for raidz. Update ZTS to recognize the new properties and validate reported values. While in zpool_get_parsable.cfg, add "fragmentation" to the list of parsable properties. Sponsored-by: Klara, Inc. Signed-off-by: Ryan Moeller <ryan.moeller@klarasystems.com>
|
behlendorf
left a comment
There was a problem hiding this comment.
It's a lot of new pool properties, but I agree it'll be nice to have the visibility in to the allocation classes. Extending zpool get to accept keywords like default instead of just all may be a nice way to limit the output to the mostly commonly used properties. That of course would be for a different PR.
Motivation and Context
The existing zpool properties accounting pool space (size, allocated,
fragmentation, expandsize, free, capacity) are based on the normal
metaslab class or are cumulative properties of several classes combined.
It would be useful to have visibility into per-class metaslab allocator stats.
Sponsored by Klara, Inc.
Description
Add properties reporting the existing space accounting metrics for each
metaslab class individually.
Also add pool-wide USABLE and USED properties reporting deflated size and allocated space, respectively.
Update ZTS to recognize the new properties and validate reported values.
While here I noticed an incorrect format description for the vdev capacity
property. This is fixed in a separate commit.
I also found that "fragmentation" was missing from the list of parsable
pool properties in ZTS, so I've added that while reformatting
zpool_get_parsable.cfg to add the new properties.
How Has This Been Tested?
ZTS test added and full test suite passed. The test tries to exercise every
statistic of the metaslab allocation classes.
Types of changes
Checklist:
Signed-off-by.