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: get wrong content when download object with specific range with compression #15323

Merged
merged 1 commit into from Jun 12, 2017

Conversation

Projects
None yet
5 participants
@fangyuxiangGL
Contributor

fangyuxiangGL commented May 27, 2017

get wrong content when download object with specific range with compression
http://tracker.ceph.com/issues/20100

look at the prototype:
RGWGetObj_Decompress::handle_data(bufferlist& bl, off_t bl_ofs, off_t bl_len)
we should trim the bl using bl_ofs and bl_len.

consider that we just need small chunk of the head object stripe to decompress the data range needed by client, but in RGWRados::get_obj_iterate_cb:

if (is_head_obj) {
/* only when reading from the head object do we need to do the atomic test */
r = append_atomic_test(rctx, bucket_info, obj, op, &astate);
if (r < 0)
return r;

if (astate &&
    obj_ofs < astate->data.length()) {
  unsigned chunk_len = min((uint64_t)astate->data.length() - obj_ofs, (uint64_t)len);

  d->data_lock.Lock();
  r = d->client_cb->handle_data(astate->data, obj_ofs, chunk_len);// pass the whole head object data to decompress filter
  d->data_lock.Unlock();
  if (r < 0)
    return r;

  d->lock.Lock();
  d->total_read += chunk_len;
  d->lock.Unlock();

  len -= chunk_len;
  read_ofs += chunk_len;
  obj_ofs += chunk_len;
  if (!len)
    return 0;
}

}

code :
r = d->client_cb->handle_data(astate->data, obj_ofs, chunk_len);
pass the whole head object data to decompress filter

@fangyuxiangGL fangyuxiangGL changed the title from rgw: several bug when compression is enabled to rgw: several bugs when compression is enabled May 27, 2017

@fangyuxiangGL fangyuxiangGL changed the title from rgw: several bugs when compression is enabled to rgw: get wrong content when download object with specific range when compression was enabled May 27, 2017

rgw: get wrong content when download object with specific range when
compression was enabled

look at the prototype:
RGWGetObj_Decompress::handle_data(bufferlist& bl, off_t bl_ofs, off_t bl_len)
we should trim the bl using bl_ofs and bl_len.

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

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

@fangyuxiangGL fangyuxiangGL changed the title from rgw: get wrong content when download object with specific range when compression was enabled to rgw: get wrong content when download object with specific range with compression May 27, 2017

@mattbenjamin

This comment has been minimized.

Contributor

mattbenjamin commented May 28, 2017

@fangyuxiangGL agree have hit this :)

@rzarzynski

This comment has been minimized.

Contributor

rzarzynski commented May 30, 2017

CC: @aclamk.

@aclamk

This comment has been minimized.

Contributor

aclamk commented May 31, 2017

The change looks OK.
I have checked encryption for similar problems in fixup_range and it is ok.

@rzarzynski rzarzynski self-assigned this May 31, 2017

@fangyuxiangGL

This comment has been minimized.

Contributor

fangyuxiangGL commented Jun 7, 2017

@rzarzynski

This comment has been minimized.

Contributor

rzarzynski commented Jun 7, 2017

@rzarzynski

This comment has been minimized.

Contributor

rzarzynski commented Jun 7, 2017

It looks we've got (hopefully unrelated) rgw multisite test failures. Verifying on the corresponding master.

@fangyuxiangGL

This comment has been minimized.

Contributor

fangyuxiangGL commented Jun 8, 2017

@rzarzynski

a bit sad for the failure...
And how is the later test?

@rzarzynski

This comment has been minimized.

Contributor

rzarzynski commented Jun 8, 2017

@fangyuxiangGL: no worries for now. I guess I already saw the test is failing on other branches as well. Scheduling on master just to be sure.

@rzarzynski

This comment has been minimized.

Contributor

rzarzynski commented Jun 8, 2017

Huh, it looks that Teuthology isn't able to deal with branches having the ref suffix in their names (like wip-rzarzynski-testing-ref):

2017-06-08T10:27:12.979 INFO:teuthology.orchestra.run.mira078:Running: "if test -f /etc/yum.repos.d/ceph.repo ; then sudo sed -i -e ':a;N;$!ba;s/enabled=1\\ngpg/enabled=1\\npriority=1\\ngpg/g' -e 's;ref/[a-zA-Z0-9_-]*/;sha1/b22355205d1ed8fa1e08be8e284ed594fd8871e0/;g' /etc/yum.repos.d/ceph.repo ; fi"

Rescheduling.

@rzarzynski

This comment has been minimized.

Contributor

rzarzynski commented Jun 12, 2017

The branch has been tested in following Teuthology runs:

Reference point:

The rgw multisite test failures don't seem to be related. The assertion on weak refs in OSD is well-known and appears in other, unrelated branches as well. In the log we can find:

2017-06-08T19:00:51.548 INFO:tasks.ceph.c1.osd.2.mira041.stderr:/home/jenkins-build/build/workspace/ceph-dev-new-build/ARCH/x86_64/AVAILABLE_ARCH/x86_64/AVAILABLE_DIST/centos7/DIST/centos7/MACHINE_SIZE/huge/release/12.0.2-2416-g3beddb9/rpm/el7/BUILD/ceph-12.0.2-2416-g3beddb9/src/common/shared_cache.hpp: 105: FAILED assert(weak_refs.empty())
2017-06-08T19:00:51.548 INFO:tasks.ceph.c1.osd.2.mira041.stderr:
2017-06-08T19:00:51.549 INFO:tasks.ceph.c1.osd.2.mira041.stderr: ceph version  12.0.2-2416-g3beddb9 (3beddb967cf6cbaaa6712bab86dbf3822cbe0843) luminous (dev)
2017-06-08T19:00:51.549 INFO:tasks.ceph.c1.osd.2.mira041.stderr: 1: (ceph::__ceph_assert_fail(char const*, char const*, int, char const*)+0x110) [0xafa850]
2017-06-08T19:00:51.549 INFO:tasks.ceph.c1.osd.2.mira041.stderr: 2: (()+0x5030da) [0x60b0da]
2017-06-08T19:00:51.549 INFO:tasks.ceph.c1.osd.2.mira041.stderr: 3: (OSDService::~OSDService()+0x17c) [0x594bac]
2017-06-08T19:00:51.549 INFO:tasks.ceph.c1.osd.2.mira041.stderr: 4: (OSD::~OSD()+0x133) [0x5e31a3]
2017-06-08T19:00:51.551 INFO:tasks.ceph.c1.osd.2.mira041.stderr: 5: (OSD::~OSD()+0x9) [0x5e37e9]
2017-06-08T19:00:51.551 INFO:tasks.ceph.c1.osd.2.mira041.stderr: 6: (main()+0x2f48) [0x4dc1b8]
2017-06-08T19:00:51.551 INFO:tasks.ceph.c1.osd.2.mira041.stderr: 7: (__libc_start_main()+0xf5) [0xd5bdb15]
2017-06-08T19:00:51.551 INFO:tasks.ceph.c1.osd.2.mira041.stderr: 8: (()+0x46d1d9) [0x5751d9]
2017-06-08T19:00:51.551 INFO:tasks.ceph.c1.osd.2.mira041.stderr: NOTE: a copy of the executable, or `objdump -rdS <executable>` is needed to interpret this.

@rzarzynski rzarzynski merged commit fe9fedf into ceph:master Jun 12, 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:compress_bug_fix branch Jan 10, 2018

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