Skip to content

Assorted fixes for the performance tests #12408

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 26, 2021
Merged

Assorted fixes for the performance tests #12408

merged 1 commit into from
Jul 26, 2021

Conversation

jwk404
Copy link
Contributor

@jwk404 jwk404 commented Jul 21, 2021

Motivation and Context

The performance suite has some minor outstanding issues.

Description

  • Allow the tests to bypass checks on the number of disks
  • Prevent the tests from running with loopback devices
  • Fix calculation of dbuf cache size in sequential_reads_dbuf_cached
  • Clean up redundant variables for better readability

How Has This Been Tested?

I've run the performance tests, and verified the logic around the number of disks
as well as the dbuf cache size calculations.

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:

behlendorf
behlendorf approved these changes Jul 21, 2021
View reviewed changes
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.

Looks good, it's nice to see the code consolidation. My only comment is regarding switching to zinject -a to drop the cache. This behavior will be very close to the export/import logic but it is possible for some of the dbufs not to be dropped if they're referenced. But for these tests that shouldn't be the case so I don't expect it to an issue.

@behlendorf behlendorf added Status: Code Review Needed Ready for review and testing Type: Performance Performance improvement or performance problem Component: Test Suite Indicates an issue with the test framework or a test case labels Jul 21, 2021
tonynguien
tonynguien approved these changes Jul 23, 2021
View reviewed changes
Copy link
Contributor

@tonynguien tonynguien left a comment

Choose a reason for hiding this comment

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

Thanks for making these changes.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Jul 23, 2021
@jwk404
Assorted fixes for the performance tests
472caf5 
- Bail out early if we're running the perf tests and forget to
  specify disks.
- Allow perf tests to run with any number of disks.
- Remove weekly vs. nightly settings
- Move variables with common values to perf.shlib
- Use zinject to clear the ARC over export/import

- Fix dbuf cache size calculation

When the meaning of `dbuf_cache_max_bytes` changed, the performance
test that covers the dbuf cache started to fail. The test would try to
write files for the test using the max possible size of the cache,
inevitably filling the pool and failing. This change uses
`dbuf_cache_shift` to correctly calculate the dbuf cache size.

Signed-off-by: John Kennedy <[email protected]>
@tonynguien tonynguien merged commit bdd2bfd into openzfs:master Jul 26, 2021
behlendorf pushed a commit to behlendorf/zfs that referenced this pull request Aug 23, 2021
@jwk404 @behlendorf
Assorted fixes for the performance tests
1a586ed 
- Bail out early if we're running the perf tests and forget to
  specify disks.
- Allow perf tests to run with any number of disks.
- Remove weekly vs. nightly settings
- Move variables with common values to perf.shlib
- Use zinject to clear the ARC over export/import
- Fix dbuf cache size calculation

When the meaning of `dbuf_cache_max_bytes` changed, the performance
test that covers the dbuf cache started to fail. The test would try to
write files for the test using the max possible size of the cache,
inevitably filling the pool and failing. This change uses
`dbuf_cache_shift` to correctly calculate the dbuf cache size.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Tony Nguyen <[email protected]>
Reviewed-by: George Melikov <[email protected]>
Signed-off-by: John Kennedy <[email protected]>
Closes openzfs#12408
behlendorf pushed a commit to behlendorf/zfs that referenced this pull request Aug 24, 2021
@jwk404 @behlendorf
Assorted fixes for the performance tests
24d48fa 
- Bail out early if we're running the perf tests and forget to
  specify disks.
- Allow perf tests to run with any number of disks.
- Remove weekly vs. nightly settings
- Move variables with common values to perf.shlib
- Use zinject to clear the ARC over export/import
- Fix dbuf cache size calculation

When the meaning of `dbuf_cache_max_bytes` changed, the performance
test that covers the dbuf cache started to fail. The test would try to
write files for the test using the max possible size of the cache,
inevitably filling the pool and failing. This change uses
`dbuf_cache_shift` to correctly calculate the dbuf cache size.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Tony Nguyen <[email protected]>
Reviewed-by: George Melikov <[email protected]>
Signed-off-by: John Kennedy <[email protected]>
Closes openzfs#12408
behlendorf pushed a commit to behlendorf/zfs that referenced this pull request Aug 24, 2021
@jwk404
Assorted fixes for the performance tests
b4927c2 
- Bail out early if we're running the perf tests and forget to
  specify disks.
- Allow perf tests to run with any number of disks.
- Remove weekly vs. nightly settings
- Move variables with common values to perf.shlib
- Use zinject to clear the ARC over export/import
- Fix dbuf cache size calculation

When the meaning of `dbuf_cache_max_bytes` changed, the performance
test that covers the dbuf cache started to fail. The test would try to
write files for the test using the max possible size of the cache,
inevitably filling the pool and failing. This change uses
`dbuf_cache_shift` to correctly calculate the dbuf cache size.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Tony Nguyen <[email protected]>
Reviewed-by: George Melikov <[email protected]>
Signed-off-by: John Kennedy <[email protected]>
Closes openzfs#12408
behlendorf pushed a commit to behlendorf/zfs that referenced this pull request Aug 24, 2021
@jwk404 @behlendorf
Assorted fixes for the performance tests
9360ea1 
- Bail out early if we're running the perf tests and forget to
  specify disks.
- Allow perf tests to run with any number of disks.
- Remove weekly vs. nightly settings
- Move variables with common values to perf.shlib
- Use zinject to clear the ARC over export/import
- Fix dbuf cache size calculation

When the meaning of `dbuf_cache_max_bytes` changed, the performance
test that covers the dbuf cache started to fail. The test would try to
write files for the test using the max possible size of the cache,
inevitably filling the pool and failing. This change uses
`dbuf_cache_shift` to correctly calculate the dbuf cache size.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Tony Nguyen <[email protected]>
Reviewed-by: George Melikov <[email protected]>
Signed-off-by: John Kennedy <[email protected]>
Closes openzfs#12408
behlendorf pushed a commit that referenced this pull request Aug 31, 2021
@jwk404 @behlendorf
Assorted fixes for the performance tests
e089098 
- Bail out early if we're running the perf tests and forget to
  specify disks.
- Allow perf tests to run with any number of disks.
- Remove weekly vs. nightly settings
- Move variables with common values to perf.shlib
- Use zinject to clear the ARC over export/import
- Fix dbuf cache size calculation

When the meaning of `dbuf_cache_max_bytes` changed, the performance
test that covers the dbuf cache started to fail. The test would try to
write files for the test using the max possible size of the cache,
inevitably filling the pool and failing. This change uses
`dbuf_cache_shift` to correctly calculate the dbuf cache size.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Tony Nguyen <[email protected]>
Reviewed-by: George Melikov <[email protected]>
Signed-off-by: John Kennedy <[email protected]>
Closes #12408
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Sep 15, 2021
@jwk404 @tonyhutter
Assorted fixes for the performance tests
9429910 
- Bail out early if we're running the perf tests and forget to
  specify disks.
- Allow perf tests to run with any number of disks.
- Remove weekly vs. nightly settings
- Move variables with common values to perf.shlib
- Use zinject to clear the ARC over export/import
- Fix dbuf cache size calculation

When the meaning of `dbuf_cache_max_bytes` changed, the performance
test that covers the dbuf cache started to fail. The test would try to
write files for the test using the max possible size of the cache,
inevitably filling the pool and failing. This change uses
`dbuf_cache_shift` to correctly calculate the dbuf cache size.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Tony Nguyen <[email protected]>
Reviewed-by: George Melikov <[email protected]>
Signed-off-by: John Kennedy <[email protected]>
Closes openzfs#12408
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@behlendorf behlendorf behlendorf approved these changes

@tonynguien tonynguien tonynguien approved these changes

@gmelikov gmelikov gmelikov approved these changes

Assignees

@tonynguien tonynguien

Labels
Component: Test Suite Indicates an issue with the test framework or a test case Status: Accepted Ready to integrate (reviewed, tested) Type: Performance Performance improvement or performance problem
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jwk404 @behlendorf @tonynguien @gmelikov