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: fix omap digest compare when scrub #9271

Merged
merged 1 commit into from Aug 4, 2016

Conversation

XinzeChi
Copy link
Contributor

@XinzeChi XinzeChi commented May 23, 2016

http://tracker.ceph.com/issues/16000

Introduce by fe1c28d.

The if logic would always return ture when deep-scrub non-omap object. And would trigger update the digest every deep-scrub.

Signed-off-by: Xinze Chi xinze@xsky.com

@liewegas
Copy link
Member

lgtm. Can you open a tracker ticket so we can mark this for backport? (And put the ticket url in the git commit message?) Thanks!

Introduce by fe1c28d.

Fixes: http://tracker.ceph.com/issues/16000
Signed-off-by: Xinze Chi <xinze@xsky.com>
@tchaikov
Copy link
Contributor

lgtm also.

@yuriw
Copy link
Contributor

yuriw commented May 24, 2016

@liewegas This tag should be against hammer, untagged

@tchaikov tchaikov added this to the hammer milestone Jun 8, 2016
@tchaikov
Copy link
Contributor

tchaikov commented Jun 8, 2016

@XinzeChi since this master is also suffering from this issue, could you target master instead? and we can backport it to jewel and hammer.

@tchaikov
Copy link
Contributor

tchaikov commented Jun 9, 2016

superseded by #9587

@tchaikov tchaikov closed this Jun 9, 2016
@dzafman
Copy link
Contributor

dzafman commented Jun 9, 2016

This only applies to hammer.

@dzafman dzafman reopened this Jun 9, 2016
@smithfarm smithfarm assigned smithfarm and unassigned XinzeChi Jun 27, 2016
smithfarm added a commit that referenced this pull request Jun 27, 2016
Reviewed-by: Nathan Cutler <ncutler@suse.com>
smithfarm added a commit that referenced this pull request Jul 18, 2016
Reviewed-by: Nathan Cutler <ncutler@suse.com>
smithfarm added a commit that referenced this pull request Jul 24, 2016
Reviewed-by: Nathan Cutler <ncutler@suse.com>
@smithfarm
Copy link
Contributor

@liewegas This PR is in the latest round of hammer-backports integration tests, which passed a rados run (the only failures are a valgrind false positive that has since been fixed by ceph/teuthology#915 and http://tracker.ceph.com/issues/15139 which is an infrastructure issue with two of the tests) - for details, see: http://tracker.ceph.com/issues/15895#note-18

Do you think this PR is OK to merge?

@smithfarm
Copy link
Contributor

@liewegas @athanatos @dzafman This PR passed a /200 rados run on Ubuntu. None of the failures were reproducible. For details see http://tracker.ceph.com/issues/15895#note-18

Do you think it's OK to merge?

@athanatos
Copy link
Contributor

I am ok with it if @dzafman is ok with it.

@smithfarm
Copy link
Contributor

@dzafman OK to merge?

@dzafman
Copy link
Contributor

dzafman commented Aug 4, 2016

@smithfarm Ok to merge

@smithfarm smithfarm merged commit da4f735 into ceph:hammer Aug 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants