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

[RGW] Wip rgw compression #11494

Merged
merged 42 commits into from Nov 3, 2016
Merged

[RGW] Wip rgw compression #11494

merged 42 commits into from Nov 3, 2016

Conversation

aclamk
Copy link
Contributor

@aclamk aclamk commented Oct 14, 2016

Continuation of #10932.
I closed old one by mistake.

@cbodley cbodley self-assigned this Oct 14, 2016
@cbodley
Copy link
Contributor

cbodley commented Oct 14, 2016

unlucky timing on the rebase - gitbuilders, which use WITH_LTTNG=ON, hit some unrelated build failures. i had to rebase the ceph/wip_rgw_compression branch on top of 9983cd3 to fix

@cbodley
Copy link
Contributor

cbodley commented Oct 17, 2016

the teuthology queue is very sluggish lately, but most of the results are in from the latest run. so far all of the failures are unrelated known issues: valgrind, sync agent (HTTPConnectionPool timeouts and 'object not deleted from destination zone'), and a single s3tests failure caused by http://tracker.ceph.com/issues/17465

@cbodley
Copy link
Contributor

cbodley commented Oct 24, 2016

there's an interesting new merge conflict from #11567, which fixes a bug with zero-length segments in RGWGetObj::read_user_manifest_part(). this pr replaces that loop with a call to RGWRados::Object::Read::iterate(), so we'll need to make sure that function doesn't have to same issue with empty segments. i'll work with @mdw-at-linuxbox to get that tested

@@ -7572,7 +7605,8 @@ int RGWRados::copy_obj_data(RGWObjectCtx& obj_ctx,
}
}

ret = processor.complete(etag, mtime, set_mtime, attrs, delete_at);
// XXX: need to copy over compression attr and its orig_size here?
ret = processor.complete(ofs, etag, mtime, set_mtime, attrs, delete_at);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aclamk could you look into this when you get a chance? we're passing the swift copy tests as is, so it may be correct as is. in that case, we should just update the comment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is OK.

Reasoning:
Here we copy object, including attrs. Copying process is compression agnostic. However, all attrs are preserved, I assume including RGW_ATTR_COMPRESSION. Basing on that object can be read.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(discussed briefly during standup)
i agree that the compression attribute is preserved. i just worry about ofs as the first argument to complete() - my changes involved passing the uncompressed size here, so the bucket could track both compressed and uncompressed sizes

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i added some code to decode the uncompressed size from the compression attribute here, and included the commit cbodley@8e0d9a5 in https://github.com/cbodley/ceph/commits/wip-rgw-compression - feel free to cherry-pick that (it's passing s3 and swift tests)

@cbodley
Copy link
Contributor

cbodley commented Oct 25, 2016

@yehudasa could you please do a final review? only two remaining blockers: my XXX comment in RGWRados::copy_obj_data(), and the merge conflict with the zero-length segment fix from @mdw-at-linuxbox

@cbodley
Copy link
Contributor

cbodley commented Oct 26, 2016

@aclamk with help from @mdw-at-linuxbox, I was able to run his test successfully on a rebased version of this branch. this means that, when rebasing, you can resolve the conflicts in RGWGetObj::read_user_manifest_part() normally. the conflict in cls_rgw.cc is also trivial

i pushed my rebased branch to https://github.com/cbodley/ceph/commits/wip-rgw-compression for comparison if that helps

@cbodley
Copy link
Contributor

cbodley commented Nov 1, 2016

would love to see this make the kraken freeze

@aclamk could you please do the rebase and pull in my fix for copy_obj_data() at cbodley@8e0d9a5?

@yehudasa could you please review my commits related to tracking accounted_size in the bucket stats? specifically, that I'm not missing anything w.r.t. backward compatibility?

Ved-vampir and others added 18 commits November 2, 2016 11:31
Signed-off-by: Alyona Kiseleva <akiselyova@mirantis.com>
Signed-off-by: Alyona Kiseleva <akiselyova@mirantis.com>
Signed-off-by: Alyona Kiseleva <akiselyova@mirantis.com>
Signed-off-by: Alyona Kiseleva <akiselyova@mirantis.com>
Signed-off-by: Alyona Kiseleva <akiselyova@mirantis.com>
Signed-off-by: Alyona Kiseleva <akiselyova@mirantis.com>
Changed RGWPutObjProcessor interface with regard to hash calculation.

Signed-off-by: Adam Kupczyk (akupczyk@mirantis.com)

Conflicts:
	src/rgw/rgw_op.cc
	src/rgw/rgw_rados.cc
…sion filters.

Signed-off-by: Adam Kupczyk (akupczyk@mirantis.com)
…so processed.

Allows to implement flush-like operations on data.

Signed-off-by: Adam Kupczyk (akupczyk@mirantis.com)

Conflicts:
	src/rgw/rgw_op.cc
Allows to plug in decompression in future.

Signed-off-by: Adam Kupczyk (a.kupczyk@mirantis.com)

Conflicts:
	src/rgw/rgw_op.cc
Signed-off-by: Adam Kupczyk (akupczyk@mirantis.com)
Signed-off-by: Alyona Kiseleva <akiselyova@mirantis.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
Signed-off-by: Adam Kupczyk <akupczyk@mirantis.com>
Signed-off-by: Adam Kupczyk <akupczyk@mirantis.com>
Signed-off-by: Adam Kupczyk <akupczyk@mirantis.com>
Signed-off-by: Adam Kupczyk <akupczyk@mirantis.com>
Signed-off-by: Adam Kupczyk <akupczyk@mirantis.com>
Adam Kupczyk and others added 21 commits November 2, 2016 12:10
Signed-off-by: Adam Kupczyk <akupczyk@mirantis.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
RGWRados::get_obj_state() now initializes accounted_size based on
the compression attribute, if present

Signed-off-by: Casey Bodley <cbodley@redhat.com>
and use accounted_size for BucketList operations

Signed-off-by: Casey Bodley <cbodley@redhat.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
and use accounted_size for ListMultipart operations

Signed-off-by: Casey Bodley <cbodley@redhat.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
Signed-off-by: Adam Kupczyk <akupczyk@mirantis.com>
Signed-off-by: Adam Kupczyk <akupczyk@mirantis.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
… for size

Signed-off-by: Casey Bodley <cbodley@redhat.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
Signed-off-by: Adam Kupczyk <akupczyk@mirantis.com>
Added tests.

Signed-off-by: Adam Kupczyk <akupczyk@mirantis.com>
Signed-off-by: Adam Kupczyk <akupczyk@mirantis.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
Signed-off-by: Adam Kupczyk <akupczyk@mirantis.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
Signed-off-by: Casey Bodley <cbodley@redhat.com>
@cbodley
Copy link
Contributor

cbodley commented Nov 2, 2016

thanks @aclamk, looks good. i opened aclamk#2 against this branch to add some documentation

doc/rgw: document rgw_compression_type
@oritwas oritwas merged commit 40ba752 into ceph:master Nov 3, 2016
@cbodley
Copy link
Contributor

cbodley commented Nov 3, 2016

thanks to @aclamk and @Ved-vampir for their great work on this!

@liewegas
Copy link
Member

liewegas commented Nov 3, 2016 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants