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.

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

osd: make scrub right now when pg stats_invalid is true #17884

Merged
merged 1 commit into from Sep 28, 2017

Conversation

kungf
Copy link

@kungf kungf commented Sep 21, 2017

reg_stamp was set to ceph_clock_now() when scrubber.must_scrub is true or (info.stats.stats_invalid && cct->_conf->osd_scrub_invalid_stats) is true, but 'must' argument of osd::reg_pg_scrub() was set according to scrubber.must_scrub, so when scrubber.must_scrub is false and (info.stats.stats_invalid && cct->_conf->osd_scrub_invalid_stats) is true, even the scrub was delayed because ceph_clock_now() larger than info.history.last_scrub_stamp.
@yangdongsheng

Signed-off-by: kungf <yang.wang@easystack.cn>
@tchaikov tchaikov self-requested a review September 21, 2017 15:04
@tchaikov tchaikov added the core label Sep 21, 2017
if (scrubber.must_scrub ||
(info.stats.stats_invalid && cct->_conf->osd_scrub_invalid_stats)) {
reg_stamp = ceph_clock_now();
must = true;

Choose a reason for hiding this comment

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

if (info.stats.stats_invalid && cct->_conf->osd_scrub_invalid_stats) == 1 and scrubber.must_scrub == false still must would become true. Though scrubber.must_scrub is false.
So Looks not correct...

Copy link
Author

Choose a reason for hiding this comment

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

@amitkumar50 thanks for your review. if (info.stats.stats_invalid && cct->_conf->osd_scrub_invalid_stats) == true, then must become ture, then scrub deadline was set to now in PG::reg_next_scrub(), and the pg will do scrub even though scrubber.must_scrub=false, doesn't this logic anything wrong ?

@kungf
Copy link
Author

kungf commented Sep 22, 2017

@liewegas need a review please.

@tchaikov
Copy link
Contributor

@kungf why do you think we should scrub immediately if "stats_invalid"?

Copy link
Contributor

@tchaikov tchaikov left a comment

Choose a reason for hiding this comment

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

the commit message fails to explain the reason why we need to change current behavior.

@yuriw yuriw merged commit fc7ad29 into ceph:master Sep 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants