common: bench test get incorrect iops result #16381

PCzhangPC · 2017-07-18T06:08:34Z

when running : rados -p test bench 20 seq(rand), if the running time of this command is less than 1s,the max/min_iops will not be updated after initialized to 0 and MAX_INT, so the result would be incorrect as below:
Average IOPS: 180
Stddev IOPS: 0
Max IOPS: 0
Min IOPS: 2147483647

Signed-off-by: PCzhangPC pengcheng.zhang@easystack.cn

joscollin

In Ceph we conventionally put the Signed-off-by at the end of a commit message. The title of the Commit should be prefixed with the submodule name followed by a short description of the fix. A detailed description should be added as the body of the Commit message. Please see one of my merged PR as an example: https://github.com/ceph/ceph/pull/15648/commits

See this too: https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst#1-sign-your-work

You can do:

    $git commit --amend.
    Modify your commit and add the `Signed-off-by` manually for this time.
    Then push using -f option again: $git push origin test -f

From the next time onwards remember to use -s when you commit your changes, so that the Signed-off-by will be added automatically to your commit, if you configure your git settings properly.
Please see: https://help.github.com/articles/setting-your-username-in-git/
https://help.github.com/articles/setting-your-email-in-git/

PCzhangPC · 2017-07-18T15:02:09Z

@joscollin i have added the Signed-off-by.but, can you tell me how to retest this? thx...

joscollin · 2017-07-19T01:55:38Z

Jenkins retest this please

joscollin · 2017-07-19T01:58:08Z

@PCzhangPC Please correct the commit title and the body appropriately.

Please see this as an example: https://github.com/ceph/ceph/pull/15648/commits.
If you don't have an external reference (Fixes:), then give a brief description by yourself in the body.

PCzhangPC · 2017-08-07T07:58:17Z

@joscollin @yangdongsheng please help to review this pr ,thx

joscollin

if the running time of this command is too short

I was wondering how did you came up with runtime values <=2 for too short situation.

joscollin · 2017-08-07T09:14:29Z

src/common/obj_bencher.cc

+      if(data.idata.max_iops == 0 && data.idata.min_iops == INT_MAX)
+          data.idata.max_iops = data.idata.min_iops = (int)(data.finished/runtime);
+  }
+  if(runtime > 1 && runtime <= 2) {


This always evaluates to runtime == 2 right ?

joscollin · 2017-08-07T09:14:58Z

src/common/obj_bencher.cc

+      if(data.idata.max_iops == 0 && data.idata.min_iops == INT_MAX)
+          data.idata.max_iops = data.idata.min_iops = (int)(data.finished/runtime);
+  }
+  if(runtime > 1 && runtime <= 2) {


PCzhangPC · 2017-08-08T05:17:31Z

There is something wrong in my last commit, and i remove the (1<runtime<2) part.
The max_iops is initialized as 0 and min_iops is initialized as MAX_INT. If the command finishes less than 1s, the function 'void *ObjBencher::status_printer' will not update the value of max/min_iops because the variable 'cyclesinceChange' is set to 0 and will not change within a second. so the result will be 0 and MAX_INT.
as for (1<runtime<2) part, for example,if the runtime is 1.2s, the max_iops and min_iops is calculated in the first second.but in the last 0.2 second, htere are orther writes/reads to finish but we dont't update the max/min_iops again. so the average_iops is lagger than max_iops. in last commit, i added an condition to handle this situation, which was not appropriate. i remove this part, so this pr just to solve the problem when runtime<1.
@joscollin @yangdongsheng

yangdongsheng · 2017-08-08T08:30:12Z

okey @PCzhangPC So the problem is MIN_IOPS and MAX_IOPS will not be updated when the runtime <= 1, right? okey sounds fair enough now. @joscollin what do you think about it?

joscollin · 2017-08-13T12:27:08Z

src/common/obj_bencher.cc

@@ -799,6 +799,11 @@ int ObjBencher::seq_read_bench(int seconds_to_run, int num_objects, int concurre
  }

  runtime = ceph_clock_now() - data.start_time;
+  if(runtime <= 1) {


so this pr just to solve the problem when runtime<1.

Are you considering runtime == 1 too ?

oh sorry. there will be no need to update the iops again if runtime==1 because it has get correct result already. and commit has been updated.

if the running time of this command is less than 1s

@PCzhangPC I was wondering whether this 1s will change to 2s in another execution in another machine with a different performance. Have you tried anything like that? Is this 1s depends on your execution environment?

@joscollin it's the same in different machine. as you can see the logic in here1 , the IOPS will only be calculated when the cycleSinceChange is not 0. and the cycleSinceChange will increase once a second. but if the runtime is less than 1s, this variable has no chance to change from 0 to 1. and so, the max/min_iops will show as 0/MAX_INT.
[1]https://github.com/ceph/ceph/blob/master/src/common/obj_bencher.cc#L127

Aah, sorry. My bad, I didn't check that part :-)

thx for your review :-)

Fixes: bench write/read test command may get wrong result when the test time is less than 1s. Signed-off-by: PCzhangPC <pengcheng.zhang@easystack.cn>

joscollin · 2017-08-18T04:19:40Z

Test Result:

joscollin · 2017-08-18T04:25:31Z

@liewegas @jdurgin
Could you please merge ? Thanks.

PCzhangPC · 2017-08-23T01:34:30Z

@ceph-jenkins Jenkins retest this please

tchaikov · 2017-08-23T07:32:41Z

@PCzhangPC i fixed this issue in a slightly simpler way at #17182, could you take a look?

joscollin requested changes Jul 18, 2017

View reviewed changes

joscollin added the bug-fix label Jul 18, 2017

PCzhangPC force-pushed the test branch from ad1c21b to cd58ab4 Compare July 18, 2017 07:15

PCzhangPC changed the title ~~fix some bench problems~~ rados: bench test iops incorrct Jul 18, 2017

PCzhangPC changed the title ~~rados: bench test iops incorrct~~ rados: bench test iops result incorrct Jul 18, 2017

PCzhangPC force-pushed the test branch 5 times, most recently from 8c0e30e to 7d9e95d Compare July 19, 2017 08:55

PCzhangPC force-pushed the test branch 2 times, most recently from c4d5f54 to ed348fe Compare August 7, 2017 07:50

PCzhangPC changed the title ~~rados: bench test iops result incorrct~~ rados: bench test get incorrct iops result Aug 7, 2017

joscollin added the common label Aug 7, 2017

joscollin changed the title ~~rados: bench test get incorrct iops result~~ common: bench test get incorrect iops result Aug 7, 2017

joscollin added the core label Aug 7, 2017

joscollin requested changes Aug 7, 2017

View reviewed changes

PCzhangPC force-pushed the test branch 2 times, most recently from ac6c37f to 1ef506e Compare August 8, 2017 04:24

PCzhangPC closed this Aug 8, 2017

PCzhangPC reopened this Aug 8, 2017

PCzhangPC force-pushed the test branch from 1ef506e to 1a90afb Compare August 8, 2017 06:14

joscollin requested changes Aug 13, 2017

View reviewed changes

common: bench test iops results incorrct

1cc9147

Fixes: bench write/read test command may get wrong result when the test time is less than 1s. Signed-off-by: PCzhangPC <pengcheng.zhang@easystack.cn>

PCzhangPC force-pushed the test branch from 1a90afb to 1cc9147 Compare August 15, 2017 07:33

joscollin approved these changes Aug 15, 2017

View reviewed changes

joscollin added needs-qa wip-jcollin-testing labels Aug 15, 2017

PCzhangPC closed this Aug 23, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

common: bench test get incorrect iops result #16381

common: bench test get incorrect iops result #16381

PCzhangPC commented Jul 18, 2017 •

edited

joscollin left a comment

PCzhangPC commented Jul 18, 2017 •

edited

joscollin commented Jul 19, 2017

joscollin commented Jul 19, 2017

PCzhangPC commented Aug 7, 2017

joscollin left a comment •

edited

joscollin Aug 7, 2017

joscollin Aug 7, 2017

PCzhangPC commented Aug 8, 2017 •

edited

yangdongsheng commented Aug 8, 2017

joscollin Aug 13, 2017

PCzhangPC Aug 15, 2017

joscollin Aug 15, 2017

PCzhangPC Aug 15, 2017 •

edited

joscollin Aug 15, 2017

PCzhangPC Aug 16, 2017 •

edited

joscollin commented Aug 18, 2017

joscollin commented Aug 18, 2017

PCzhangPC commented Aug 23, 2017

tchaikov commented Aug 23, 2017

common: bench test get incorrect iops result #16381

common: bench test get incorrect iops result #16381

Conversation

PCzhangPC commented Jul 18, 2017 • edited

joscollin left a comment

Choose a reason for hiding this comment

PCzhangPC commented Jul 18, 2017 • edited

joscollin commented Jul 19, 2017

joscollin commented Jul 19, 2017

PCzhangPC commented Aug 7, 2017

joscollin left a comment • edited

Choose a reason for hiding this comment

joscollin Aug 7, 2017

Choose a reason for hiding this comment

joscollin Aug 7, 2017

Choose a reason for hiding this comment

PCzhangPC commented Aug 8, 2017 • edited

yangdongsheng commented Aug 8, 2017

joscollin Aug 13, 2017

Choose a reason for hiding this comment

PCzhangPC Aug 15, 2017

Choose a reason for hiding this comment

joscollin Aug 15, 2017

Choose a reason for hiding this comment

PCzhangPC Aug 15, 2017 • edited

Choose a reason for hiding this comment

joscollin Aug 15, 2017

Choose a reason for hiding this comment

PCzhangPC Aug 16, 2017 • edited

Choose a reason for hiding this comment

joscollin commented Aug 18, 2017

joscollin commented Aug 18, 2017

PCzhangPC commented Aug 23, 2017

tchaikov commented Aug 23, 2017

PCzhangPC commented Jul 18, 2017 •

edited

PCzhangPC commented Jul 18, 2017 •

edited

joscollin left a comment •

edited

PCzhangPC commented Aug 8, 2017 •

edited

PCzhangPC Aug 15, 2017 •

edited

PCzhangPC Aug 16, 2017 •

edited