From 016ebcfdb988eca8477a3d692a76e5ca9864d01e Mon Sep 17 00:00:00 2001
From: Cory Snyder <csnyder@iland.com>
Date: Thu, 10 Nov 2022 15:41:15 +0000
Subject: [PATCH] rgw: improvements to RGWDeleteMultiObj concurrency impl

Improves details of multi-object delete concurrency implementation
by avoiding extra allocations, using references instead of pointers,
etc.

Signed-off-by: Cory Snyder <csnyder@iland.com>
(cherry picked from commit d9a11462b89a929a57c464e75c21224e7a22ddd9)

Conflicts:
	src/common/options/rgw.yaml.in
	src/rgw/rgw_op.cc

Cherry-pick notes:
- Conflicts due to change in configuration declaration post-pacific
- White space conflicts
---
 src/common/options.cc |  2 +-
 src/rgw/rgw_op.cc     | 67 +++++++++++++++++++++----------------------
 src/rgw/rgw_op.h      |  3 +-
 3 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/src/common/options.cc b/src/common/options.cc
index 9fe5a1a5d5f..0cda39dda1d 100644
--- a/src/common/options.cc
+++ b/src/common/options.cc
@@ -5869,7 +5869,7 @@ std::vector<Option> get_rgw_options() {
     .set_description("Max number of concurrent RADOS requests when handling bucket shards."),
 
     Option("rgw_multi_obj_del_max_aio", Option::TYPE_UINT, Option::LEVEL_ADVANCED)
-    .set_default(128)
+    .set_default(16)
     .set_description("Max number of concurrent RADOS requests per multi-object delete request."),
 
     Option("rgw_enable_quota_threads", Option::TYPE_BOOL, Option::LEVEL_ADVANCED)
diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc
index 64e6b10da9b..075a0944c6e 100644
--- a/src/rgw/rgw_op.cc
+++ b/src/rgw/rgw_op.cc
@@ -12,7 +12,6 @@
 #include <boost/algorithm/string/predicate.hpp>
 #include <boost/optional.hpp>
 #include <boost/utility/in_place_factory.hpp>
-#include <boost/asio.hpp>
 
 #include "include/scope_guard.h"
 #include "common/Clock.h"
@@ -7049,21 +7048,21 @@ void RGWDeleteMultiObj::wait_flush(optional_yield y,
   }
 }
 
