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

Revert "osdc: After write try merge bh." #11262

Merged
merged 1 commit into from Oct 17, 2016
Merged

Conversation

jcsp
Copy link
Contributor

@jcsp jcsp commented Sep 29, 2016

This reverts commit 1a48a8a.

Fixes: http://tracker.ceph.com/issues/17270
Signed-off-by: John Spray john.spray@redhat.com

This reverts commit 1a48a8a.

Fixes: http://tracker.ceph.com/issues/17270
Signed-off-by: John Spray <john.spray@redhat.com>
@jcsp jcsp added cephfs Ceph File System rbd bug-fix labels Sep 29, 2016
@dillaman
Copy link

lgtm

@gregsfortytwo
Copy link
Member

Can you please explain in the commit and/or ticket why it's bad? At first blush we certainly do want to merge bufferheads whenever possible...

@dillaman
Copy link

dillaman commented Oct 5, 2016

@gregsfortytwo This loop [1] operates on bhs that might be merged together. e.g. if the first and second bhs can be merged, they will -- then the loop will advance to the now merged/destructed second bh and try to merge it.

[1] https://github.com/ceph/ceph/pull/11262/files#diff-9bcd2f7647a2bd574b6ebe6baf8e61b3L1166

@gregsfortytwo
Copy link
Member

Right, makes sense. We still want to do the merging though so we probably need a better data structure/algorithm instead of just tossing it out.
(I'm not 100% sure we're allowed to have non-merged adjacent buffers...)

@gregsfortytwo
Copy link
Member

Testing http://pulpito.ceph.com/gregf-2016-10-06_18:07:58-fs-greg-fs-testing-105---basic-mira/ looks pretty good; OSD failure (http://tracker.ceph.com/issues/17553), pjd failure due to missing qa-suite wip-getuid merge, 2xlibcephfs-java failures (http://tracker.ceph.com/issues/17548), 1 dead job.

I'd still prefer we do a better job merging though.

@jcsp jcsp merged commit 8279264 into ceph:master Oct 17, 2016
@jcsp jcsp deleted the wip-17270-master branch October 17, 2016 12:06
@jcsp
Copy link
Contributor Author

jcsp commented Oct 17, 2016

@gregsfortytwo you're right that we should add the behaviour back, but for the moment this needs to merge because the current code crashes. I've created http://tracker.ceph.com/issues/17589 for the job of adding it back

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-fix cephfs Ceph File System rbd
Projects
None yet
3 participants