osd/ReplicatedPG: be more careful about calling publish_stats_to_osd() #8039
Conversation
|
@athanatos I'll test this through the FS suite but can you check if my reasoning makes any sense? |
| @@ -8303,6 +8301,7 @@ void ReplicatedPG::eval_repop(RepGather *repop) | |||
| repop->on_committed.erase(p++)) { | |||
| (*p)(); | |||
| } | |||
| publish_stats_to_osd(); | |||
There was a problem hiding this comment.
I think this should go a bit further down, here https://github.com/ceph/ceph/pull/8039/files#diff-72747d40a424e7b5404366b557ff12a3R8357 next to calc_min_last_complete_ondisk(). We could also do it as an on_complete lambda, but I think it makes sense to do this unconditionally.
There was a problem hiding this comment.
Uh, maybe? This is where it was located previously, and immediately follows where it would be called as part of an on_complete lambda, so I put it back as that's the minimal change and I'm unsure if there are any side effects to be worried about. (Though I definitely don't see anything that makes me think publish_stats_to_osd() is sensitive to much of anything.)
There was a problem hiding this comment.
Yeah, next to calc_min_last_complete_ondisk would be better.
…rrectly We had moved the call out of eval_repop into a lambda, but that left out a few other code paths and is fragile. So just call it unconditionally in eval_repop() when we're done with the repop instead. Fixes: ceph#14962 Signed-off-by: Greg Farnum <gfarnum@redhat.com>
gregsfortytwo
force-pushed
the
wip-14962-greg
branch
2 times, most recently
from
f62a73d
to
d9af48a
Compare
…into greg-fs-testing #8039
|
This passed my cephfs testing: http://pulpito.ceph.com/gregf-2016-03-15_04:07:43-fs-greg-fs-testing-314---basic-mira/ |
|
@athanatos @liewegas can we merge this now? :) |
gregsfortytwo
changed the title
liewegas
changed the title
osd/ReplicatedPG: be more careful about calling publish_stats_to_osd() Reviewed-by: Samuel Just <sjust@redhat.com> Reviewed-by: Sage Weil <sage@redhat.com>
We had moved the call out of eval_repop into a lambda, but that left out
a few other code paths and is fragile. So just call it unconditionally in
eval_repop() instead.
Fixes: #14962
Signed-off-by: Greg Farnum gfarnum@redhat.com