Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

common: bench test fall into dead loop when <seconds>=0 #16382

Merged
merged 1 commit into from Aug 10, 2017
Merged

common: bench test fall into dead loop when <seconds>=0 #16382

merged 1 commit into from Aug 10, 2017
+3 −3

Conversation

PCzhangPC
Copy link

@PCzhangPC PCzhangPC commented Jul 18, 2017 •
edited

when using the command : rados bench -p <pool_name> <write|seq|rand> -b -t –no-cleanup , if the user set the "seconds" as 0, the program will fall into a dead loop by continuously adding new write or new read.

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

joscollin
joscollin requested changes Jul 18, 2017
View reviewed changes
Copy link
Member

@joscollin joscollin left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 bug2 -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/

Also squash your commits into one, if it is not difficult to review at once.
Please see: https://github.com/ginatrapani/todo.txt-android/wiki/Squash-All-Commits-Related-to-a-Single-Issue-into-a-Single-Commit

@PCzhangPC PCzhangPC force-pushed the bug2 branch 3 times, most recently from 163e898 to e3dafde Compare July 18, 2017 07:09
@PCzhangPC PCzhangPC changed the title rados bench bug when <seconds>=0 rados： bench test bug when <seconds>=0 Jul 18, 2017
@PCzhangPC
Copy link
Author

now i have added the Signed-off-by already.

@joscollin
Copy link
Member

Jenkins Retest this please

@joscollin
Copy link
Member

joscollin commented Jul 19, 2017 •
edited

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

Please see this as an example: https://github.com/ceph/ceph/pull/15648/commits.
Please note that I specified "Fixes:" in the commit message, as there is an external reference in the Coverity Scan. If you don't have a reference, then give a brief description by yourself.

@PCzhangPC
Copy link
Author

@yangdongsheng please help to review this pr ,thx

@PCzhangPC PCzhangPC changed the title rados： bench test bug when <seconds>=0 common： bench test fall into dead loop when <seconds>=0 Aug 7, 2017
common：bench test bug when <seconds>=0
7258c99 
Fixes:
src/common/obj_bencher.cc b/src/common/obj_bencher.cc :write_bench(),seq_read_bench(),rand_read_bench()
the program may fall into a dead loop if the <seconds> set as 0

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

please help to review this,thx @joscollin

joscollin
joscollin approved these changes Aug 8, 2017
View reviewed changes
Copy link
Member

@joscollin joscollin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I believe secondsToRun is the value that can be 0.

I hope you have tested the fix on your side.

@PCzhangPC
Copy link
Author

yes, i have tested it on my side.

@liewegas liewegas changed the title common： bench test fall into dead loop when <seconds>=0 common: bench test fall into dead loop when <seconds>=0 Aug 8, 2017
@joscollin
Copy link
Member

Success:

@joscollin joscollin merged commit 5fbb81f into ceph:master Aug 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joscollin joscollin joscollin approved these changes

Assignees
No one assigned
Labels
bug-fix common needs-qa wip-jcollin-testing
Projects
None yet
Milestone
No milestone
2 participants
@PCzhangPC @joscollin