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

zdb: add extra -T flag to show histograms of BRT refcounts #16692

Merged
merged 1 commit into from
Nov 1, 2024

Conversation

robn
Copy link
Member

@robn robn commented Oct 27, 2024

Motivation and Context

I was comparing my local BRT use against what fast dedup might get me1, and realised I would like a nice breakdown of refcount distribution. So I made one.

Description

Repurposes -TTT to show histograms. The full per-DVA list is now -TTTT.

How Has This Been Tested?

By hand:

$ ./zdb -TTT crayon
BRT: used 292M; saved 309M; ratio 2.05x
BRT: vdev 0: refcnt 12.2K; used 292M; saved 309M

BRT: vdev 0: DVAs with 2^n refcnts:
			 1:  11788 ****************************************
			 2:    645 ***
			 3:     40 *
			 4:      3 *
			 5:      1 *

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

Footnotes

  1. Turns out BRT is getting me basically everything I'd get from dedup. dedup would be marginally better at the cost of 11M unique entries. No thanks.

Copy link
Member

@amotin amotin left a comment

Choose a reason for hiding this comment

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

As I read it, you are counting references with certain number of references, not DVAs with certain number of references. I guess what you want may depend on what you are going to compare it against, but the histogram title seems not match. Am I wrong? For DDT we seem to count both.

@robn robn force-pushed the zdb-brt-histograms branch from f0d56d3 to df2e02a Compare October 27, 2024 22:09
@robn
Copy link
Member Author

robn commented Oct 27, 2024

Oh ugh, yeah. I meant to be counting DVAs. I'm not quite sure what I was thinking (at 11pm, probably not at all). Pushed what I think I meant.

@amotin amotin added the Status: Code Review Needed Ready for review and testing label Oct 29, 2024
@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Nov 1, 2024
@amotin amotin merged commit 673efbb into openzfs:master Nov 1, 2024
20 checks passed
behlendorf pushed a commit to behlendorf/zfs that referenced this pull request Nov 1, 2024
Sponsored-by: https://despairlabs.com/sponsor/
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Signed-off-by: Rob Norris <[email protected]>
Closes openzfs#16692
robn added a commit to robn/zfs that referenced this pull request Nov 5, 2024
Sponsored-by: https://despairlabs.com/sponsor/
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Signed-off-by: Rob Norris <[email protected]>
Closes openzfs#16692
(cherry picked from commit 673efbb)
@robn robn mentioned this pull request Nov 5, 2024
13 tasks
ptr1337 pushed a commit to CachyOS/zfs that referenced this pull request Nov 14, 2024
Sponsored-by: https://despairlabs.com/sponsor/
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Signed-off-by: Rob Norris <[email protected]>
Closes openzfs#16692
(cherry picked from commit 673efbb)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants