daos icon indicating copy to clipboard operation
daos copied to clipboard

DAOS-15801 test: Add aio ioengine to pil4dfs_fio.py functional test

Open knard38 opened this issue 1 year ago • 3 comments

Description

Run FIO with the libaio ioengine and the PIL4DFS interception library. Compare the bandwidth of the previous FIO run with one using the dfs ioengine.

Before requesting gatekeeper:

  • [ ] Two review approvals and any prior change requests have been resolved.
  • [ ] Testing is complete and all tests passed or there is a reason documented in the PR why it should be force landed and forced-landing tag is set.
  • [ ] Features: (or Test-tag*) commit pragma was used or there is a reason documented that there are no appropriate tags for this PR.
  • [ ] Commit messages follows the guidelines outlined here.
  • [ ] Any tests skipped by the ticket being addressed have been run and passed in the PR.

Gatekeeper:

  • [ ] You are the appropriate gatekeeper to be landing the patch.
  • [ ] The PR has 2 reviews by people familiar with the code, including appropriate owners.
  • [ ] Githooks were used. If not, request that user install them and check copyright dates.
  • [ ] Checkpatch issues are resolved. Pay particular attention to ones that will show up on future PRs.
  • [ ] All builds have passed. Check non-required builds for any new compiler warnings.
  • [ ] Sufficient testing is done. Check feature pragmas and test tags and that tests skipped for the ticket are run and now pass with the changes.
  • [ ] If applicable, the PR has addressed any potential version compatibility issues.
  • [ ] Check the target branch. If it is master branch, should the PR go to a feature branch? If it is a release branch, does it have merge approval in the JIRA ticket.
  • [ ] Extra checks if forced landing is requested
    • [ ] Review comments are sufficiently resolved, particularly by prior reviewers that requested changes.
    • [ ] No new NLT or valgrind warnings. Check the classic view.
    • [ ] Quick-build or Quick-functional is not used.
  • [ ] Fix the commit message upon landing. Check the standard here. Edit it to create a single commit. If necessary, ask submitter for a new summary.

knard38 avatar May 15 '24 08:05 knard38

Ticket title is 'Add functional test with aio to pil4dfs_fio.py functional test' Status is 'In Review' Labels: 'triaged' https://daosio.atlassian.net/browse/DAOS-15801

github-actions[bot] avatar May 15 '24 08:05 github-actions[bot]

