Re: [Fuego] [PATCH 1/1] fio/parser.py: Add support to parse later versions of fio 2.16

Hey Venkata,

See my comments inline below.

> -----Original Message-----
> From: [email protected] <[email protected]>
>=20
> Hi Tim,
>=20
> Thanks for the prompt response and support always,
>=20
> I would like know, whether there is any policy for keeping the Benchmark =
test versions in the fuego-core,
> I mean the current version of fio in fuego-core is 2.0.8, but can we upgr=
ade the version to the latest version (may be 3.30)?
> Because the new versions got more details in the results, which may help =
us to use it.
> I don't have strong reason to upgrade it to latest version, but it is nic=
e to have latest version as they give more details.

Upgrading 'fio' seems like something that would not be a problem.  There's =
no hard and fast policy, but
I try hard to be backwards-compatible to avoid breaking any existing users =
and their workflows.

In the case of 'fio', your team has found the parsing differences between '=
fio' versions.  I would want to
look at whether there are results differences between the different fio ver=
sions as well.

I did some experimenting here, and found a few things:
 - with 'fio' version 2.0.8 (the version that Fuego ships source for), the =
output includes the 'aggrb=3D' string
 - with 'fio' version 2.2.10 (a version I tried on one of my boards), the o=
utput also included 'aggrb=3D'
    - this meant that the parsing change that was submitted failed, because=
 the conditional was not correct
 - with 'fio' version 3.16 (another version available to me), the output in=
cluded the 'bw=3D' string

However, the Fuego test had other problems with subsequent fio versions, be=
cause one of the test
elements has been deprecated.  I got the following messages in my testlog.t=
xt:

./min1.default.6.6/consolelog.txt:fio: failed parsing rwmixcycle=3D40
./min1.default.6.6/consolelog.txt:Option rwmixcycle is deprecated
This is due to the test using the workload files from the 2.0.8 version of =
the test, but a later binary
where the rwmixcycle sub-workload is not supported.

(As an aside, are you seeing this message in any of your console logs or te=
stlogs for the fio tests
on your systems?)

I checked, and it would be a bit more complicated to fix that issue. It app=
ears that the
example workload files have not been updated to match the 'fio' binary (in =
the upstream
project), even for 3.x version binary packages. So these likely need to be =
patched when
using even updated workload files.
(for example, /usr/share/doc/fio/examples/fsx.fio still includes the statem=
ent
rwmixcycle=3D40, the Ubuntu 3.16 package for fio, even though this directiv=
e is obsolete
in the 'fio' binary for that package.)

So, I'm happy to upgrade base packages, but there must be sufficient time
and testing to make sure nothing breaks.  The fio parser.py change has turn=
ed out to
be more work than I expected.

>=20
> As you may know, in Toshiba we were using Debian based system, so we use =
Debian versions of Benchmark tests
> and we skip the build and deploy phases while running the fuego Benchmark=
 tests, but we had to change the parsing logic of test results
> such that it had to parse old and newer version of the test results.
> Can we have mechanism to choose the parser.py based on test version?
> May be like below just an idea, need to check how feasible it is
> parser_2.0.8.py
> parser_3.25.py

This is possible, but so far the changes between the different version test=
 outputs are
simple enough that I think it's better to keep them in one parser.py, for t=
he current
set of test measures.  However, maybe you desire to add other measures to t=
he
Benchmark?

There certainly is a lot more data that could be
extracted from the test output, that might be of interest.  I haven't compa=
red
the rest of the output (only the lines that the parser.py is currently exam=
ining),
to see if having a single parser is feasible.  If not, then using different=
 parsers
based on version is possible.  But I'd still lean towards just using versio=
n checks
in the current parser (and only having a single parser.py module) to handle
the different fio versions' results output.

>=20
> Also, please find my inline comments in below.
>=20
> Thanks,
> Venkata.
>=20
> >-----Original Message-----
> >From: Bird, Tim <[email protected]>
> >
> >> -----Original Message-----
> >> From: [email protected]
> >...
> >> >
> >> > I'm a little confused about how you are executing an fio version
> >> > greater than 2.0.8, which is the one installed by fuego_test.sh.
> >> >
> >> We are using the fio from Debian package system which is fio-3.25
> >> version and that is pre-installed in the board, so we wanted to update=
 the
> >parsing logic in the fuego-core to report the results correctly.
> >>
> >> We contributing this to upstream so that it will be useful in future i=
f the fuego-
> >core test are upgraded with later versions.
> >>
> >> > This test doesn't have logic that detects and uses a different
> >> > version of the 'fio' binary if it is already on the board.  Do you
> >> > have other changes to this test to go along with the parse change?
> >> >
> >> Currently we don't have any other changes, in future if we see any iss=
ue will
> >share with you.
> >I figured you were running 'fio' from a distribution package (not the fi=
o binary
> >built by the Fuego test).
> >My question was "how"?
> >
> >On my system, even if I have a newer version of 'fio' installed in /usr/=
bin, Fuego
> >still executes /home/fuego/fuego.Benchmar.fio/fio
> >
> >I missed the part of fuego_test.sh where 'get_program_path fio' is used.
> >Apparently in your lab, you either do not run test_deploy, or you have m=
odified
> >the test_deploy() function to not put 'fio' into
> >$BOARD_TESTDIR/fuego.$TESTDIR.
> >
> >That's the part I was asking about.
> >
> >I was wondering if you had something like this in your
> >Benchmark.fio/feugo_test.sh:test_deploy() function:
> >if ! cmd test -f /usr/bin/fio ; then
> >   put fio $BOARD_TESTDIR/fuego.$TESTDIR fi
>
> Thanks for reminding this, we have a change similar to that to apply test=
 configurations present in test_deploy function.
> But they are little complex to apply them in upstream, so we decided to c=
urrently keep them local, may be one day
> I will make it simple and forward the patch to upstream.

OK - if you wouldn't mind, can you send a diff as an RFC?  (RFC=3DRequest F=
or Comments) to
the mailing list?

This is not a patch submission, but rather just a patch that indicates how =
you've done something,
for possible discussion on the mailing list.  I'd like to take a look at it=
, if you wouldn't mind.

It *may be* too complicated to apply to the upstream code, but it may also =
give me ideas
for how to support your use case without you having to maintain your own di=
fferences
to the base code.

In general I appreciate the patches you are sending, and want to make sure
that Fuego continues to support your use cases.  And I'd like to understand=
 your use cases
and the tests that are most important to you, to make sure they get needed =
maintenance
and that I avoid breaking them.

It might be worthwhile to have a conference call sometime, to share informa=
tion and
let me know how best to have Fuego support your testing efforts.
I have been working recently on supporting more use cases and install setup=
s that
I believe are similar to how Toshiba is using the tool, and I think my work=
 could
be better if I knew more about your experience and current practices with F=
uego

Let me know what you think and if you'd be amenable to a call sometime.
 -- Tim