JSON output "steadystate" section - bandwidth is in bytes instead of kilobytes

[louwrentius]

Background:

JSON OUTPUT
The json output format is intended to be both human readable and convenient for automated parsing. For the most part its sections mirror those of the normal output. The runtime value is reported in
msec and the bw value is reported in 1024 bytes per second units.

Unfortunately, it seems that the bandwidth numbers in the steadystate section are in bytes/s. (see data below)

I would like to request to scale this data to kilobytes too, conform the standard for JSON OUTPUT, if possible.

"steadystate" : {
"ss" : "iops:1.000000%",
"duration" : 10,
"attained" : 1,
"criterion" : "0.744490%",
"max_deviation" : 126.800000,
"slope" : 0.000000,
"data" : {
"bw_mean" : 69762791,
"iops_mean" : 17031,
"iops" : [
17040,
16968,
17026,
17115,
17014,
17148,
16905,
17128,
17002,
16972
],
"bw" : [
69796004,
69500928,
69738602,
70103040,
69689344,
70238814,
69242880,
70156812,
69640192,
69521297
]

I double-checked that the bandwidth was around 70 MB/s with dstat when that benchmark was run.

[sitsofe]

It's probably too late to change the units for steady state bw (otherwise it breaks all existing users) but I would guess another key could be introduced called bw_kb that duplicates the values but scales them?

[louwrentius]

This metric is inconsistent within the format but that is what it is.

At this point it's maybe better to explicitly call this out in the documtation and leave everything as-is.

Even with the new metric, I want to support older Fio-versions so I have to keep my special-case code inplace for a long time.

I would rather see an allert to all Fio users that the metric is format is going to change months in advace and just check the fio version in order to know how to parse the data.

(How many people would care?(I have no idea))

At some point in the future (maybe a year) I can drop the special-case code and 'require' a minimal Fio version.

That would make the JSON data fully consistent

How wide-spread is the steadystate code?

[sitsofe]

@louwrentius Well I guess my thinking was that if we introduce bw_kb we could then have a transition period and then remove the old value entirely at some point...

[louwrentius]

That would be fine by me, for what it's worth :)

[sitsofe]

@louwrentius so digging around I've found #378 (comment) . Looking closer, there is a mishmash of bytes/kbytes scattered across the JSON output which is defeating consistency...

What are your thoughts on moving in this direction:

• Where there are bandwidth values, ensure there's a key which ends in "_bytes" with a value in bytes
• When introducing these new values, group them in the same way as the latency values are grouped today
• Do similar grouping for iops
• Update the documentation to say bandwidth is kbytes except when there's a clear suffix or it's steady state and keys X, Y, Z are deprecated and will go away in the future
• Introduce a JSON format version key which only changes when backwards incompatible changes are done

[louwrentius]

Hello Sitsofe, That is some digging! I think this is a good approach, the JSON version key is also awesome, easy to check than keeping track of a minimum Or specific Fio version. With regards, Louwrentius

…

On 23 Dec 2020, at 11:46, Sitsofe Wheeler ***@***.***> wrote:  @louwrentius so digging around I've found #378 (comment) . Looking closer, there is a mishmash of bytes/kbytes scattered across the JSON output which is defeating consistency... What are your thoughts on moving in this direction: Where there are bandwidth values, ensure there's a key which ends in "_bytes" with a value in bytes When introducing these new values, group them in the same way as the latency values are grouped today Do similar grouping for iops Update the documentation to say bandwidth is kbytes except when there's a clear suffix or it's steady state and keys X, Y, Z are deprecated and will go away in the future Introduce a JSON format version key which only changes when backwards incompatible changes are done — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.