-void RGWDeleteMultiObj::handle_individual_object(const rgw_obj_key *o, optional_yield y,
+void RGWDeleteMultiObj::handle_individual_object(const rgw_obj_key& o, optional_yield y,
                                                  boost::asio::deadline_timer *formatter_flush_cond)
 {
   std::string version_id;
   RGWObjectCtx *obj_ctx = static_cast<RGWObjectCtx *>(s->obj_ctx);
-  std::unique_ptr<rgw::sal::RGWObject> obj = bucket->get_object(*o);
+  std::unique_ptr<rgw::sal::RGWObject> obj = bucket->get_object(o);
   if (s->iam_policy || ! s->iam_user_policies.empty() || !s->session_policies.empty()) {
     auto identity_policy_res = eval_identity_or_session_policies(s->iam_user_policies, s->env,
                                             boost::none,
-                                            o->instance.empty() ?
+                                            o.instance.empty() ?
                                             rgw::IAM::s3DeleteObject :
                                             rgw::IAM::s3DeleteObjectVersion,
                                             ARN(obj->get_obj()));
     if (identity_policy_res == Effect::Deny) {
-      send_partial_response(*o, false, "", -EACCES, formatter_flush_cond);
+      send_partial_response(o, false, "", -EACCES, formatter_flush_cond);
       return;
     }
 
@@ -7071,54 +7070,54 @@ void RGWDeleteMultiObj::handle_individual_object(const rgw_obj_key *o, optional_
     rgw::IAM::PolicyPrincipal princ_type = rgw::IAM::PolicyPrincipal::Other;
     if (s->iam_policy) {
       e = s->iam_policy->eval(s->env,
-      			   *s->auth.identity,
-      			   o->instance.empty() ?
-      			   rgw::IAM::s3DeleteObject :
-      			   rgw::IAM::s3DeleteObjectVersion,
-      			   ARN(obj->get_obj()),
-         princ_type);
+      			      *s->auth.identity,
+      			      o.instance.empty() ?
+      			      rgw::IAM::s3DeleteObject :
+      			      rgw::IAM::s3DeleteObjectVersion,
+      			      ARN(obj->get_obj()),
+                              princ_type);
     }
     if (e == Effect::Deny) {
-      send_partial_response(*o, false, "", -EACCES, formatter_flush_cond);
+      send_partial_response(o, false, "", -EACCES, formatter_flush_cond);
       return;
     }
 
     if (!s->session_policies.empty()) {
       auto session_policy_res = eval_identity_or_session_policies(s->session_policies, s->env,
-                                              boost::none,
-                                              o->instance.empty() ?
-                                            rgw::IAM::s3DeleteObject :
-                                            rgw::IAM::s3DeleteObjectVersion,
-                                            ARN(obj->get_obj()));
+                                                                  boost::none,
+                                                                  o.instance.empty() ?
+                                                                  rgw::IAM::s3DeleteObject :
+                                                                  rgw::IAM::s3DeleteObjectVersion,
+                                                                  ARN(obj->get_obj()));
       if (session_policy_res == Effect::Deny) {
-        send_partial_response(*o, false, "", -EACCES, formatter_flush_cond);
+        send_partial_response(o, false, "", -EACCES, formatter_flush_cond);
         return;
       }
       if (princ_type == rgw::IAM::PolicyPrincipal::Role) {
         //Intersection of session policy and identity policy plus intersection of session policy and bucket policy
         if ((session_policy_res != Effect::Allow || identity_policy_res != Effect::Allow) &&
             (session_policy_res != Effect::Allow || e != Effect::Allow)) {
-          send_partial_response(*o, false, "", -EACCES, formatter_flush_cond);
+          send_partial_response(o, false, "", -EACCES, formatter_flush_cond);
           return;
         }
       } else if (princ_type == rgw::IAM::PolicyPrincipal::Session) {
         //Intersection of session policy and identity policy plus bucket policy
         if ((session_policy_res != Effect::Allow || identity_policy_res != Effect::Allow) && e != Effect::Allow) {
-          send_partial_response(*o, false, "", -EACCES, formatter_flush_cond);
+          send_partial_response(o, false, "", -EACCES, formatter_flush_cond);
           return;
         }
       } else if (princ_type == rgw::IAM::PolicyPrincipal::Other) {// there was no match in the bucket policy
         if (session_policy_res != Effect::Allow || identity_policy_res != Effect::Allow) {
-          send_partial_response(*o, false, "", -EACCES, formatter_flush_cond);
+          send_partial_response(o, false, "", -EACCES, formatter_flush_cond);
           return;
         }
       }
-      send_partial_response(*o, false, "", -EACCES, formatter_flush_cond);
+      send_partial_response(o, false, "", -EACCES, formatter_flush_cond);
       return;
     }
 
     if ((identity_policy_res == Effect::Pass && e == Effect::Pass && !acl_allowed)) {
-      send_partial_response(*o, false, "", -EACCES, formatter_flush_cond);
+      send_partial_response(o, false, "", -EACCES, formatter_flush_cond);
       return;
     }
   }
@@ -7137,7 +7136,7 @@ void RGWDeleteMultiObj::handle_individual_object(const rgw_obj_key *o, optional_
         check_obj_lock = false;
       } else {
         // Something went wrong.
-        send_partial_response(*o, false, "", ret, formatter_flush_cond);
+        send_partial_response(o, false, "", ret, formatter_flush_cond);
         return;
       }
     } else {
@@ -7149,7 +7148,7 @@ void RGWDeleteMultiObj::handle_individual_object(const rgw_obj_key *o, optional_
       ceph_assert(astate);
       int object_lock_response = verify_object_lock(this, astate->attrset, bypass_perm, bypass_governance_mode);
       if (object_lock_response != 0) {
-        send_partial_response(*o, false, "", object_lock_response, formatter_flush_cond);
+        send_partial_response(o, false, "", object_lock_response, formatter_flush_cond);
         return;
       }
     }
@@ -7162,7 +7161,7 @@ void RGWDeleteMultiObj::handle_individual_object(const rgw_obj_key *o, optional_
       rgw::notify::ObjectRemovedDeleteMarkerCreated : rgw::notify::ObjectRemovedDelete;
   op_ret = rgw::notify::publish_reserve(this, event_type, res, nullptr);
   if (op_ret < 0) {
-    send_partial_response(*o, false, "", op_ret, formatter_flush_cond);
+    send_partial_response(o, false, "", op_ret, formatter_flush_cond);
     return;
   }
 
@@ -7174,7 +7173,7 @@ void RGWDeleteMultiObj::handle_individual_object(const rgw_obj_key *o, optional_
     op_ret = 0;
   }
 
-  send_partial_response(*o, obj->get_delete_marker(), version_id, op_ret, formatter_flush_cond);
+  send_partial_response(o, obj->get_delete_marker(), version_id, op_ret, formatter_flush_cond);
 
   // send request to notification manager
   const auto ret = rgw::notify::publish_commit(obj.get(), obj_size, ceph::real_clock::now(), etag, event_type, res, this);
@@ -7192,9 +7191,9 @@ void RGWDeleteMultiObj::execute(optional_yield y)
   uint32_t aio_count = 0;
   const uint32_t max_aio = s->cct->_conf->rgw_multi_obj_del_max_aio;
   char* buf;
-  std::unique_ptr<boost::asio::deadline_timer> formatter_flush_cond;
+  std::optional<boost::asio::deadline_timer> formatter_flush_cond;
   if (y) {
-    formatter_flush_cond = std::make_unique<boost::asio::deadline_timer>(y.get_io_context());  
+    formatter_flush_cond = std::make_optional<boost::asio::deadline_timer>(y.get_io_context());  
   }
 
   buf = data.c_str();
@@ -7256,21 +7255,21 @@ void RGWDeleteMultiObj::execute(optional_yield y)
   for (iter = multi_delete->objects.begin();
         iter != multi_delete->objects.end();
         ++iter) {
-    rgw_obj_key* obj_key = &*iter;
+    rgw_obj_key obj_key = *iter;
     if (y && max_aio > 1) {
-      wait_flush(y, formatter_flush_cond.get(), [&aio_count, max_aio] {
+      wait_flush(y, &*formatter_flush_cond, [&aio_count, max_aio] {
         return aio_count < max_aio;
       });
       aio_count++;
       spawn::spawn(y.get_yield_context(), [this, &y, &aio_count, obj_key, &formatter_flush_cond] (yield_context yield) {
-        handle_individual_object(obj_key, optional_yield { y.get_io_context(), yield }, formatter_flush_cond.get()); 
+        handle_individual_object(obj_key, optional_yield { y.get_io_context(), yield }, &*formatter_flush_cond); 
         aio_count--;
       }); 
     } else {
-      handle_individual_object(obj_key, y, formatter_flush_cond.get());
+      handle_individual_object(obj_key, y, &*formatter_flush_cond);
     }
   }
-  wait_flush(y, formatter_flush_cond.get(), [this, n=multi_delete->objects.size()] {
+  wait_flush(y, &*formatter_flush_cond, [this, n=multi_delete->objects.size()] {
     return n == ops_log_entries.size();
   });
 
diff --git a/src/rgw/rgw_op.h b/src/rgw/rgw_op.h
index da4dd75b0ad..9ba59c8ffb1 100644
--- a/src/rgw/rgw_op.h
+++ b/src/rgw/rgw_op.h
@@ -26,6 +26,7 @@
 #include <boost/utility/in_place_factory.hpp>
 #include <boost/function.hpp>
 #include <boost/container/flat_map.hpp>
+#include <boost/asio/deadline_timer.hpp>
 
 #include "common/armor.h"
 #include "common/mime.h"
@@ -1911,7 +1912,7 @@ class RGWDeleteMultiObj : public RGWOp {
    * Handles the deletion of an individual object and uses
    * set_partial_response to record the outcome. 
    */
-  void handle_individual_object(const rgw_obj_key *o,
+  void handle_individual_object(const rgw_obj_key& o,
 				optional_yield y,
                                 boost::asio::deadline_timer *formatter_flush_cond);
   
-- 
2.47.3