feat: update block perf tests to use fio json #4780
Conversation
ShadowCurse
force-pushed
the
fio_json
branch
3 times, most recently
from
fbd02c4 to
632dfb6
Compare
ShadowCurse
added
Status: Awaiting review
ShadowCurse
requested review from
pb8o and
roypat
and removed request for
roypat
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4780 +/- ##
=======================================
Coverage 84.34% 84.34%
=======================================
Files 249 249
Lines 27460 27460
=======================================
Hits 23160 23160
Misses 4300 4300
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Can you elaborate a bit more on what the problem was here, and why the json format fixes it? I thought the fio logs contained "kilobytes send since the last log line", with log lines being emitted every second. What do you mean by them being "kb/s" and "wrong to sum up"? |
ShadowCurse
force-pushed
the
fio_json
branch
from
632dfb6 to
6591ea5
Compare
Block tests were using fio logs to calculate bandwidth. The issue is that the values in logs are in KB/sec and we were just summing them up, so getting a sum of averages. But what we want is just an average bandwidth for the test. Now we use json output of fio which contains much more info in a nice format. This info already includes all bandwidth calculations. This change will also allow us to output other metrics in the future if we want. Signed-off-by: Egor Lazarchuk <[email protected]>
ShadowCurse
force-pushed
the
fio_json
branch
from
6591ea5 to
032beb5
Compare
|
Each line in the log contains |
There was a problem hiding this comment.
Ah, I see now. This won't work with our A/B-tests, because they require a data series to be emitted, not just a single data point.
I admittedly still don't quite understand what was wrong with the old approach. It gives you a data series where each entry is "number of bytes written/read since the last entry", with entries being emitted once per second. That's different from "kb/s", because its not "average bandwidth since the test started". So it's fine to sum up the data points at each second (and if it weren't, I don't see why your summing up of all datapoints wouldn't be just as problematic).
Edit (we commented at the same time 😔):
Each line in the log contains value seen over the specified period of time (I assume this is an average over 1s because we specify to log every second). So each line contains KB/s (not just KB). We were summing these KB/s and were getting a sum of averages.
Well, to compute KB/s, you first need to count the number of kilobytes you observe during some duration, and then divide this number by the duration. In this case, we count the number of kilobytes observed during one second, and then divide by 1. So "KB/s=KB", if you excuse the abuse of notation
3 participants
Changes
Block tests were using fio logs to calculate bandwidth. The issue is that the values in logs are in KB/sec and we were just summing them up, so getting a sum of averages. But what we want is just an average bandwidth for the test.
Now we use json output of fio which contains much more info in a nice format. This info already includes all bandwidth calculations. This change will also allow us to output other metrics in the future if we want.
Reason
More correct block performance 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
PR.
CHANGELOG.md.TODOs link to an issue.contribution quality standards.
rust-vmm.