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

rgw: set placement rule properly #15221

Merged
merged 1 commit into from May 26, 2017

Conversation

Projects
None yet
3 participants
@fangyuxiangGL
Contributor

fangyuxiangGL commented May 23, 2017

we should set placement rule in RGWObjManifest::obj_iterator::update_location()
when ofs < manifest->get_head_size().

Signed-off-by: fang yuxiang fang.yuxiang@eisoo.com

rgw: set placement rule properly
we should set placement rule in RGWObjManifest::obj_iterator::update_location()
when ofs < manifest->get_head_size().

Signed-off-by: fang yuxiang fang.yuxiang@eisoo.com

@liewegas liewegas added the rgw label May 23, 2017

@cbodley

This comment has been minimized.

Contributor

cbodley commented May 23, 2017

@fangyuxiangGL this looks correct, but could you please open a tracker ticket and describe what failures this causes and the steps to reproduce them? that would help a lot with review and testing

@fangyuxiangGL

This comment has been minimized.

Contributor

fangyuxiangGL commented May 24, 2017

@cbodley

ok, will do it

@fangyuxiangGL

This comment has been minimized.

Contributor

fangyuxiangGL commented May 24, 2017

Hi, @cbodley

I test the master and this doesn't cause any functional issue, but will bring redundant reading of the head object in some case.

We know that the head object will be prefetched in RGWRados::get_obj_state_impl(RGWRados::raw_obj_stat) from somewhere, we don't need to really read it when do RGWRados::iterate_obj.

suggest that we have a bucket using a placement rule with different data pool from default placement rule, let's see some codes in RGWRados::iterate_obj:

  while (ofs < next_stripe_ofs && ofs <= end) {
    read_obj = iter.get_location().get_raw_obj(this); // read_obj has pool from default rule
    uint64_t read_len = min(len, iter.get_stripe_size() - (ofs - stripe_ofs));
    read_ofs = iter.location_ofs() + (ofs - stripe_ofs);

    if (read_len > max_chunk_size) {
      read_len = max_chunk_size;
    }
    
    reading_from_head = (read_obj == head_obj); // will be false, because head_obj has pool from the specific placement rule
    r = iterate_obj_cb(bucket_info, obj, read_obj, ofs, 
      read_ofs, read_len, reading_from_head, astate, arg, stripe_size);
    if (r < 0) {
      return r;
    }

    len -= read_len;
    ofs += read_len;
  }

consider that ofs == 0 and the iter pointer to the head object.
so, code "read_obj = iter.get_location().get_raw_obj(this); " will return the raw_obj of the head object.
without set the head place rule, get_raw_obj will use the default place rule.

but head_obj has pool from the specific placement rule, so reading_from_head will be false.
then, in RGWRados::get_obj_iterate_cb, it will reread the head object for the is_head_obj is false.

I am writing some codes about seperating the head object and tail stripe storage, so it bring some issue in my codes. But for the master, it just bring some performance issue, not functional issue.

@cbodley

thanks for the detailed explanation 👍

@cbodley

This comment has been minimized.

@cbodley cbodley merged commit e3bf1df into ceph:master May 26, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details

@fangyuxiangGL fangyuxiangGL deleted the fangyuxiangGL:set-placement-rule branch Jan 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment