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

jewel: osd: omap threadpool heartbeat is only reset every 100 values #16167

Merged
merged 1 commit into from Sep 5, 2017

Conversation

Projects
None yet
5 participants
@Vicente-Cheng
Contributor

Vicente-Cheng commented Jul 6, 2017

osd/ReplicatedBackend: reset thread heartbeat after every omap entry …
…in deep-scrub

Doing this every 100 entries could be after 100MB of reads. There's
little cost to reset this, so remove the option for configuring it.

This reduces the likelihood of crashing the osd due to too many omap
values on an object.

Fixes: http://tracker.ceph.com/issues/20375
Signed-off-by: Josh Durgin <jdurgin@redhat.com>
(cherry picked from commit 15ce608)

Conflicts:
	src/osd/ReplicatedBackend.cc
            - remain the iterator checker (do not check status)

@tchaikov tchaikov added this to the jewel milestone Jul 6, 2017

@Vicente-Cheng

This comment has been minimized.

Show comment
Hide comment
@Vicente-Cheng

Vicente-Cheng Jul 6, 2017

Contributor

retest this please

Contributor

Vicente-Cheng commented Jul 6, 2017

retest this please

handle.reset_tp_timeout();
}
++keys_scanned;
handle.reset_tp_timeout();

This comment has been minimized.

@liu-chunmei

liu-chunmei Jul 17, 2017

Contributor

what's the benefit we reset timeout every each iter?

@liu-chunmei

liu-chunmei Jul 17, 2017

Contributor

what's the benefit we reset timeout every each iter?

This comment has been minimized.

@Vicente-Cheng

Vicente-Cheng Aug 1, 2017

Contributor

hi @liu-chunmei , sorry for late reply
I think the origin pull request have a explain. #15823
In some cases, reset thread heartbeat every 100 entries would get more cost than every omap.

@Vicente-Cheng

Vicente-Cheng Aug 1, 2017

Contributor

hi @liu-chunmei , sorry for late reply
I think the origin pull request have a explain. #15823
In some cases, reset thread heartbeat every 100 entries would get more cost than every omap.

@smithfarm smithfarm requested review from jdurgin and tchaikov Sep 3, 2017

@smithfarm

This comment has been minimized.

Show comment
Hide comment
@smithfarm

smithfarm Sep 3, 2017

Contributor

@jdurgin @tchaikov This passed a rados run at http://tracker.ceph.com/issues/20613#note-40 with some failures, all of which I tried to reproduce. The only one that is reliably reproducible is http://tracker.ceph.com/issues/20981

Please review.

Contributor

smithfarm commented Sep 3, 2017

@jdurgin @tchaikov This passed a rados run at http://tracker.ceph.com/issues/20613#note-40 with some failures, all of which I tried to reproduce. The only one that is reliably reproducible is http://tracker.ceph.com/issues/20981

Please review.

@jdurgin

jdurgin approved these changes Sep 5, 2017

@jdurgin jdurgin merged commit baa5183 into ceph:jewel Sep 5, 2017

4 checks passed

Docs: build check OK - docs built
Details
Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment