rgw: can't download object with range when compression enabled

[fangyuxiangGL]

Fixes: http://tracker.ceph.com/issues/22852

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

[fangyuxiangGL]

@aclamk @cbodley please have a look, thanks.

out_bl may be empty if break from:

    if (ofs_in_bl + (off_t)first_block->len > bl_len) {
      // not complete block, put it to waiting
      unsigned tail = bl_len - ofs_in_bl;
      in_bl.copy(ofs_in_bl, tail, waiting);
      cur_ofs -= tail;
      break;
    }

And q_len may be 0 if bytes were all sent in:

    while (out_bl.length() - q_ofs >= cct->_conf->rgw_max_chunk_size)
    {
      off_t ch_len = std::min<off_t>(cct->_conf->rgw_max_chunk_size, q_len);
      q_len -= ch_len;
      r = next->handle_data(out_bl, q_ofs, ch_len);
      if (r < 0) {
        lderr(cct) << "handle_data failed with exit code " << r << dendl;
        return r;
      }
      out_bl.splice(0, q_ofs + ch_len);
      q_ofs = 0;
    }

[aclamk]

@fangyuxiangGL As I understand, the problem is when we try to next->handle_data on 0 sized data. Maybe check ch_len>0 will be more natural?

[fangyuxiangGL]

@aclamk
yeah, root cause is that no bytes need to be handled, so ch_len > 0 may be more comprehensive and concise. But I just think out_bl.length() && q_len is more intuitive, even more relevant to codes above.

[fangyuxiangGL]

@aclamk use ch_len > 0 to judge now..
@cbodley could you also add a 'bug fix' label?
thanks

[cbodley]

thanks @fangyuxiangGL, looks good. could you please look into s3test coverage for this? i do see a lot of Range headers in our tests, so i'm surprised we haven't hit this yet (the teuthology suite runs with compression=random)

[cbodley]

@fangyuxiangGL that sounds great. if you can add range requests that cover both the default 4MB and your 512k case, all the better 👍

[fangyuxiangGL]

@cbodley
I took a look at the s3test case about download range bytes. Current use cases only download a small range from the beginning or to the end, such as [0, 40] or [8MB-40, 8MB], which doen't reach bellow codes:

    if (ofs_in_bl + (off_t)first_block->len > bl_len) {
      // not complete block, put it to waiting
      unsigned tail = bl_len - ofs_in_bl;
      in_bl.copy(ofs_in_bl, tail, waiting);
      cur_ofs -= tail;
      break;
    }

I think we should add such a case:

1. upload an object with size 8MB
2. download bytes of range [2MB, 7MB]

But, I am also worried that even such a case might not expose the issue, because head object size is 512KB in our private codes and is different from the master(has a 4MB size head object), but I think we also have great probability to expose the issue.

[fangyuxiangGL]

@cbodley
generally speaking, the issue may happen if a data block needed by decompression span two rados objects! But now we can not have 512KB size head object, which is confined to be 4MB by rgw_max_chunk_size in ceph master. I will add proper test case later.

[cbodley]

test case at ceph/s3-tests#209 can merge once this does

[yuriw]

wip-yuri-testing-2018-02-14-2132

[aclamk]

Look good.