rgw: Head/GetObject support partNumber#50148

Merged

cbodley merged 8 commits intoceph:mainfrom

cbodley:wip-58750

Nov 24, 2023

Contributor

cbodley commented Feb 17, 2023

aws s3 supports HEAD/GET requests for individual parts of a completed multipart upload with the partNumber query parameter

Show available Jenkins commands

jenkins retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test dashboard cephadm
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox
jenkins test windows

cbodley added feature rgw labels

cbodley commented

View reviewed changes

src/rgw/driver/rados/rgw_rados.cc Outdated Show resolved Hide resolved

cbodley mentioned this pull request

test Head/GetObject on completed multipart parts ceph/s3-tests#491

Merged

cbodley force-pushed the wip-58750 branch from 3e7c6a6 to 1b500ae Compare

February 20, 2023 21:31

Contributor Author

cbodley commented Feb 20, 2023

i rebased this on top of #50169 and #50172 so it can call get_obj_state() on the part's head object to get its attrs like etag etc

unfortunately, it turns out that multipart part uploads don't store their RGW_ATTR_MANIFEST in this head object. instead, they encode it in the RGWUploadPartInfo that we write to omap on the multipart meta object: https://github.com/ceph/ceph/blob/2a7301d/src/rgw/driver/rados/rgw_putobj_processor.cc#L457-L491

storing them in the multipart meta object makes it easier for RadosMultipartUpload::complete() to stitch the part manifests together for the final object without having to read the head object of each part

but without a manifest in part's head object, RGWRados::iterate_obj() would only read the first 4MB from the part's head object instead of the entire part. so get_part_obj_state() actually initializes a new RGWObjStateManifest::manifest, and uses RGWObjManifest::generator to copy over all of the requested part's extents from the original multipart manifest

i had to add a new get_obj_state() overload to expose RGWObjStateManifest directly. the original get_obj_state() function now wraps this new overload, returning RGWObjState and RGWObjManifest separately for the existing callers

Contributor Author

cbodley commented Feb 20, 2023

in the sal layer, this feature is only implemented by sal::Object::ReadOp and not sal::Object as a whole. sal shouldn't be able to accidentally overwrite or delete individual parts

the only part information that 'leaks' from ReadOp into sal::Object is the part object's size and its attributes; both of which are necessary for GetObj to return the correct Content-Length and Etag headers

cbodley force-pushed the wip-58750 branch from 8cd9d45 to 0dbc32e Compare

February 20, 2023 22:59

github-actions bot added the needs-rebase label

github-actions bot commented Mar 2, 2023

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

cbodley force-pushed the wip-58750 branch from 0dbc32e to 20de936 Compare

March 3, 2023 16:40

github-actions bot removed the needs-rebase label

github-actions bot added the needs-rebase label

github-actions bot commented Mar 12, 2023

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

cbodley force-pushed the wip-58750 branch from 20de936 to 6a8815f Compare

August 3, 2023 19:28

github-actions bot removed the needs-rebase label

cbodley changed the title ~~rgw: start on Head/GetObject support for partNumber~~ rgw: Head/GetObject support partNumber

cbodley marked this pull request as ready for review

August 3, 2023 20:49

cbodley requested a review from a team as a code owner

August 3, 2023 20:49

github-actions bot added documentation needs-rebase labels

github-actions bot commented Aug 4, 2023

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

dang approved these changes

View reviewed changes

Contributor Author

cbodley commented Aug 4, 2023

thanks @dang. this is passing the test cases in ceph/s3-tests#491. i'll tag for qa after rebasing

cbodley force-pushed the wip-58750 branch from b150b59 to 72465b0 Compare

August 4, 2023 14:50

github-actions bot removed the needs-rebase label

cbodley added needs-qa needs-rebase and removed needs-rebase labels

github-actions bot added the needs-rebase label

github-actions bot commented Oct 5, 2023

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

cbodley force-pushed the wip-58750 branch from 5f90248 to cc1fbab Compare

October 5, 2023 17:07

github-actions bot removed the needs-rebase label

cbodley force-pushed the wip-58750 branch from cc1fbab to de78703 Compare

October 6, 2023 14:35

github-actions bot added the needs-rebase label

github-actions bot commented Oct 10, 2023

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

Contributor Author

cbodley commented Oct 11, 2023

last qa run had one consistent java test failure:

suite > Object tests > ObjectTest.testDownloadHLAPI FAILED
    com.amazonaws.services.s3.model.AmazonS3Exception at ObjectTest.java:1979

there isn't any additional context for debugging, and i'm having trouble running the tests locally

cbodley commented

View reviewed changes

src/rgw/driver/rados/rgw_rados.cc

Comment on lines +6526 to +6544

+                // navigate to the requested part in the manifest
+                RGWObjManifest::obj_iterator end = manifest->obj_end(dpp);
+                if (end.get_cur_part_id() == 0) { // not multipart
+                  ldpp_dout(dpp, 20) << "object does not have a multipart manifest" << dendl;
+                  return -ERR_INVALID_PART;
+                }

Contributor Author

cbodley Oct 11, 2023

interesting, a java test case ObjectTest.testDownloadHLAPI is failing on this because it's requesting partNumber=1 for a non-multipart upload. i guess it expects to read the entire object in that case?

i suppose that could be used as an optimization to read partNumber=1 which, if multipart, will return the x-amz-mp-parts-count that you can then read as parts in parallel

Contributor Author

cbodley Nov 21, 2023

updated the test_non_multipart_get_part test case for this and see it passing

dang approved these changes

View reviewed changes

cbodley removed the needs-qa label

cbodley added 8 commits

November 21, 2023 10:24


          rgw/s3: add part param and response to GetObj

3354b67

Signed-off-by: Casey Bodley <cbodley@redhat.com>


          rgw/rados: remove get_obj_state() overload for follow_olh=true

60dadf3

and just add the follow_olh=true argument to callers

Signed-off-by: Casey Bodley <cbodley@redhat.com>


          rgw/rados: add get_obj_state() overload for RGWObjStateManifest

a308e3a

add an overload to expose the manifest storage to callers of
get_obj_state(). the existing RGWObjState+RGWObjManifest overload
just calls the RGWObjStateManifest one

Signed-off-by: Casey Bodley <cbodley@redhat.com>


          rgw/rados: add obj_find_part() to RGWObjManifest

8ae61ca

Signed-off-by: Casey Bodley <cbodley@redhat.com>


          rgw/rados: part support for RGWRados::Object::Read

01d8b4c

Signed-off-by: Casey Bodley <cbodley@redhat.com>


          rgw/rados: RadosReadOp::prepare only updates object instance

6fc5715

when called on a versioned object, prepare() may follow olh and look up
a different object instance

but when called on a multipart part, we should not overwrite the
original object name with the part's object name (of the form
mymultipart.2~_XLFNqOW0NuiALg7q4-Hi_7hdtAkZUH.1)

Signed-off-by: Casey Bodley <cbodley@redhat.com>


          ReleaseNotes: document support for partNumber

c02129e

Signed-off-by: Casey Bodley <cbodley@redhat.com>


          rgw: non-multipart uploads serve entire range on partNumber=1

338440a

and omit the x-amz-mp-parts-count response header

Signed-off-by: Casey Bodley <cbodley@redhat.com>

cbodley force-pushed the wip-58750 branch from 055ad05 to 338440a Compare

November 21, 2023 16:45

cbodley added the needs-qa label

github-actions bot removed the needs-rebase label

cbodley added the wip-cbodley-testing label

Contributor Author

cbodley commented Nov 24, 2023

passed qa in https://pulpito.ceph.com/cbodley-2023-11-23_19:53:09-rgw-wip-cbodley-testing-distro-default-smithi/ with rerun https://pulpito.ceph.com/cbodley-2023-11-23_21:48:12-rgw-wip-cbodley-testing-distro-default-smithi/

Contributor Author

cbodley commented Nov 24, 2023

included test cases from ceph/s3-tests#491:

s3tests_boto3/functional/test_s3.py::test_multipart_get_part PASSED [ 67%]
s3tests_boto3/functional/test_s3.py::test_non_multipart_get_part PASSED [ 67%]

cbodley merged commit e342927 into ceph:main

cbodley deleted the wip-58750 branch

November 24, 2023 00:13

cbodley mentioned this pull request

rgw/d3n: filter driver for d3n. #50198

Closed

14 tasks

cbodley mentioned this pull request

rgw: implement GetObjectAttributes #55259

Merged

14 tasks

cbodley mentioned this pull request

rgw: fix multipart get part when count==1 #58288

Merged

cbodley mentioned this pull request

reef: rgw: Head/GetObject support partNumber #62544

Merged

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

Labels

documentation feature needs-qa rgw wip-cbodley-testing