rgw: release cls lock if taken in RGWCompleteMultipart

Follows Casey's proposal to conditionally release the lock in
::complete(), in order to avoid duplicated code in various early
return cases.

Fixes: http://tracker.ceph.com/issues/21596
Signed-off-by: Matt Benjamin <mbenjamin@redhat.com>
(cherry picked from commit 704f793f08a02760d23eb5778b738bb07be0e7cf)

diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc
index f7e5caf67f51dd45763bc8c3eec39391ab2ae005..d8559b583db174ebe0be48336151312a76e6d8bc 100644 (file)
--- a/src/rgw/rgw_op.cc
+++ b/src/rgw/rgw_op.cc
@@ -5343,22 +5343,16 @@ void RGWCompleteMultipart::execute()
     from deleting the parts*/
   rgw_pool meta_pool;
   rgw_raw_obj raw_obj;
-  librados::ObjectWriteOperation op;
-  librados::IoCtx ioctx;
-  rados::cls::lock::Lock l("RGWCompleteMultipart");
-  int max_lock_secs_mp = s->cct->_conf->get_val<int64_t>("rgw_mp_lock_max_time");
+  int max_lock_secs_mp =
+    s->cct->_conf->get_val<int64_t>("rgw_mp_lock_max_time");
+  utime_t dur(max_lock_secs_mp, 0);
 
-  op.assert_exists();
   store->obj_to_raw((s->bucket_info).placement_rule, meta_obj, &raw_obj);
-  store->get_obj_data_pool((s->bucket_info).placement_rule,meta_obj,&meta_pool);
-  store->open_pool_ctx(meta_pool, ioctx);
-
-  const string raw_meta_oid = raw_obj.oid;
-  utime_t time(max_lock_secs_mp, 0);
-  l.set_duration(time);
-  l.lock_exclusive(&op);
-  op_ret = ioctx.operate(raw_meta_oid, &op);
+  store->get_obj_data_pool((s->bucket_info).placement_rule,
+                          meta_obj,&meta_pool);
+  store->open_pool_ctx(meta_pool, serializer.ioctx);
 
+  op_ret = serializer.try_lock(raw_obj.oid, dur);
   if (op_ret < 0) {
     dout(0) << "RGWCompleteMultipart::execute() failed to acquire lock " << dendl;
     op_ret = -ERR_INTERNAL_ERROR;
@@ -5515,13 +5509,41 @@ void RGWCompleteMultipart::execute()
   // remove the upload obj
   int r = store->delete_obj(*static_cast<RGWObjectCtx *>(s->obj_ctx),
                            s->bucket_info, meta_obj, 0);
-  if (r < 0) {
-    ldout(store->ctx(), 0) << "WARNING: failed to remove object " << meta_obj << dendl;
-    r = l.unlock(&ioctx, raw_meta_oid);
+  if (r >= 0)  {
+    /* serializer's exclusive lock is released */
+    serializer.clear_locked();
+  } else {
+      ldout(store->ctx(), 0) << "WARNING: failed to remove object "
+                            << meta_obj << dendl;
+  }
+}
+
+int RGWCompleteMultipart::MPSerializer::try_lock(
+  const std::string& _oid,
+  utime_t dur)
+{
+  oid = _oid;
+  op.assert_exists();
+  lock.set_duration(dur);
+  lock.lock_exclusive(&op);
+  int ret = ioctx.operate(oid, &op);
+  if (! ret) {
+    locked = true;
+  }
+  return ret;
+}
+
+void RGWCompleteMultipart::complete()
+{
+  /* release exclusive lock iff not already */
+  if (unlikely(serializer.locked)) {
+    int r = serializer.unlock();
     if (r < 0) {
-      ldout(store->ctx(), 0) << "WARNING: failed to unlock " << raw_meta_oid << dendl;
+      ldout(store->ctx(), 0) << "WARNING: failed to unlock "
+                            << serializer.oid << dendl;
     }
   }
+  send_response();
 }
 
 int RGWAbortMultipart::verify_permission()

diff --git a/src/rgw/rgw_op.h b/src/rgw/rgw_op.h
index f9e32b76818bf4ca1e3122225ffc7732d3f2be8c..a48083d354ab21c4f5f4df41a4c050b679167e2f 100644 (file)
--- a/src/rgw/rgw_op.h
+++ b/src/rgw/rgw_op.h
@@ -42,6 +42,8 @@
 #include "rgw_lc.h"
 #include "rgw_torrent.h"
 #include "rgw_tag.h"
+#include "cls/lock/cls_lock_client.h"
+#include "cls/rgw/cls_rgw_client.h"
 
 #include "include/assert.h"
 
@@ -1594,6 +1596,27 @@ protected:
   char *data;
   int len;
 
+  struct MPSerializer {
+    librados::IoCtx ioctx;
+    rados::cls::lock::Lock lock;
+    librados::ObjectWriteOperation op;
+    std::string oid;
+    bool locked;
+
+    MPSerializer() : lock("RGWCompleteMultipart"), locked(false)
+      {}
+
+    int try_lock(const std::string& oid, utime_t dur);
+
+    int unlock() {
+      return lock.unlock(&ioctx, oid);
+    }
+
+    void clear_locked() {
+      locked = false;
+    }
+  } serializer;
+
 public:
   RGWCompleteMultipart() {
     data = NULL;
@@ -1606,6 +1629,7 @@ public:
   int verify_permission() override;
   void pre_exec() override;
   void execute() override;
+  void complete() override;
 
   virtual int get_params() = 0;
   void send_response() override = 0;