From 8cc0881bbbf1d958c9548c8ffc0a2a8174f53af8 Mon Sep 17 00:00:00 2001 From: Andriy Tkachuk Date: Mon, 12 Sep 2022 14:28:54 +0100 Subject: [PATCH 01/11] rgw_sal_motr: [CORTX-34181] allow multiparts with offline server Currently, we create an additional index for each multipart- uploaded object to store its parts. If some server is down, the index creation operation would fail, thus failing the multipart upload. Solution: don't create indexes on multipart uploads. Create an additional index to store objs parts when the bucket is created. Signed-off-by: Andriy Tkachuk --- src/rgw/motr/gc/gc.cc | 8 +-- src/rgw/motr/gc/gc.h | 2 +- src/rgw/rgw_sal_motr.cc | 112 +++++++++++++++++++--------------------- 3 files changed, 57 insertions(+), 65 deletions(-) diff --git a/src/rgw/motr/gc/gc.cc b/src/rgw/motr/gc/gc.cc index a87a92431a1f7..8841dd5dceb16 100644 --- a/src/rgw/motr/gc/gc.cc +++ b/src/rgw/motr/gc/gc.cc @@ -263,6 +263,7 @@ int MotrGC::process_parts(motr_gc_obj_info ginfo, std::time_t end_time) { std::vector keys(max_entries); std::vector vals(max_entries); + keys[0] = name; rc = store->next_query_by_name(ginfo.multipart_iname, keys, vals); if (rc < 0) { ldout(cct, 0) <<__func__<<": ERROR: next query failed. rc=" @@ -290,8 +291,7 @@ int MotrGC::process_parts(motr_gc_obj_info ginfo, std::time_t end_time) { part_name.append(buff); std::string obj_fqdn = ginfo.name + "." + part_name; - motr_gc_obj_info gc_obj(tag, obj_fqdn, mobj, ginfo.deletion_time, - info.size, false, ""); + motr_gc_obj_info gc_obj(tag, obj_fqdn, mobj, ginfo.deletion_time, info.size); rc = enqueue(gc_obj); if (rc < 0) { ldout(cct, 0) <<__func__<< ": ERROR: failed to push " @@ -312,10 +312,6 @@ int MotrGC::process_parts(motr_gc_obj_info ginfo, std::time_t end_time) { } } - if (processed_parts == number_of_parts) { - store->delete_motr_idx_by_name(ginfo.multipart_iname); - } - return rc; } diff --git a/src/rgw/motr/gc/gc.h b/src/rgw/motr/gc/gc.h index 25862cb3e0392..2d3bc7d1a3aa4 100644 --- a/src/rgw/motr/gc/gc.h +++ b/src/rgw/motr/gc/gc.h @@ -81,7 +81,7 @@ struct motr_gc_obj_info { motr_gc_obj_info() {} motr_gc_obj_info(const std::string& _tag, const std::string& _name, Meta& _mobj, const std::time_t& _deletion_time, const std::uint64_t& _size, - bool _is_multipart, const std::string& _multipart_iname) + bool _is_multipart=false, const std::string& _multipart_iname="") : tag(_tag), name(_name), mobj(_mobj), deletion_time(_deletion_time), size(_size), is_multipart(_is_multipart), multipart_iname(_multipart_iname) {} diff --git a/src/rgw/rgw_sal_motr.cc b/src/rgw/rgw_sal_motr.cc index 66225e1fee8f0..759e61b2c8c98 100644 --- a/src/rgw/rgw_sal_motr.cc +++ b/src/rgw/rgw_sal_motr.cc @@ -877,8 +877,14 @@ int MotrBucket::remove_bucket(const DoutPrefixProvider *dpp, bool delete_childre } // 3. Remove mp index?? - string bucket_multipart_iname = "motr.rgw.bucket." + tenant_bkt_name + ".multiparts"; - ret = store->delete_motr_idx_by_name(bucket_multipart_iname); + string iname = "motr.rgw.bucket." + tenant_bkt_name + ".multiparts.in-progress"; + ret = store->delete_motr_idx_by_name(iname); + if (ret < 0) { + ldpp_dout(dpp, LOG_ERROR) <<__func__ << ": ERROR: failed to remove multipart.in-progress index rc=" << ret << dendl; + return ret; + } + iname = "motr.rgw.bucket." + tenant_bkt_name + ".multiparts"; + ret = store->delete_motr_idx_by_name(iname); if (ret < 0) { ldpp_dout(dpp, LOG_ERROR) <<__func__ << ": ERROR: failed to remove multipart index rc=" << ret << dendl; return ret; @@ -1096,18 +1102,30 @@ int MotrBucket::create_multipart_indices() int rc; string tenant_bkt_name = get_bucket_name(info.bucket.tenant, info.bucket.name); - // Bucket multipart index stores in-progress multipart uploads. + // There are two additional indexes per bucket for multiparts: + // one for in-progress uploads, another one for completed uploads. + // Key is the object name + upload_id, value is a rgw_bucket_dir_entry. // An entry is inserted when a multipart upload is initialised ( // MotrMultipartUpload::init()) and will be removed when the upload // is completed (MotrMultipartUpload::complete()). // MotrBucket::list_multiparts() will scan this index to return all // in-progress multipart uploads in the bucket. - string bucket_multipart_iname = "motr.rgw.bucket." + tenant_bkt_name + ".multiparts"; - rc = store->create_motr_idx_by_name(bucket_multipart_iname); + string iname = "motr.rgw.bucket." + tenant_bkt_name + ".multiparts.in-progress"; + rc = store->create_motr_idx_by_name(iname); + if (rc < 0) { + ldout(store->cctx, LOG_ERROR) <<__func__ + << ": ERROR: failed to create bucket in-progress multiparts index " + << iname << ", rc=" << rc << dendl; + return rc; + } + + iname = "motr.rgw.bucket." + tenant_bkt_name + ".multiparts"; + rc = store->create_motr_idx_by_name(iname); if (rc < 0) { - ldout(store->cctx, LOG_ERROR) <<__func__ << ": ERROR: failed to create bucket multipart index " - << bucket_multipart_iname << dendl; + ldout(store->cctx, LOG_ERROR) <<__func__ + << ": ERROR: failed to create bucket multiparts index " + << iname << ", rc=" << rc << dendl; return rc; } @@ -1455,7 +1473,7 @@ int MotrBucket::list_multiparts(const DoutPrefixProvider *dpp, string tenant_bkt_name = get_bucket_name(this->get_tenant(), this->get_name()); string bucket_multipart_iname = - "motr.rgw.bucket." + tenant_bkt_name + ".multiparts"; + "motr.rgw.bucket." + tenant_bkt_name + ".multiparts.in-progress"; key_vec[0].clear(); key_vec[0].assign(marker.begin(), marker.end()); rc = store->next_query_by_name(bucket_multipart_iname, key_vec, val_vec, prefix, delim); @@ -2212,7 +2230,8 @@ int MotrObject::MotrDeleteOp::create_delete_marker(const DoutPrefixProvider* dpp int MotrObject::remove_mobj_and_index_entry( const DoutPrefixProvider* dpp, rgw_bucket_dir_entry& ent, std::string delete_key, std::string bucket_index_iname, - std::string bucket_name) { + std::string bucket_name) +{ int rc; bufferlist bl; uint64_t size_rounded = 0; @@ -2228,14 +2247,12 @@ int MotrObject::remove_mobj_and_index_entry( if (rc < 0) { ldpp_dout(dpp, 0) <<__func__<< ": ERROR: get_upload_id failed. rc=" << rc << dendl; } else { - std::string obj_fqdn = bucket_name + "/" + delete_key; - std::string obj_part_iname = "motr.rgw.object." + bucket_name + "." + - this->get_name() + "." + upload_id + - ".parts"; - ldpp_dout(dpp, 20) << __func__ << ": object part index=" << obj_part_iname << dendl; + std::string obj_fqdn = delete_key + "." + upload_id; + std::string iname = "motr.rgw.bucket." + bucket_name + ".multiparts"; + ldpp_dout(dpp, 20) << __func__ << ": object part index=" << iname << dendl; ::Meta *mobj = reinterpret_cast<::Meta*>(&this->meta); motr_gc_obj_info gc_obj(upload_id, obj_fqdn, *mobj, std::time(nullptr), - ent.meta.size, true, obj_part_iname); + ent.meta.size, true, iname); rc = store->get_gc()->enqueue(gc_obj); if (rc == 0) { pushed_to_gc = true; @@ -3975,10 +3992,7 @@ int MotrMultipartUpload::delete_parts(const DoutPrefixProvider *dpp, std::string << ", rc=" << rc << dendl; } - // Delete object part index. - string obj_part_iname = "motr.rgw.object." + tenant_bkt_name + "." + mp_obj.get_key() + - "." + upload_id + ".parts"; - return store->delete_motr_idx_by_name(obj_part_iname); + return rc; } int MotrMultipartUpload::abort(const DoutPrefixProvider *dpp, CephContext *cct, @@ -3991,7 +4005,7 @@ int MotrMultipartUpload::abort(const DoutPrefixProvider *dpp, CephContext *cct, meta_obj = get_meta_obj(); string tenant_bkt_name = get_bucket_name(meta_obj->get_bucket()->get_tenant(), meta_obj->get_bucket()->get_name()); string bucket_multipart_iname = - "motr.rgw.bucket." + tenant_bkt_name + ".multiparts"; + "motr.rgw.bucket." + tenant_bkt_name + ".multiparts.in-progress"; rc = store->do_idx_op_by_name(bucket_multipart_iname, M0_IC_GET, meta_obj->get_key().to_str(), bl); if (rc < 0) { @@ -4099,7 +4113,7 @@ int MotrMultipartUpload::init(const DoutPrefixProvider *dpp, optional_yield y, // when listing a bucket. string tenant_bkt_name = get_bucket_name(obj->get_bucket()->get_tenant(), obj->get_bucket()->get_name()); string bucket_multipart_iname = - "motr.rgw.bucket." + tenant_bkt_name + ".multiparts"; + "motr.rgw.bucket." + tenant_bkt_name + ".multiparts.in-progress"; rc = store->do_idx_op_by_name(bucket_multipart_iname, M0_IC_PUT, obj->get_key().to_str(), bl); } while (rc == -EEXIST); @@ -4108,22 +4122,6 @@ int MotrMultipartUpload::init(const DoutPrefixProvider *dpp, optional_yield y, ldpp_dout(dpp, LOG_ERROR) <<__func__ << ": ERROR: index opration failed, M0_IC_PUT rc="<< rc << dendl; return rc; } - string tenant_bkt_name = get_bucket_name(bucket->get_tenant(), bucket->get_name()); - //This is multipart init, you will always have upload id here. - string upload_id = get_upload_id(); - - // Create object part index. - string obj_part_iname = "motr.rgw.object." + tenant_bkt_name + "." + mp_obj.get_key() + - "." + upload_id + ".parts"; - ldpp_dout(dpp, 20) <<__func__ << ": object part index=" << obj_part_iname << dendl; - rc = store->create_motr_idx_by_name(obj_part_iname); - if (rc == -EEXIST) - rc = 0; - if (rc < 0) { - // TODO: clean the bucket index entry - ldpp_dout(dpp, LOG_ERROR) <<__func__ << ": ERROR: Failed to create object multipart index " << obj_part_iname << dendl; - return rc; - } // Add one to the object_count of the current bucket stats // Size will be added when parts are uploaded @@ -4174,15 +4172,14 @@ int MotrMultipartUpload::list_parts(const DoutPrefixProvider *dpp, CephContext * } } - string obj_part_iname = "motr.rgw.object." + tenant_bkt_name + "." + mp_obj.get_key() + - "." + upload_id + ".parts"; - ldpp_dout(dpp, 20) <<__func__ << ": object part index=" << obj_part_iname << dendl; + string iname = "motr.rgw.bucket." + tenant_bkt_name + ".multiparts"; + ldpp_dout(dpp, 20) <<__func__ << ": object part index=" << iname << dendl; key_vec[0].clear(); - key_vec[0] = "part."; + key_vec[0] = mp_obj.get_key() + "." + upload_id + "."; char buf[32]; snprintf(buf, sizeof(buf), "%08d", marker + 1); key_vec[0].append(buf); - rc = store->next_query_by_name(obj_part_iname, key_vec, val_vec); + rc = store->next_query_by_name(iname, key_vec, val_vec); if (rc < 0) { ldpp_dout(dpp, LOG_ERROR) <<__func__ << ": ERROR: NEXT query failed. rc=" << rc << dendl; return rc; @@ -4377,7 +4374,7 @@ int MotrMultipartUpload::complete(const DoutPrefixProvider *dpp, meta_obj = get_meta_obj(); string tenant_bkt_name = get_bucket_name(meta_obj->get_bucket()->get_tenant(), meta_obj->get_bucket()->get_name()); string bucket_multipart_iname = - "motr.rgw.bucket." + tenant_bkt_name + ".multiparts"; + "motr.rgw.bucket." + tenant_bkt_name + ".multiparts.in-progress"; rc = this->store->do_idx_op_by_name(bucket_multipart_iname, M0_IC_GET, meta_obj->get_key().to_str(), bl); ldpp_dout(dpp, 20) <<__func__ << ": read entry from bucket multipart index rc=" << rc << dendl; @@ -4481,7 +4478,7 @@ int MotrMultipartUpload::get_info(const DoutPrefixProvider *dpp, optional_yield bufferlist bl; string tenant_bkt_name = get_bucket_name(meta_obj->get_bucket()->get_tenant(), meta_obj->get_bucket()->get_name()); string bucket_multipart_iname = - "motr.rgw.bucket." + tenant_bkt_name + ".multiparts"; + "motr.rgw.bucket." + tenant_bkt_name + ".multiparts.in-progress"; int rc = this->store->do_idx_op_by_name(bucket_multipart_iname, M0_IC_GET, meta_obj->get_key().to_str(), bl); if (rc < 0) { @@ -4608,21 +4605,20 @@ int MotrMultipartWriter::complete(size_t accounted_size, const std::string& etag encode(attrs, bl); part_obj->meta.encode(bl); - string p = "part."; - char buf[32]; - snprintf(buf, sizeof(buf), "%08d", (int)part_num); - p.append(buf); - string tenant_bkt_name = get_bucket_name(head_obj->get_bucket()->get_tenant(), head_obj->get_bucket()->get_name()); //This is a MultipartComplete operation so this should always have valid upload id. - string upload_id_str = upload_id; - string obj_part_iname = "motr.rgw.object." + tenant_bkt_name + "." + - head_obj->get_key().to_str() + "." + upload_id_str + ".parts"; - ldpp_dout(dpp, 20) <<__func__ << ": object part index=" << obj_part_iname << dendl; + string part = head_obj->get_key().to_str() + "." + upload_id; + char buf[32]; + snprintf(buf, sizeof(buf), ".%08d", (int)part_num); + part.append(buf); // Before updating object part index with entry for new part, check if // old part exists. Perform M0_IC_GET operation on object part index. + string iname = "motr.rgw.bucket." + + get_bucket_name(head_obj->get_bucket()->get_tenant(), + head_obj->get_bucket()->get_name()) + + ".multiparts"; bufferlist old_part_check_bl; - rc = store->do_idx_op_by_name(obj_part_iname, M0_IC_GET, p, old_part_check_bl); + rc = store->do_idx_op_by_name(iname, M0_IC_GET, part, old_part_check_bl); if (rc == 0 && old_part_check_bl.length() > 0) { // Old part exists. Try to delete it. RGWUploadPartInfo old_part_info; @@ -4631,7 +4627,7 @@ int MotrMultipartWriter::complete(size_t accounted_size, const std::string& etag head_obj->get_key().to_str() + ".part." + std::to_string(part_num); std::unique_ptr old_part_obj = - std::make_unique(this->store, rgw_obj_key(part_obj_name),head_obj->get_bucket()); + std::make_unique(this->store, rgw_obj_key(part_obj_name), head_obj->get_bucket()); if (old_part_obj == nullptr) return -ENOMEM; @@ -4648,14 +4644,14 @@ int MotrMultipartWriter::complete(size_t accounted_size, const std::string& etag // Delete old object rc = old_mobj->delete_mobj(dpp); if (rc == 0) { - ldpp_dout(dpp, 20) <<__func__ << ": Old part [" << p << "] deleted succesfully" << dendl; + ldpp_dout(dpp, 20) <<__func__ << ": Old part [" << part << "] deleted succesfully" << dendl; } else { - ldpp_dout(dpp, 0) <<__func__ << ": Failed to delete old part [" << p << "]. Error=" << rc << dendl; + ldpp_dout(dpp, 0) <<__func__ << ": Failed to delete old part [" << part << "], rc=" << rc << dendl; return rc; } } - rc = store->do_idx_op_by_name(obj_part_iname, M0_IC_PUT, p, bl); + rc = store->do_idx_op_by_name(iname, M0_IC_PUT, part, bl); if (rc < 0) { ldpp_dout(dpp, 0) <<__func__ << ": failed to add part obj in part index, rc=" << rc << dendl; return rc == -ENOENT ? -ERR_NO_SUCH_UPLOAD : rc; From 0faff883a7cb2fd482ff244688eefe2102b90722 Mon Sep 17 00:00:00 2001 From: Andriy Tkachuk Date: Tue, 13 Sep 2022 13:36:15 +0100 Subject: [PATCH 02/11] Fix issues after 1st review round Signed-off-by: Andriy Tkachuk --- src/rgw/motr/gc/gc.cc | 2 +- src/rgw/rgw_sal_motr.cc | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/rgw/motr/gc/gc.cc b/src/rgw/motr/gc/gc.cc index 8841dd5dceb16..0f0d1e4d284e0 100644 --- a/src/rgw/motr/gc/gc.cc +++ b/src/rgw/motr/gc/gc.cc @@ -263,7 +263,7 @@ int MotrGC::process_parts(motr_gc_obj_info ginfo, std::time_t end_time) { std::vector keys(max_entries); std::vector vals(max_entries); - keys[0] = name; + keys[0] = ginfo.name; rc = store->next_query_by_name(ginfo.multipart_iname, keys, vals); if (rc < 0) { ldout(cct, 0) <<__func__<<": ERROR: next query failed. rc=" diff --git a/src/rgw/rgw_sal_motr.cc b/src/rgw/rgw_sal_motr.cc index 759e61b2c8c98..028485454ae75 100644 --- a/src/rgw/rgw_sal_motr.cc +++ b/src/rgw/rgw_sal_motr.cc @@ -4613,10 +4613,9 @@ int MotrMultipartWriter::complete(size_t accounted_size, const std::string& etag // Before updating object part index with entry for new part, check if // old part exists. Perform M0_IC_GET operation on object part index. - string iname = "motr.rgw.bucket." - + get_bucket_name(head_obj->get_bucket()->get_tenant(), - head_obj->get_bucket()->get_name()) - + ".multiparts"; + string tenant_bkt_name = get_bucket_name(head_obj->get_bucket()->get_tenant(), + head_obj->get_bucket()->get_name()); + string iname = "motr.rgw.bucket." + tenant_bkt_name + ".multiparts"; bufferlist old_part_check_bl; rc = store->do_idx_op_by_name(iname, M0_IC_GET, part, old_part_check_bl); if (rc == 0 && old_part_check_bl.length() > 0) { From d13dc95280c09cedb1258d8050635c7401817a68 Mon Sep 17 00:00:00 2001 From: Andriy Tkachuk Date: Tue, 13 Sep 2022 15:48:11 +0100 Subject: [PATCH 03/11] Drop not used variables in MotrGC::process_parts() Signed-off-by: Andriy Tkachuk --- src/rgw/motr/gc/gc.cc | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/rgw/motr/gc/gc.cc b/src/rgw/motr/gc/gc.cc index 0f0d1e4d284e0..86968d8db492a 100644 --- a/src/rgw/motr/gc/gc.cc +++ b/src/rgw/motr/gc/gc.cc @@ -258,8 +258,6 @@ int MotrGC::delete_obj_from_gc(motr_gc_obj_info ginfo) { int MotrGC::process_parts(motr_gc_obj_info ginfo, std::time_t end_time) { int rc = 0; int max_entries = 10000; - int number_of_parts = 0; - int processed_parts = 0; std::vector keys(max_entries); std::vector vals(max_entries); @@ -270,7 +268,6 @@ int MotrGC::process_parts(motr_gc_obj_info ginfo, std::time_t end_time) { << rc << dendl; return rc; } - number_of_parts = rc; for (const auto& bl: vals) { if (bl.length() == 0) break; @@ -298,7 +295,6 @@ int MotrGC::process_parts(motr_gc_obj_info ginfo, std::time_t end_time) { << obj_fqdn << "into GC queue " << dendl; continue; } - processed_parts++; bufferlist bl_del; rc = store->do_idx_op_by_name(ginfo.multipart_iname, M0_IC_DEL, part_name, bl_del); From fac635ab3a2756926f6abed13e7c79245049dfd7 Mon Sep 17 00:00:00 2001 From: Andriy Tkachuk Date: Tue, 13 Sep 2022 15:53:38 +0100 Subject: [PATCH 04/11] Fix compilation error at MotrMultipartUpload::init() Signed-off-by: Andriy Tkachuk --- src/rgw/rgw_sal_motr.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rgw/rgw_sal_motr.cc b/src/rgw/rgw_sal_motr.cc index 028485454ae75..e066042c27db1 100644 --- a/src/rgw/rgw_sal_motr.cc +++ b/src/rgw/rgw_sal_motr.cc @@ -4066,6 +4066,7 @@ int MotrMultipartUpload::init(const DoutPrefixProvider *dpp, optional_yield y, owner = _owner; + string tenant_bkt_name = get_bucket_name(bucket()->get_tenant(), bucket()->get_name()); do { char buf[33]; string tmp_obj_name; @@ -4111,7 +4112,6 @@ int MotrMultipartUpload::init(const DoutPrefixProvider *dpp, optional_yield y, encode(attrs, bl); // Insert an entry into bucket multipart index so it is not shown // when listing a bucket. - string tenant_bkt_name = get_bucket_name(obj->get_bucket()->get_tenant(), obj->get_bucket()->get_name()); string bucket_multipart_iname = "motr.rgw.bucket." + tenant_bkt_name + ".multiparts.in-progress"; rc = store->do_idx_op_by_name(bucket_multipart_iname, From ae4cfc0f71295e36a88fe782b227b1f9c0f9b739 Mon Sep 17 00:00:00 2001 From: Andriy Tkachuk Date: Tue, 13 Sep 2022 16:05:57 +0100 Subject: [PATCH 05/11] Drop excess is_multipart arg from motr_gc_obj_info() Signed-off-by: Andriy Tkachuk --- src/rgw/motr/gc/gc.h | 8 ++++---- src/rgw/rgw_sal_motr.cc | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/rgw/motr/gc/gc.h b/src/rgw/motr/gc/gc.h index 2d3bc7d1a3aa4..d269a63d64372 100644 --- a/src/rgw/motr/gc/gc.h +++ b/src/rgw/motr/gc/gc.h @@ -81,10 +81,10 @@ struct motr_gc_obj_info { motr_gc_obj_info() {} motr_gc_obj_info(const std::string& _tag, const std::string& _name, Meta& _mobj, const std::time_t& _deletion_time, const std::uint64_t& _size, - bool _is_multipart=false, const std::string& _multipart_iname="") - : tag(_tag), name(_name), mobj(_mobj), - deletion_time(_deletion_time), size(_size), - is_multipart(_is_multipart), multipart_iname(_multipart_iname) {} + const std::string& _multipart_iname="") + : tag(_tag), name(_name), mobj(_mobj), deletion_time(_deletion_time), + size(_size), is_multipart(_multipart_iname != "" ? true : false), + multipart_iname(_multipart_iname) {} void encode(bufferlist &bl) const { ENCODE_START(11, 2, bl); diff --git a/src/rgw/rgw_sal_motr.cc b/src/rgw/rgw_sal_motr.cc index e066042c27db1..056ed0671f555 100644 --- a/src/rgw/rgw_sal_motr.cc +++ b/src/rgw/rgw_sal_motr.cc @@ -2252,7 +2252,7 @@ int MotrObject::remove_mobj_and_index_entry( ldpp_dout(dpp, 20) << __func__ << ": object part index=" << iname << dendl; ::Meta *mobj = reinterpret_cast<::Meta*>(&this->meta); motr_gc_obj_info gc_obj(upload_id, obj_fqdn, *mobj, std::time(nullptr), - ent.meta.size, true, iname); + ent.meta.size, iname); rc = store->get_gc()->enqueue(gc_obj); if (rc == 0) { pushed_to_gc = true; @@ -2283,7 +2283,7 @@ int MotrObject::remove_mobj_and_index_entry( std::string obj_fqdn = bucket_name + "/" + delete_key; ::Meta *mobj = reinterpret_cast<::Meta*>(&this->meta); motr_gc_obj_info gc_obj(tag, obj_fqdn, *mobj, std::time(nullptr), - ent.meta.size, false, ""); + ent.meta.size); rc = store->get_gc()->enqueue(gc_obj); if (rc == 0) { pushed_to_gc = true; From 19e1b96dd6ad6f035dd82ee61ce93d0124bf4365 Mon Sep 17 00:00:00 2001 From: Andriy Tkachuk Date: Wed, 14 Sep 2022 10:46:30 +0100 Subject: [PATCH 06/11] Update src/rgw/rgw_sal_motr.cc Co-authored-by: Sumedh Anantrao Kulkarni Signed-off-by: Andriy Tkachuk --- src/rgw/rgw_sal_motr.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rgw/rgw_sal_motr.cc b/src/rgw/rgw_sal_motr.cc index 056ed0671f555..3160c90b683e0 100644 --- a/src/rgw/rgw_sal_motr.cc +++ b/src/rgw/rgw_sal_motr.cc @@ -2247,7 +2247,7 @@ int MotrObject::remove_mobj_and_index_entry( if (rc < 0) { ldpp_dout(dpp, 0) <<__func__<< ": ERROR: get_upload_id failed. rc=" << rc << dendl; } else { - std::string obj_fqdn = delete_key + "." + upload_id; + std::string obj_fqdn = this->get_name() + "." + upload_id; std::string iname = "motr.rgw.bucket." + bucket_name + ".multiparts"; ldpp_dout(dpp, 20) << __func__ << ": object part index=" << iname << dendl; ::Meta *mobj = reinterpret_cast<::Meta*>(&this->meta); From 29b4a97dfbacfe7c2f9a3adf13ed6fb5229de9ad Mon Sep 17 00:00:00 2001 From: Andriy Tkachuk Date: Wed, 14 Sep 2022 12:54:20 +0100 Subject: [PATCH 07/11] Fix obj-part key names in bucket parts index Signed-off-by: Andriy Tkachuk --- src/rgw/rgw_sal_motr.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/rgw/rgw_sal_motr.cc b/src/rgw/rgw_sal_motr.cc index 3160c90b683e0..b210ea9fc127b 100644 --- a/src/rgw/rgw_sal_motr.cc +++ b/src/rgw/rgw_sal_motr.cc @@ -2247,7 +2247,7 @@ int MotrObject::remove_mobj_and_index_entry( if (rc < 0) { ldpp_dout(dpp, 0) <<__func__<< ": ERROR: get_upload_id failed. rc=" << rc << dendl; } else { - std::string obj_fqdn = this->get_name() + "." + upload_id; + std::string obj_fqdn = this->get_key_str() + "." + upload_id; std::string iname = "motr.rgw.bucket." + bucket_name + ".multiparts"; ldpp_dout(dpp, 20) << __func__ << ": object part index=" << iname << dendl; ::Meta *mobj = reinterpret_cast<::Meta*>(&this->meta); @@ -4175,9 +4175,9 @@ int MotrMultipartUpload::list_parts(const DoutPrefixProvider *dpp, CephContext * string iname = "motr.rgw.bucket." + tenant_bkt_name + ".multiparts"; ldpp_dout(dpp, 20) <<__func__ << ": object part index=" << iname << dendl; key_vec[0].clear(); - key_vec[0] = mp_obj.get_key() + "." + upload_id + "."; + key_vec[0] = mp_obj.get_key_str() + "." + upload_id; char buf[32]; - snprintf(buf, sizeof(buf), "%08d", marker + 1); + snprintf(buf, sizeof(buf), ".%08d", marker + 1); key_vec[0].append(buf); rc = store->next_query_by_name(iname, key_vec, val_vec); if (rc < 0) { @@ -4606,7 +4606,7 @@ int MotrMultipartWriter::complete(size_t accounted_size, const std::string& etag part_obj->meta.encode(bl); //This is a MultipartComplete operation so this should always have valid upload id. - string part = head_obj->get_key().to_str() + "." + upload_id; + string part = head_obj->get_key_str() + "." + upload_id; char buf[32]; snprintf(buf, sizeof(buf), ".%08d", (int)part_num); part.append(buf); From 2eff226123d9125b16b47e06f496c7189ebd9764 Mon Sep 17 00:00:00 2001 From: Andriy Tkachuk Date: Wed, 14 Sep 2022 13:03:21 +0100 Subject: [PATCH 08/11] Fix minor silly overcoding issue Signed-off-by: Andriy Tkachuk --- src/rgw/motr/gc/gc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rgw/motr/gc/gc.h b/src/rgw/motr/gc/gc.h index d269a63d64372..2f90ec6b60c94 100644 --- a/src/rgw/motr/gc/gc.h +++ b/src/rgw/motr/gc/gc.h @@ -83,7 +83,7 @@ struct motr_gc_obj_info { const std::time_t& _deletion_time, const std::uint64_t& _size, const std::string& _multipart_iname="") : tag(_tag), name(_name), mobj(_mobj), deletion_time(_deletion_time), - size(_size), is_multipart(_multipart_iname != "" ? true : false), + size(_size), is_multipart(_multipart_iname != ""), multipart_iname(_multipart_iname) {} void encode(bufferlist &bl) const { From abc4e74d6be9ede2043ad95ec54e45bd0f22683b Mon Sep 17 00:00:00 2001 From: Andriy Tkachuk Date: Wed, 14 Sep 2022 13:35:08 +0100 Subject: [PATCH 09/11] Fix obj-part key names again Signed-off-by: Andriy Tkachuk --- src/rgw/rgw_sal_motr.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/rgw/rgw_sal_motr.cc b/src/rgw/rgw_sal_motr.cc index b210ea9fc127b..279fe69cb1d6a 100644 --- a/src/rgw/rgw_sal_motr.cc +++ b/src/rgw/rgw_sal_motr.cc @@ -2247,7 +2247,7 @@ int MotrObject::remove_mobj_and_index_entry( if (rc < 0) { ldpp_dout(dpp, 0) <<__func__<< ": ERROR: get_upload_id failed. rc=" << rc << dendl; } else { - std::string obj_fqdn = this->get_key_str() + "." + upload_id; + std::string obj_fqdn = this->get_name() + "." + upload_id; std::string iname = "motr.rgw.bucket." + bucket_name + ".multiparts"; ldpp_dout(dpp, 20) << __func__ << ": object part index=" << iname << dendl; ::Meta *mobj = reinterpret_cast<::Meta*>(&this->meta); @@ -4175,7 +4175,7 @@ int MotrMultipartUpload::list_parts(const DoutPrefixProvider *dpp, CephContext * string iname = "motr.rgw.bucket." + tenant_bkt_name + ".multiparts"; ldpp_dout(dpp, 20) <<__func__ << ": object part index=" << iname << dendl; key_vec[0].clear(); - key_vec[0] = mp_obj.get_key_str() + "." + upload_id; + key_vec[0] = mp_obj.get_key() + "." + upload_id; char buf[32]; snprintf(buf, sizeof(buf), ".%08d", marker + 1); key_vec[0].append(buf); @@ -4606,7 +4606,7 @@ int MotrMultipartWriter::complete(size_t accounted_size, const std::string& etag part_obj->meta.encode(bl); //This is a MultipartComplete operation so this should always have valid upload id. - string part = head_obj->get_key_str() + "." + upload_id; + string part = head_obj->get_name() + "." + upload_id; char buf[32]; snprintf(buf, sizeof(buf), ".%08d", (int)part_num); part.append(buf); From 9b53c15bbbbf541f6bdfa9ae5dcc56370793042a Mon Sep 17 00:00:00 2001 From: Andriy Tkachuk Date: Wed, 14 Sep 2022 14:21:28 +0100 Subject: [PATCH 10/11] Fix compilation error and warning Signed-off-by: Andriy Tkachuk --- src/rgw/rgw_sal_motr.cc | 12 +++++------- src/rgw/rgw_sal_motr.h | 2 +- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/rgw/rgw_sal_motr.cc b/src/rgw/rgw_sal_motr.cc index 279fe69cb1d6a..bcc843243ab98 100644 --- a/src/rgw/rgw_sal_motr.cc +++ b/src/rgw/rgw_sal_motr.cc @@ -2714,10 +2714,9 @@ int MotrObject::create_mobj(const DoutPrefixProvider *dpp, uint64_t sz) } expected_obj_size = sz; chunk_io_sz = expected_obj_size; - if (expected_obj_size > MAX_ACC_SIZE) { + if (expected_obj_size > MAX_ACC_SIZE) // Cap it to MAX_ACC_SIZE chunk_io_sz = MAX_ACC_SIZE; - } ldpp_dout(dpp, 20) <<__func__ << ": key=" << this->get_key().to_str() << " size=" << sz << " meta:oid=[0x" << std::hex @@ -2945,11 +2944,10 @@ int MotrObject::write_mobj(const DoutPrefixProvider *dpp, bufferlist&& in_buffer available_data = io_ctxt.total_bufer_sz; } bs = this->get_optimal_bs(chunk_io_sz); - if (bs < chunk_io_sz) { + if (bs < chunk_io_sz) chunk_io_sz = bs; - } - int64_t remaining_bytes = - expected_obj_size - processed_bytes; + + int64_t remaining_bytes = expected_obj_size - processed_bytes; // Check if this is the last io of the original object size if (remaining_bytes <= 0) last_io = true; @@ -4066,7 +4064,7 @@ int MotrMultipartUpload::init(const DoutPrefixProvider *dpp, optional_yield y, owner = _owner; - string tenant_bkt_name = get_bucket_name(bucket()->get_tenant(), bucket()->get_name()); + string tenant_bkt_name = get_bucket_name(bucket->get_tenant(), bucket->get_name()); do { char buf[33]; string tmp_obj_name; diff --git a/src/rgw/rgw_sal_motr.h b/src/rgw/rgw_sal_motr.h index 64166e75720ae..295d9b3f4f18e 100644 --- a/src/rgw/rgw_sal_motr.h +++ b/src/rgw/rgw_sal_motr.h @@ -563,7 +563,7 @@ class MotrObject : public Object { uint64_t part_num; // Object size as available from Content-Length header uint64_t expected_obj_size = 0; - uint64_t chunk_io_sz = 0; + int64_t chunk_io_sz = 0; // Total Number of bytes processed so far uint64_t processed_bytes = 0; struct AccumulateIOCtxt io_ctxt = {}; From f2bd5e0ea703bba24fec4b3d8ccdfa275d7c3c30 Mon Sep 17 00:00:00 2001 From: Andriy Tkachuk Date: Fri, 16 Sep 2022 17:06:57 +0100 Subject: [PATCH 11/11] Fix list_parts() and MotrGC::process_parts() Use key prefix when calling next_query_by_name() from bucket multiparts index to avoid fetching not relevant parts. Also, use the correct key at MotrGC::process_parts() when deleting the parts from bucket multiparts index. Signed-off-by: Andriy Tkachuk --- src/rgw/motr/gc/gc.cc | 5 +++-- src/rgw/rgw_sal_motr.cc | 10 ++++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/rgw/motr/gc/gc.cc b/src/rgw/motr/gc/gc.cc index 86968d8db492a..473cbadf31103 100644 --- a/src/rgw/motr/gc/gc.cc +++ b/src/rgw/motr/gc/gc.cc @@ -262,7 +262,7 @@ int MotrGC::process_parts(motr_gc_obj_info ginfo, std::time_t end_time) { std::vector vals(max_entries); keys[0] = ginfo.name; - rc = store->next_query_by_name(ginfo.multipart_iname, keys, vals); + rc = store->next_query_by_name(ginfo.multipart_iname, keys, vals, ginfo.name); if (rc < 0) { ldout(cct, 0) <<__func__<<": ERROR: next query failed. rc=" << rc << dendl; @@ -296,8 +296,9 @@ int MotrGC::process_parts(motr_gc_obj_info ginfo, std::time_t end_time) { continue; } bufferlist bl_del; + int index = &bl - &vals[0]; rc = store->do_idx_op_by_name(ginfo.multipart_iname, - M0_IC_DEL, part_name, bl_del); + M0_IC_DEL, keys[index], bl_del); if (rc < 0) { ldout(cct, 0) <<__func__<< ": ERROR: failed to remove part " << part_name << " from part index " << ginfo.multipart_iname << dendl; diff --git a/src/rgw/rgw_sal_motr.cc b/src/rgw/rgw_sal_motr.cc index 4dfd8ed39af8c..43f288947448f 100644 --- a/src/rgw/rgw_sal_motr.cc +++ b/src/rgw/rgw_sal_motr.cc @@ -4187,10 +4187,11 @@ int MotrMultipartUpload::list_parts(const DoutPrefixProvider *dpp, CephContext * ldpp_dout(dpp, 20) <<__func__ << ": object part index=" << iname << dendl; key_vec[0].clear(); key_vec[0] = mp_obj.get_key() + "." + upload_id; + string prefix = key_vec[0]; char buf[32]; snprintf(buf, sizeof(buf), ".%08d", marker + 1); key_vec[0].append(buf); - rc = store->next_query_by_name(iname, key_vec, val_vec); + rc = store->next_query_by_name(iname, key_vec, val_vec, prefix); if (rc < 0) { ldpp_dout(dpp, LOG_ERROR) <<__func__ << ": ERROR: NEXT query failed. rc=" << rc << dendl; return rc; @@ -4229,9 +4230,10 @@ int MotrMultipartUpload::list_parts(const DoutPrefixProvider *dpp, CephContext * } // Does it have more parts? - if (truncated) - *truncated = part_cnt < num_parts? false : true; - ldpp_dout(dpp, 20) <<__func__ << ": truncated=" << *truncated << dendl; + if (truncated != NULL) { + *truncated = part_cnt >= num_parts; + ldpp_dout(dpp, 20) <<__func__ << ": truncated=" << *truncated << dendl; + } if (next_marker) *next_marker = last_num;