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

test(perf): Accumulated Block Test fixes + baselines #3940

Merged
merged 7 commits into from
Jul 7, 2023

Conversation

roypat
Copy link
Contributor

@roypat roypat commented Jul 5, 2023

This PR accumulates some fixes to the block performance test that all each require new baselines. These include:

  • Pinning the fio workers to vcpus inside of the guest to prevent guest-scheduling to impact test results
  • Change parameterization from baseline level to pytest level to improve readability of test progress and do not reuse microvms across multiple tests
  • Introduce a write test, as the randrw fio mode tries to balance reads and writes, leading to its results being highly correlated with the read tests.

License Acceptance

By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license. For more information on following
Developer Certificate of Origin and signing off your commits, please check
CONTRIBUTING.md.

PR Checklist

  • If a specific issue led to this PR, this PR closes the issue.
  • The description of changes is clear and encompassing.
  • Any required documentation changes (code and docs) are included in this PR.
  • API changes follow the Runbook for Firecracker API changes.
  • User-facing changes are mentioned in CHANGELOG.md.
  • All added/changed functionality is tested.
  • New TODOs link to an issue.
  • Commits meet contribution quality standards.

  • This functionality cannot be added in rust-vmm.

@roypat roypat added the Status: Awaiting review Indicates that a pull request is ready to be reviewed label Jul 5, 2023
roypat added 2 commits July 6, 2023 11:11
Make sure that fio workers inside the guest during block performance
test are pinned to specific vcpus.

Analyzing the raw data from block performance tests indicated that each
fio worker has a unique performance characteristic, but that two workers
sometimes "swap" their characteristics. I am hypothesizing that this is
due to scheduling inside the guest.

Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
The variable determined how many fio workers should be started per vcpu.
It was always set to 1, and its unclear why we would ever want it to be
not 1, or what the impact of that would be.

Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
@roypat roypat force-pushed the pin-fio-process-in-guest branch from 79545ee to 067b1ae Compare July 6, 2023 10:11
@roypat roypat marked this pull request as draft July 6, 2023 13:03
roypat added 3 commits July 6, 2023 14:33
Makes its easier to see what tests are run/failing.

Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
The randrw fio mode tries to perfectly balance reads and writes. This
can be seens from the perfect correlation between the bw_read and
bw_write metrics (r=1) emitted by this test. This means we really only
measure read OR write throughput (whicheverone is lower throttles the
other). Therefore, to make sure we actually measure write throughput,
use randread and randwrite as our two fio modes.

Also removes the associated baselines. New baselines will be gathered in
follow up.

Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
this caused pytest to crash during test collection if running on an
unsupported kernel version.

Fixes firecracker-microvm#3726

Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
@roypat roypat force-pushed the pin-fio-process-in-guest branch from cf8f02b to e374bcb Compare July 6, 2023 14:11
Collect new block baselines for all instances/kernel versions since we
added a new test case, and the change of parameterization from
baseline-level to pytest-level caused a change in performance behavior
across the board (since previously, multiple tests would reuse the same
microvm).

Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
@roypat roypat changed the title test(perf): block pin fio workers test(perf): Accumulated Block Test fixes + baselines Jul 7, 2023
@roypat roypat marked this pull request as ready for review July 7, 2023 07:54
@roypat roypat requested a review from pb8o July 7, 2023 08:28
@roypat roypat requested a review from kalyazin July 7, 2023 09:11
@roypat roypat merged commit a539998 into firecracker-microvm:main Jul 7, 2023
@roypat roypat deleted the pin-fio-process-in-guest branch April 15, 2024 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Awaiting review Indicates that a pull request is ready to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants