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

zstreamdump: add per-record-type counters and an overhead counter #8432

Merged
merged 1 commit into from
Jun 22, 2019

Conversation

allanjude
Copy link
Contributor

@allanjude allanjude commented Feb 19, 2019

Signed-off-by: Allan Jude allanjude@freebsd.org

Motivation and Context

zstreamdump is a tool used to analyze the contents of ZFS replication streams.
I am using zstreamdump to debug zfs send size estimation, and found it
useful to get a breakdown of the actual size of the replication stream, and
what that size was made up of.

Description

Count the bytes of payload for each replication record type
Count the bytes of overhead (replication records themselves)
Include these counters in the output summary at the end of the run.

Example output:

SUMMARY:
SUMMARY:
        Total DRR_BEGIN records = 5 (752 bytes)
        Total DRR_END records = 6 (0 bytes)
        Total DRR_OBJECT records = 764290 (130437264 bytes)
        Total DRR_FREEOBJECTS records = 39 (0 bytes)
        Total DRR_WRITE records = 504587 (3902546432 bytes)
        Total DRR_WRITE_BYREF records = 0 (0 bytes)
        Total DRR_WRITE_EMBEDDED records = 267961 (17836736 bytes)
        Total DRR_FREE records = 768868 (0 bytes)
        Total DRR_SPILL records = 0 (0 bytes)
        Total records = 2305756
        Total payload size = 4050821184 (0xf172a040)
        Total header overhead = 719395872 (0x2ae11c20)
        Total stream length = 4770217056 (0x11c53bc60)

How Has This Been Tested?

The counters correctly sum up to the total stream length, which matches the actual stream length.

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)
  • Documentation (a change to man pages or other documentation)

Checklist:

@allanjude allanjude force-pushed the zstreamdump_counters branch 2 times, most recently from a92e53e to ea49825 Compare February 20, 2019 02:15
@bunder2015 bunder2015 added the Status: Code Review Needed Ready for review and testing label Feb 20, 2019
Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

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

Thanks, this'll be really handy. Just a few suggestions.

cmd/zstreamdump/zstreamdump.c Outdated Show resolved Hide resolved
cmd/zstreamdump/zstreamdump.c Show resolved Hide resolved
cmd/zstreamdump/zstreamdump.c Outdated Show resolved Hide resolved
@behlendorf behlendorf added Status: Revision Needed Changes are required for the PR to be accepted and removed Status: Code Review Needed Ready for review and testing labels Mar 1, 2019
@ahrens
Copy link
Member

ahrens commented Mar 3, 2019

I think the summary should have a "total payload size" and "total header overhead", the sum of which would be the "total stream length". The "total payload" seems more relevant than "total WRITE record payload" which is what we have today (I think).

@behlendorf
Copy link
Contributor

Thanks for refreshing this. It looks like several of the rsend tests failed because they parse the output of zstreamdump. I'd expect that updating the verify_stream_size function in zfs-tests/tests/functional/rsend/rsend.kshlib would should resolve the issue.

Count the bytes of payload for each replication record type

Count the bytes of overhead (replication records themselves)

Include these counters in the output summary at the end of the run.

Signed-off-by: Allan Jude <allanjude@freebsd.org>
Sponsored-By: Klara Systems and Catalogic
@codecov
Copy link

codecov bot commented Jun 14, 2019

Codecov Report

Merging #8432 into master will increase coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8432      +/-   ##
==========================================
+ Coverage   78.51%   78.58%   +0.06%     
==========================================
  Files         382      382              
  Lines      117840   117858      +18     
==========================================
+ Hits        92526    92613      +87     
+ Misses      25314    25245      -69
Flag Coverage Δ
#kernel 79.27% <ø> (+0.04%) ⬆️
#user 66.84% <100%> (-0.19%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b1b4ac2...800ada4. Read the comment docs.

@behlendorf behlendorf added Status: Code Review Needed Ready for review and testing and removed Status: Revision Needed Changes are required for the PR to be accepted labels Jun 14, 2019
@behlendorf behlendorf requested a review from ahrens June 14, 2019 18:59
@@ -683,6 +687,7 @@ main(int argc, char *argv[])
print_block(buf,
P2ROUNDUP(drrwe->drr_psize, 8));
}
payload_size = P2ROUNDUP(drrwe->drr_psize, 8);
Copy link
Member

Choose a reason for hiding this comment

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

If you feel like cleaning this up a bit, we could avoid repeating the P2ROUNDUP 3x:

payload_size = ...;
ssread(... payload_size ...);
if (dump)
  print_block (... payload_size)

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Jun 20, 2019
@behlendorf behlendorf merged commit fb6e6f1 into openzfs:master Jun 22, 2019
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Aug 13, 2019
Count the bytes of payload for each replication record type

Count the bytes of overhead (replication records themselves)

Include these counters in the output summary at the end of the run.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Matt Ahrens <mahrens@delphix.com>
Signed-off-by: Allan Jude <allanjude@freebsd.org>
Sponsored-By: Klara Systems and Catalogic
Closes openzfs#8432
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Aug 22, 2019
Count the bytes of payload for each replication record type

Count the bytes of overhead (replication records themselves)

Include these counters in the output summary at the end of the run.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Matt Ahrens <mahrens@delphix.com>
Signed-off-by: Allan Jude <allanjude@freebsd.org>
Sponsored-By: Klara Systems and Catalogic
Closes openzfs#8432
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Aug 23, 2019
Count the bytes of payload for each replication record type

Count the bytes of overhead (replication records themselves)

Include these counters in the output summary at the end of the run.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Matt Ahrens <mahrens@delphix.com>
Signed-off-by: Allan Jude <allanjude@freebsd.org>
Sponsored-By: Klara Systems and Catalogic
Closes openzfs#8432
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Sep 17, 2019
Count the bytes of payload for each replication record type

Count the bytes of overhead (replication records themselves)

Include these counters in the output summary at the end of the run.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Matt Ahrens <mahrens@delphix.com>
Signed-off-by: Allan Jude <allanjude@freebsd.org>
Sponsored-By: Klara Systems and Catalogic
Closes openzfs#8432
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Sep 18, 2019
Count the bytes of payload for each replication record type

Count the bytes of overhead (replication records themselves)

Include these counters in the output summary at the end of the run.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Matt Ahrens <mahrens@delphix.com>
Signed-off-by: Allan Jude <allanjude@freebsd.org>
Sponsored-By: Klara Systems and Catalogic
Closes openzfs#8432
tonyhutter pushed a commit that referenced this pull request Sep 26, 2019
Count the bytes of payload for each replication record type

Count the bytes of overhead (replication records themselves)

Include these counters in the output summary at the end of the run.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Matt Ahrens <mahrens@delphix.com>
Signed-off-by: Allan Jude <allanjude@freebsd.org>
Sponsored-By: Klara Systems and Catalogic
Closes #8432
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.

4 participants