if i understand correctly this will just use the same parameters for psync but also add a libaio variation? For libaio we want to also vary the io_depth as we should be doing for the dfs engine (if we don't do that already).

At this time the iopdepth is fixed for fair comparison between the different ioengine. However, I have not issue to vary it value. If yes, @mchaarawi , could you help me to define how it should vary:

  • the range of values to use
  • for which engine it should change

We have also to take into account, that this test already takes some time.

knard38 avatar May 16 '24 13:05 knard38

if i understand correctly this will just use the same parameters for psync but also add a libaio variation? For libaio we want to also vary the io_depth as we should be doing for the dfs engine (if we don't do that already).

At this time the iopdepth is fixed for fair comparison between the different ioengine. However, I have not issue to vary it value. If yes, @mchaarawi , could you help me to define how it should vary:

  • the range of values to use
  • for which engine it should change

We have also to take into account, that this test already takes some time.

For DFS and libaio it would be good to try io_depth of 16, 32 if adding those increases the test a lot we should discuss (maybe during WG tomorrow) all the parameters again to see what combinations we can eliminate.

mchaarawi avatar May 16 '24 13:05 mchaarawi

if i understand correctly this will just use the same parameters for psync but also add a libaio variation? For libaio we want to also vary the io_depth as we should be doing for the dfs engine (if we don't do that already).

At this time the iopdepth is fixed for fair comparison between the different ioengine. However, I have not issue to vary it value. If yes, @mchaarawi , could you help me to define how it should vary:

  • the range of values to use
  • for which engine it should change

We have also to take into account, that this test already takes some time.

For DFS and libaio it would be good to try io_depth of 16, 32 if adding those increases the test a lot we should discuss (maybe during WG tomorrow) all the parameters again to see what combinations we can eliminate.

  • [x] Add test with iodepth=16

knard38 avatar May 21 '24 13:05 knard38

  • Add test with iodepth=16

Fixed with commit f0a70af00

knard38 avatar May 21 '24 13:05 knard38

I am looking at the result here: https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-14375/lastSuccessfulBuild/artifact/Functional%20Hardware%20Medium/dfuse/pil4dfs_fio.py/job.log and i don't see any results with iodepth != 1 I also see a lot of libaio engine runs with iodepth == 1 (which we don't need). am i missing something?

mchaarawi avatar May 21 '24 14:05 mchaarawi

I am looking at the result here: https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-14375/lastSuccessfulBuild/artifact/Functional%20Hardware%20Medium/dfuse/pil4dfs_fio.py/job.log and i don't see any results with iodepth != 1 I also see a lot of libaio engine runs with iodepth == 1 (which we don't need). am i missing something?

The CI have not yet run the the functional hardware test of the last PR push. I have locally successfully run this last PR push. I will keep you inform as soon as it will have been run by the CI.

Regarding the test with libaio and iodepth=1, this part has not changed.

This the different configuration which are tested: image

Please keep me inform if some configurations are useless or missing

knard38 avatar May 21 '24 17:05 knard38

Please keep me inform if some configurations are useless or missing

we can exclude all runs with engine=libaio and iodepth==1. they are just repetitive IMO. If we have the CI bw to do more runs, those should increase iodeoth to 16 rather than 1 (the ones comparing dfs and libaio). but we can do that in a later patch if we want to.

mchaarawi avatar May 21 '24 18:05 mchaarawi

have the CI bw to do more r

@mchaarawi , my apologies, but I do not understand your last sentence. Do you want to add some tests with iodepth == 16 ? If yes, with which configuration.

knard38 avatar May 22 '24 05:05 knard38

  • [x] Remove useless test with libaio

knard38 avatar May 22 '24 05:05 knard38

@mchaarawi , my apologies, but I do not understand your last sentence. Do you want to add some tests with iodepth == 16 ? If yes, with which configuration.

I just meant that in your table i see you added a lot of libaio tests with ts=256b,1m ; thread=1,0. IF you want you can change those test to do iodepth = 16 instead of 1 (for both libaio and dfs). but that is not in scope of this PR. the existing tests you have with iodepth==16 should cover the functional testing needed for now.

mchaarawi avatar May 22 '24 12:05 mchaarawi

  • [x] Remove useless test with libaio

Fixed with commit 62c2745e81e989f6d59324c0cb0dbd4f2bf4dff3

knard38 avatar May 22 '24 12:05 knard38

Latest version of the functional test have been validated by the CI. This the different configuration which are tested: image

knard38 avatar May 30 '24 14:05 knard38

Latest version of the functional test have been validated by the CI. This the different configuration which are tested: image

@mchaarawi , are the test configurations OK for you ?

knard38 avatar May 31 '24 07:05 knard38

Latest version of the functional test have been validated by the CI. This the different configuration which are tested: image

@mchaarawi , are the test configurations OK for you ?

yes. thanks!

mchaarawi avatar May 31 '24 12:05 mchaarawi

@daos-stack/daos-gatekeeper , please could you lend this PR with the following message: Title: DAOS-15801 test: Add aio ioengine to pil4dfs_fio.py functional test Message:

Add functional tests running FIO using the libaio ioengine and the PIL4DFS interception library.
Compare the bandwidth of these runs with one using the dfs ioengine.

knard38 avatar May 31 '24 12:05 knard38

please backport to 2.6. tia

mchaarawi avatar May 31 '24 12:05 mchaarawi

please backport to 2.6. tia

Asked for backport in the Jira ticket. I will do the backport as soon as I will have the approval. Thanks for your attention.

knard38 avatar May 31 '24 12:05 knard38