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

jewel: osd: adjust scrub boundary to object without SnapSet #11311

Merged
4 commits merged into from Oct 14, 2016

Conversation

dzafman
Copy link
Contributor

@dzafman dzafman commented Oct 4, 2016

backport preparation for http://tracker.ceph.com/issues/17470

athanatos and others added 4 commits October 3, 2016 21:43
Signed-off-by: Samuel Just <sjust@redhat.com>
(cherry picked from commit a2c27c9)
Previously, we needed to scrub all objects in clones in a single
hash value mainly to ensure that _scrub had access to all clones
of a single object at the same time.  Instead, just avoid letting
head or snapdir be a boundary (see the comment in the commit
for details).

Signed-off-by: Samuel Just <sjust@redhat.com>
(cherry picked from commit 27bdc8c)
Previously, ScrubMap::objects was always sorted bitwise (nibblewise
before the comparator change was made.  It didn't matter because we
always scrubbed whole hash values.  Now, we need the objects in the
objectstore ordering because we may be missing objects at the tail of
the scanned range and need them to show up at the tail of the
ScrubMap::objects mapping.  We don't need to do anything else to handle
the upgrade process since the actual objects *in* the map were
determined by the objectstore ordering.

Signed-off-by: Samuel Just <sjust@redhat.com>
(cherry picked from commit 6d410e9)
See comment in commit.

Signed-off-by: Samuel Just <sjust@redhat.com>
@dzafman
Copy link
Contributor Author

dzafman commented Oct 4, 2016

The commit 8833c64 is needed to backport the other code into Jewel because of interoperation with Hammer OSDs.

@dzafman
Copy link
Contributor Author

dzafman commented Oct 8, 2016

@dzafman
Copy link
Contributor Author

dzafman commented Oct 8, 2016

Second run passed with 5 failures accounted for:

http://pulpito.ceph.com/dzafman-2016-10-07_15:56:21-rados-wip-zafman-testing-jewel-distro-basic-smithi/
job 461453 - ERROR: osd init failed: (36) File name too long
job 461294 probably the same as 461453
job 461405 probably the same as 461453
job 461332: http://tracker.ceph.com/issues/16236
job 461503: http://tracker.ceph.com/issues/16516

@dzafman dzafman removed the needs-test label Oct 8, 2016
@dzafman
Copy link
Contributor Author

dzafman commented Oct 8, 2016

@liewegas Ready for review and merge after master.

@dzafman dzafman changed the title Wip scrub boundary jewel Jewel: Wip scrub boundary Oct 8, 2016
@ghost ghost self-assigned this Oct 10, 2016
@ghost ghost changed the title Jewel: Wip scrub boundary jewel: scrub boundary Oct 10, 2016
@ghost ghost changed the title jewel: scrub boundary DNM: jewel: scrub boundary Oct 10, 2016
@ghost
Copy link

ghost commented Oct 10, 2016

Is there an issue / pull request for this already ? If not it's fine, can be added later ;-)

@dzafman
Copy link
Contributor Author

dzafman commented Oct 10, 2016

@dachary All that exists now is http://tracker.ceph.com/issues/17470, a feature tracker marked for backport to Jewel and Hammer.

@liewegas liewegas changed the title DNM: jewel: scrub boundary jewel: osd: adjust scrub boundary to object without SnapSet Oct 10, 2016
@ghost
Copy link

ghost commented Oct 10, 2016

@dzafman thanks for the link :-)

@dzafman
Copy link
Contributor Author

dzafman commented Oct 11, 2016

@liewegas Sam wrote and I modified these changes. Although you could count me as the reviewer, maybe you should review this so it could make 10.2.4.

@liewegas
Copy link
Member

Reviewed-by:

ghost pushed a commit that referenced this pull request Oct 13, 2016
…t without SnapSet

Reviewed-by: Loic Dachary <ldachary@redhat.com>
@ghost
Copy link

ghost commented Oct 14, 2016

It passed the rados (http://tracker.ceph.com/issues/17487#note-12), upgrade/jewel-x and upgrade/hammer-x (http://tracker.ceph.com/issues/17487#note-15) suites (except for two bugs that are, I believe, unrelated)

@ghost ghost merged commit 92a3538 into ceph:jewel Oct 14, 2016
@dzafman dzafman deleted the wip-scrub-boundary-jewel branch November 1, 2016 20:11
@@ -4184,13 +4192,15 @@ void PG::chunky_scrub(ThreadPool::TPHandle &handle)
p != pg_log.get_log().log.rend();
++p) {
if (cmp(p->soid, scrubber.start, get_sort_bitwise()) >= 0 &&
cmp(p->soid, scrubber.end, get_sort_bitwise()) < 0) {
cmp(p->soid, scrubber.end, get_sort_bitwise()) <= 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@dzafman @dachary In origial PR #11255, we do not modify the compare logic(origin is p->soid < scrubber.end ). Could you tell me why we modify this line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The original PR didn't have the change because the later release would never have to interact with Hammer OSDs. Hammer OSDs had a small bug for EC pools http://tracker.ceph.com/issues/17491 that would include the end element. See comment in PG.h.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants