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

librbd: do write_full for whole object write #5750

Merged
merged 1 commit into from Sep 21, 2015

Conversation

wonzhq
Copy link
Contributor

@wonzhq wonzhq commented Sep 2, 2015

This is useful when ec is the base tier in the cache tiering setup. The
write_full can be proxied from the cache tier to the base tier. So that
we don't need to promote the object.

Signed-off-by: Zhiqiang Wang zhiqiang.wang@intel.com
Suggested-by: Nick Fisknick@fisk.me.uk

This is useful when ec is the base tier in the cache tiering setup. The
write_full can be proxied from the cache tier to the base tier. So that
we don't need to promote the object.

Signed-off-by: Zhiqiang Wang <zhiqiang.wang@intel.com>
Suggested-by: Nick Fisk<nick@fisk.me.uk>
@wonzhq
Copy link
Contributor Author

wonzhq commented Sep 2, 2015

@fiskn here comes the implementation of what we discussed in the ceph-user mail list yesterday :-)

@fiskn
Copy link
Contributor

fiskn commented Sep 2, 2015

Wow, that's quick progress. Couple of questions if its ok?

  1. m_object_off==0 in the IF statement, this checks that the write is aligned to the start of the object?
  2. m_object_len == m_ictx->get_object_size(). This checks that the write IO is equal to the object size. I assume this means if the IO is bigger than the object, then it will do a normal write instead of a write_full? I wonder if its possible to split an IO into several write_full requests to handle the case where an IO > Obj_size.

@ghost ghost added the rbd label Sep 2, 2015
@ghost
Copy link

ghost commented Sep 2, 2015

@wonzhq could you run part of the rbd suite to show it passes ? You would have to chose at least one job that walks that code path and link the pulpito result as a comment to this pull request.

@ghost ghost added the performance label Sep 2, 2015
@ghost
Copy link

ghost commented Sep 2, 2015

@wonzhq you will need to wait for http://tracker.ceph.com/issues/12903 to be fixed before running a successfull test.

@wonzhq
Copy link
Contributor Author

wonzhq commented Sep 2, 2015

@fiskn

m_object_off==0 in the IF statement, this checks that the write is aligned to the start of the object?

Yes.

m_object_len == m_ictx->get_object_size(). This checks that the write IO is equal to the object size. I assume this means if the IO is bigger than the object, then it will do a normal write instead of a write_full? I wonder if its possible to split an IO into several write_full requests to handle the case where an IO > Obj_size.

Actually the IO has been split when reaching here. This is done by file_to_extents in aio_write.

@wonzhq
Copy link
Contributor Author

wonzhq commented Sep 2, 2015

@dachary i proposed #5776 to fix 12903.

@ghost
Copy link

ghost commented Sep 2, 2015

@wonzhq wonzhq-rbd-write-full is building in http://ceph.com/gitbuilder.cgi

@fiskn
Copy link
Contributor

fiskn commented Sep 2, 2015

@wonzhq Thanks for the response, all makes sense, really looking forward to giving this a test.

@idryomov
Copy link
Contributor

idryomov commented Sep 2, 2015

I'll do the same for the kernel client once this is merged.

@dillaman
Copy link

dillaman commented Sep 3, 2015

LGTM once tested

@fiskn
Copy link
Contributor

fiskn commented Sep 3, 2015

Should this also be considered for the CephFS clients?

@wonzhq
Copy link
Contributor Author

wonzhq commented Sep 4, 2015

Yes, we should be able to do the same thing for cephfs and rgw. Please let me get familiar with the cephfs and rgw code first.

@jdurgin
Copy link
Member

jdurgin commented Sep 4, 2015

lgtm too. rgw already uses write_full when possible I think

@tchaikov
Copy link
Contributor

@jdurgin shall we run some rbd tests for it also?

@jdurgin
Copy link
Member

jdurgin commented Sep 14, 2015

@tchaikov yes, a rados run doesn't execute this code. I'll schedule an rbd run with this and maybe a few other branches

jdurgin added a commit that referenced this pull request Sep 21, 2015
librbd: do write_full for whole object write

Reviewed-by: Jason Dillaman <dillaman@redhat.com>
Reviewed-by: Josh Durgin <jdurgin@redhat.com>
@jdurgin jdurgin merged commit 19a480b into ceph:master Sep 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants