From 338440ae2c04ed6a14d35825f4503d1222266ed5 Mon Sep 17 00:00:00 2001 From: Casey Bodley Date: Wed, 11 Oct 2023 17:45:17 -0400 Subject: [PATCH] rgw: non-multipart uploads serve entire range on partNumber=1 and omit the x-amz-mp-parts-count response header Signed-off-by: Casey Bodley --- src/rgw/driver/rados/rgw_rados.cc | 22 +++++++++++++++------- src/rgw/driver/rados/rgw_rados.h | 2 +- src/rgw/driver/rados/rgw_sal_rados.cc | 2 +- src/rgw/rgw_op.cc | 3 +-- src/rgw/rgw_sal.h | 6 +++--- 5 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/rgw/driver/rados/rgw_rados.cc b/src/rgw/driver/rados/rgw_rados.cc index c5624ba0171ec..3ee43dd91a89c 100644 --- a/src/rgw/driver/rados/rgw_rados.cc +++ b/src/rgw/driver/rados/rgw_rados.cc @@ -6533,6 +6533,9 @@ static int get_part_obj_state(const DoutPrefixProvider* dpp, optional_yield y, int part_num, int* parts_count, bool prefetch, RGWObjState** pstate, RGWObjManifest** pmanifest) { + if (!manifest) { + return -ERR_INVALID_PART; + } // 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 @@ -6639,22 +6642,27 @@ int RGWRados::Object::Read::prepare(optional_yield y, const DoutPrefixProvider * RGWBucketInfo& bucket_info = source->get_bucket_info(); if (params.part_num) { + int parts_count = 0; // use the manifest to redirect to the requested part number - if (!manifest) { - return -ERR_INVALID_PART; - } r = get_part_obj_state(dpp, y, store, bucket_info, &source->get_ctx(), - manifest, *params.part_num, params.parts_count, + manifest, *params.part_num, &parts_count, part_prefetch, &astate, &manifest); - if (r < 0) { + if (r == -ERR_INVALID_PART && *params.part_num == 1) { + // for non-multipart uploads, treat requests for the first part as a + // request for the entire range. this behavior is expected by the java + // sdk's TransferManager.download() + ldpp_dout(dpp, 4) << "requested part #" << *params.part_num + << ": " << cpp_strerror(r) << dendl; + } else if (r < 0) { ldpp_dout(dpp, 4) << "failed to read part #" << *params.part_num << ": " << cpp_strerror(r) << dendl; return -ERR_INVALID_PART; - } - if (!astate->exists) { + } else if (!astate->exists) { ldpp_dout(dpp, 4) << "part #" << *params.part_num << " does not exist" << dendl; return -ERR_INVALID_PART; + } else { + params.parts_count = parts_count; } } diff --git a/src/rgw/driver/rados/rgw_rados.h b/src/rgw/driver/rados/rgw_rados.h index 89e4576a4788e..f61730cb4de3e 100644 --- a/src/rgw/driver/rados/rgw_rados.h +++ b/src/rgw/driver/rados/rgw_rados.h @@ -770,7 +770,7 @@ public: rgw_obj *target_obj; uint64_t *epoch; int* part_num = nullptr; - int* parts_count = nullptr; + std::optional parts_count; Params() : lastmod(nullptr), obj_size(nullptr), attrs(nullptr), target_obj(nullptr), epoch(nullptr) diff --git a/src/rgw/driver/rados/rgw_sal_rados.cc b/src/rgw/driver/rados/rgw_sal_rados.cc index f536b7609e85f..b60ddd7a4d18b 100644 --- a/src/rgw/driver/rados/rgw_sal_rados.cc +++ b/src/rgw/driver/rados/rgw_sal_rados.cc @@ -1947,7 +1947,6 @@ int RadosObject::RadosReadOp::prepare(optional_yield y, const DoutPrefixProvider parent_op.params.lastmod = params.lastmod; parent_op.params.target_obj = params.target_obj; parent_op.params.part_num = params.part_num; - parent_op.params.parts_count = params.parts_count; parent_op.params.obj_size = &obj_size; parent_op.params.attrs = &source->get_attrs(); @@ -1957,6 +1956,7 @@ int RadosObject::RadosReadOp::prepare(optional_yield y, const DoutPrefixProvider source->set_instance(parent_op.state.obj.key.instance); source->set_obj_size(obj_size); + params.parts_count = parent_op.params.parts_count; return ret; } diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc index 1a9cd11c6eff5..ae89c703376a2 100644 --- a/src/rgw/rgw_op.cc +++ b/src/rgw/rgw_op.cc @@ -2283,8 +2283,6 @@ void RGWGetObj::execute(optional_yield y) read_op->params.lastmod = &lastmod; if (multipart_part_num) { read_op->params.part_num = &*multipart_part_num; - multipart_parts_count.emplace(0); - read_op->params.parts_count = &*multipart_parts_count; } op_ret = read_op->prepare(s->yield, this); @@ -2293,6 +2291,7 @@ void RGWGetObj::execute(optional_yield y) version_id = s->object->get_instance(); s->obj_size = s->object->get_obj_size(); attrs = s->object->get_attrs(); + multipart_parts_count = read_op->params.parts_count; /* STAT ops don't need data, and do no i/o */ if (get_type() == RGW_OP_STAT_OBJ) { diff --git a/src/rgw/rgw_sal.h b/src/rgw/rgw_sal.h index 57c7c20d6e15f..f0314517bebda 100644 --- a/src/rgw/rgw_sal.h +++ b/src/rgw/rgw_sal.h @@ -846,9 +846,9 @@ class Object { /// If non-null, read data/attributes from the given multipart part. int* part_num{nullptr}; - /// If part_num is specified, the total number of multipart parts is - /// written to this output parameter. - int* parts_count{nullptr}; + /// If part_num is specified and the object is multipart, the total + /// number of multipart parts is assigned to this output parameter. + std::optional parts_count; } params; virtual ~ReadOp() = default; -- 2.39.5