From 5702139c4a003c5731107e8f877584cfab9f01ad Mon Sep 17 00:00:00 2001 From: Shriya Deshmukh Date: Mon, 8 Aug 2022 03:59:05 -0600 Subject: [PATCH 1/3] rgw_sal_motr:[CORTX-33799] Handle progress_cb response in case of copy-object operation failure. Problem: If copy operation failed after copying some data, then it was throwing an invalid XML error due to partial progress response. i.e rgw was not closing 'CopyObjectResult' tag in case of failure. Solution: Remove progress_cb call from rgw_sal_motr layer, and send 'dump_continue' signal to avoid client timeout during copy operation. Signed-off-by: Shriya Deshmukh --- src/rgw/rgw_sal_motr.cc | 16 +++++++--------- src/rgw/rgw_sal_motr.h | 7 +++++-- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/rgw/rgw_sal_motr.cc b/src/rgw/rgw_sal_motr.cc index f675939f17689..10f393311c1cd 100644 --- a/src/rgw/rgw_sal_motr.cc +++ b/src/rgw/rgw_sal_motr.cc @@ -41,6 +41,7 @@ extern "C" { #include "rgw_bucket.h" #include "rgw_quota.h" #include "motr/addb/rgw_addb.h" +#include "rgw_rest.h" #define dout_subsys ceph_subsys_rgw @@ -2230,12 +2231,12 @@ int MotrObject::delete_obj_aio(const DoutPrefixProvider* dpp, RGWObjState* astat int MotrCopyObj_CB::handle_data(bufferlist& bl, off_t bl_ofs, off_t bl_len) { - progress_cb progress_CB = this->get_progress_cb(); int rc = 0; ldpp_dout(m_dpp, 20) << "Offset=" << bl_ofs << " Length = " << " Write Offset=" << write_offset << bl_len << dendl; + struct req_state* s = static_cast(obj_ctx->get_private()); //offset is zero and bufferlength is equal to bl_len if (!bl_ofs && bl_len == bl.length()) { bufferptr bptr(bl.c_str(), bl_len); @@ -2248,9 +2249,7 @@ int MotrCopyObj_CB::handle_data(bufferlist& bl, off_t bl_ofs, off_t bl_len) write_offset << "failed rc=" << rc << dendl; } write_offset += bl_len; - if(progress_CB){ - progress_CB(write_offset, this->get_progress_data()); - } + dump_continue(s); return rc; } @@ -2264,6 +2263,7 @@ int MotrCopyObj_CB::handle_data(bufferlist& bl, off_t bl_ofs, off_t bl_len) << " Write Offset=" << write_offset << dendl; return rc; } + dump_continue(s); write_offset += bl_len; ldpp_dout(m_dpp, 20) << "MotrCopyObj_CB handle_data called rc=" << rc << dendl; @@ -2346,7 +2346,7 @@ int MotrObject::copy_object_same_zone(RGWObjectCtx& obj_ctx, } // Create filter object. - MotrCopyObj_CB cb(dpp, dst_writer); + MotrCopyObj_CB cb(dpp, dst_writer, &obj_ctx); MotrCopyObj_Filter* filter = &cb; // Get offsets. @@ -2357,9 +2357,7 @@ int MotrObject::copy_object_same_zone(RGWObjectCtx& obj_ctx, return rc; } - //setting the values of progress_cb and progress_data in MotrCopyObj_Filter class - filter->set_progress_callback(progress_cb, progress_data); - + struct req_state* s = static_cast(obj_ctx.get_private()); // read::iterate -> handle_data() -> write::process rc = read_op->iterate(dpp, cur_ofs, cur_end, filter, y); if (rc < 0){ @@ -2387,7 +2385,7 @@ int MotrObject::copy_object_same_zone(RGWObjectCtx& obj_ctx, } //Set object tags based on tagging-directive - struct req_state* s = static_cast(obj_ctx.get_private()); + auto tagging_drctv = s->info.env->get("HTTP_X_AMZ_TAGGING_DIRECTIVE"); bufferlist tags_bl; diff --git a/src/rgw/rgw_sal_motr.h b/src/rgw/rgw_sal_motr.h index f79cc983b9a67..b3158febd48a1 100644 --- a/src/rgw/rgw_sal_motr.h +++ b/src/rgw/rgw_sal_motr.h @@ -528,13 +528,16 @@ class MotrCopyObj_CB : public MotrCopyObj_Filter const DoutPrefixProvider* m_dpp; std::shared_ptr m_dst_writer; off_t write_offset; + RGWObjectCtx* obj_ctx; public: explicit MotrCopyObj_CB(const DoutPrefixProvider* dpp, - std::shared_ptr dst_writer) : + std::shared_ptr dst_writer, RGWObjectCtx* obj_ctx_1) : m_dpp(dpp), m_dst_writer(dst_writer), - write_offset(0) {} + write_offset(0) { + obj_ctx = obj_ctx_1; + } virtual ~MotrCopyObj_CB() override {} int handle_data(bufferlist& bl, off_t bl_ofs, off_t bl_len) override; }; From 79e24e0ba36d43e18bade1f734c69c5462081cf4 Mon Sep 17 00:00:00 2001 From: Shriya Deshmukh Date: Wed, 10 Aug 2022 05:13:06 -0600 Subject: [PATCH 2/3] Addressed review comments. Signed-off-by: Shriya Deshmukh --- src/rgw/rgw_sal_motr.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/rgw/rgw_sal_motr.cc b/src/rgw/rgw_sal_motr.cc index 10f393311c1cd..9e04f6bcecfdd 100644 --- a/src/rgw/rgw_sal_motr.cc +++ b/src/rgw/rgw_sal_motr.cc @@ -2357,7 +2357,6 @@ int MotrObject::copy_object_same_zone(RGWObjectCtx& obj_ctx, return rc; } - struct req_state* s = static_cast(obj_ctx.get_private()); // read::iterate -> handle_data() -> write::process rc = read_op->iterate(dpp, cur_ofs, cur_end, filter, y); if (rc < 0){ @@ -2385,7 +2384,7 @@ int MotrObject::copy_object_same_zone(RGWObjectCtx& obj_ctx, } //Set object tags based on tagging-directive - + struct req_state* s = static_cast(obj_ctx.get_private()); auto tagging_drctv = s->info.env->get("HTTP_X_AMZ_TAGGING_DIRECTIVE"); bufferlist tags_bl; From a8713d80b23889e7cb4828d5c9fa24ed9d61dea3 Mon Sep 17 00:00:00 2001 From: Shriya Deshmukh Date: Thu, 11 Aug 2022 22:43:19 -0600 Subject: [PATCH 3/3] Addressed review comments. Signed-off-by: Shriya Deshmukh --- src/rgw/rgw_sal_motr.cc | 4 +--- src/rgw/rgw_sal_motr.h | 7 +++---- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/rgw/rgw_sal_motr.cc b/src/rgw/rgw_sal_motr.cc index 9e04f6bcecfdd..2c43fc144e342 100644 --- a/src/rgw/rgw_sal_motr.cc +++ b/src/rgw/rgw_sal_motr.cc @@ -2235,8 +2235,6 @@ int MotrCopyObj_CB::handle_data(bufferlist& bl, off_t bl_ofs, off_t bl_len) ldpp_dout(m_dpp, 20) << "Offset=" << bl_ofs << " Length = " << " Write Offset=" << write_offset << bl_len << dendl; - - struct req_state* s = static_cast(obj_ctx->get_private()); //offset is zero and bufferlength is equal to bl_len if (!bl_ofs && bl_len == bl.length()) { bufferptr bptr(bl.c_str(), bl_len); @@ -2346,7 +2344,7 @@ int MotrObject::copy_object_same_zone(RGWObjectCtx& obj_ctx, } // Create filter object. - MotrCopyObj_CB cb(dpp, dst_writer, &obj_ctx); + MotrCopyObj_CB cb(dpp, dst_writer, obj_ctx); MotrCopyObj_Filter* filter = &cb; // Get offsets. diff --git a/src/rgw/rgw_sal_motr.h b/src/rgw/rgw_sal_motr.h index b3158febd48a1..6bbe942572eec 100644 --- a/src/rgw/rgw_sal_motr.h +++ b/src/rgw/rgw_sal_motr.h @@ -528,15 +528,14 @@ class MotrCopyObj_CB : public MotrCopyObj_Filter const DoutPrefixProvider* m_dpp; std::shared_ptr m_dst_writer; off_t write_offset; - RGWObjectCtx* obj_ctx; - + struct req_state *s; public: explicit MotrCopyObj_CB(const DoutPrefixProvider* dpp, - std::shared_ptr dst_writer, RGWObjectCtx* obj_ctx_1) : + std::shared_ptr dst_writer, RGWObjectCtx& obj_ctx) : m_dpp(dpp), m_dst_writer(dst_writer), write_offset(0) { - obj_ctx = obj_ctx_1; + s = static_cast(obj_ctx.get_private()); } virtual ~MotrCopyObj_CB() override {} int handle_data(bufferlist& bl, off_t bl_ofs, off_t bl_len) override;