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

Hi Tim,

Sorry for my late replay,

Please see my inline coments.

>-----Original Message-----
>From: Bird, Tim <[email protected]>
>Sent: 21 April 2022 05:55
>To: pyla venkata(=1B$B#T#S#I#P=1B(B) <[email protected]>; nakk=
ala
>sireesha(=1B$B#T#S#I#P=1B(B) <[email protected]>
>Cc: [email protected]; dinesh kumar(=1B$B#T#S#I#P=1B(B)
><[email protected]>; hayashi kazuhiro(=1B$BNS=1B(B =1B$BOB9(=
=1B(B =1B$B""#S#W#C"~#A#C#T=1B(B)
><[email protected]>
>Subject: RE: [PATCH 1/1] fio/parser.py: Add support to parse later version=
s of fio
>2.16
>
>Hey Venkata,
>
>See my comments inline below.
>
>> -----Original Message-----
>> From: [email protected] <[email protected]>
>>
>> Hi Tim,
>>
>> Thanks for the prompt response and support always,
>>
>> 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 upgrade 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 ni=
ce 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 b=
reaking
>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 b=
etween
>the different fio versions 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 =
output also
>included 'aggrb=3D'
>    - this meant that the parsing change that was submitted failed, becaus=
e the
>conditional was not correct
> - with 'fio' version 3.16 (another version available to me), the output i=
ncluded
>the 'bw=3D' string
>
The condition check problem we will re-verify it and send the corrected ver=
sion.

Regarding the fio result output change , the main change we observed is
 'aggrb=3D' to 'bw=3D', and there are other changes but we are not using th=
em in our parsing logic.


>However, the Fuego test had other problems with subsequent fio versions,
>because one of the test elements has been deprecated.  I got the following
>messages in my testlog.txt:
>
>./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 t=
estlogs
>for the fio tests on your systems?)

In other mail chain I have sent the RFC patch, in that we fixed this proble=
m,
 kindly please check it

>
>I checked, and it would be a bit more complicated to fix that issue. It ap=
pears
>that the example workload files have not been updated to match the 'fio' b=
inary
>(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 state=
ment
>rwmixcycle=3D40, the Ubuntu 3.16 package for fio, even though this directi=
ve 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 turned =
out to
>be more work than I expected.
>
>>
>> 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 cha=
nge
>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 tes=
t outputs
>are simple enough that I think it's better to keep them in one parser.py, =
for the
>current set of test measures.  However, maybe you desire to add other
>measures to the Benchmark?
>
>There certainly is a lot more data that could be extracted from the test o=
utput,
>that might be of interest.  I haven't compared the rest of the output (onl=
y the
>lines that the parser.py is currently examining), to see if having a singl=
e parser is
>feasible.  If not, then using different parsers based on version is possib=
le.  But I'd
>still lean towards just using version checks in the current parser (and on=
ly having
>a single parser.py module) to handle the different fio versions' results o=
utput.
>
>>
>> Also, please find my inline comments in below.
>>
>> Thanks,
>> Venkata.
>>
>> >-----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 if 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 chan=
ge?
>> >> >
>> >> Currently we don't have any other changes, in future if we see any
>> >> issue will
>> >share with you.
>> >I figured you were running 'fio' from a distribution package (not the
>> >fio 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 modified 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 tes=
t
>configurations present in test_deploy function.
>> But they are little complex to apply them in upstream, so we decided
>> to currently keep them local, may be one day I will make it simple and f=
orward
>the patch to upstream.
>
>OK - if you wouldn't mind, can you send a diff as an RFC?  (RFC=3DRequest =
For
>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 y=
our
>own differences to the base code.
I have shared the RFC patch for the change we are doing, but that is not th=
e correct patch for
Upstream anyway, I may work to improve it  and send you more appropriate fo=
r the upstream,
But meanwhile if you have any comment please provide in that patch.

>
>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 you=
r use
>cases and the tests that are most important to you, to make sure they get
>needed maintenance and that I avoid breaking them.

Basically in my project we are using fuego with non-jenkins version (ftc co=
mmand)
 and for the benchmark tests we use Debian provided versions which are inst=
alled in the=20
target board.

As I understood fuego is using the fixed test version for their Benchmark/F=
unctional tests
I think this is  helpful for comparing the results with the same test versi=
ons, and also to use
fuego with different test versions maybe we can make fuego more easily conf=
igurable to adopt=20
different test versions.
This is just an idea, I am not sure about the complexity or it deviates the=
 goal of this project.=20

>
>It might be worthwhile to have a conference call sometime, to share
>information 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 setu=
ps
>that I believe are similar to how Toshiba is using the tool, and I think m=
y work
>could be better if I knew more about your experience and current practices=
 with
>Fuego
>
>Let me know what you think and if you'd be amenable to a call sometime.
I think it is good idea to share our thoughts in the call.
Let me know your available times=20
For me it is good in following dates {11,12,13}/May,=20
any time between 2 ~ 5PM (Indian Standard Time)

> -- Tim