-
Notifications
You must be signed in to change notification settings - Fork 6k
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.
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
doc/rados/operations: OSD_OUT_OF_ORDER_FULL fullness order is wrong #31588
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@usefulalgorithm thanks for the fix. but the title of the commit message reads like a bug report. ideally, the title of the commit message should use the imperative mood. see also https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst#3-describe-your-changes . please use git commit --amend
and git push -f
for revising the commit message and its title.
also, could you prefix the title of your commit message with the subcomponent your are changing ? see https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst#3-describe-your-changes. in this case, it would be "doc/rados/operations ", as you put in the title of this pull request.
and, could you add a "Signed-off-by
" line at the end of your commit message? "git commit -s" will do the trick for you. see https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst#1-sign-your-work
and/or `failsafe_full` are not ascending. In particular, we expect | ||
`backfillfull < nearfull`, `nearfull < full`, and `full < | ||
`nearfull < backfillfull`, `backfillfull < full`, and `full < |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, i am wondering if we could revise the way to explain the order of the full ratios. can we just put something like
In particular, we expect
nearfull
<backfillfull
<full
<failsafe_full
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be clearer if we do it the way you suggested. Should I fix it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i am fine either way. my only concern is that line-wrap could render the text a little bit difficult to read. but it's a compromise, i guess.
but anyway, it's already an improvement with your change!
Signed-off-by: Tsung-Ju Lii <usefulalgorithm@gmail.com>
2099d3d
to
253cb99
Compare
@tchaikov it's done. Thanks for the tips! |
jenkins render docs |
Doc render available at http://docs.ceph.com/ceph-prs/31588/ |
A pool that's
backfillfull
should be fuller thannearfull
.Signed-off-by: Tsung-Ju Lii usefulalgorithm@gmail.com
Checklist
Show available Jenkins commands
jenkins retest this please
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test dashboard backend
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox