Merge pull request ceph#62604 from jzhu116-bloomberg/wip-70746

rgw: keep the tails when copying object to itself

Reviewed-by: Casey Bodley <[email protected]>

Author: cbodley

qa/workunits/rgw/test_rgw_orphan_list.sh

@@ -460,6 +460,37 @@ done
 mys3cmd rb --recursive s3://$d_bkt
 mys3cmd rb --recursive s3://$o_bkt
 
+############################################################
+# copy a multipart object to itself
+# run gc process after the copy, then try to get the object
+
+o_bkt="orig-mp-bkt-8"
+
+mys3cmd mb s3://$o_bkt
+
+mp_obj="multipart-obj-1"
+mys3cmd put -q $huge_obj s3://${o_bkt}/$mp_obj
+
+mys3cmd cp --mime-type="text/plain" s3://${o_bkt}/$mp_obj s3://${o_bkt}/$mp_obj
+
+#####################################################################
+# FORCE GARBAGE COLLECTION
+sleep 6 # since for testing age at which gc can happen is 5 secs
+radosgw-admin gc process --include-all
+#####################################################################
+
+mp_obj_retrieved="multipart-obj-1-retrieved"
+mys3cmd get s3://${o_bkt}/$mp_obj $mp_obj_retrieved --force
+
+if [ -f $mp_obj_retrieved ]; then
+    rm -f $mp_obj_retrieved
+else
+    echo "ERROR: failed to get the object after copying to itself"
+    exit 1
+fi
+
+mys3cmd rb --recursive s3://$o_bkt
+
 #####################################################################
 # FORCE GARBAGE COLLECTION
 sleep 6 # since for testing age at which gc can happen is 5 secs

src/rgw/driver/dbstore/common/dbstore.h

@@ -517,7 +517,7 @@ class DBOp {
        *
        * - RGWObjState. Below are omitted from that struct
        *    as they seem in-memory variables
-       *    * is_atomic, has_atts, exists, prefetch_data, keep_tail, 
+       *    * is_atomic, has_atts, exists, prefetch_data, 
        * - RGWObjManifest
        *
        * Extra field added "IsMultipart" to flag multipart uploads,

src/rgw/driver/rados/rgw_putobj_processor.cc

@@ -717,7 +717,7 @@ int AppendObjectProcessor::prepare(optional_yield y)
       tail_placement_rule.storage_class = RGW_STORAGE_CLASS_STANDARD;
     }
     manifest.set_prefix(cur_manifest->get_prefix());
-    astate->keep_tail = true;
+    keep_tail = true;
   }
   manifest.set_multipart_part_rule(store->ctx()->_conf->rgw_obj_stripe_size, cur_part_num);
 
@@ -789,6 +789,7 @@ int AppendObjectProcessor::complete(
   obj_op.meta.user_data = user_data;
   obj_op.meta.zones_trace = zones_trace;
   obj_op.meta.modify_tail = true;
+  obj_op.meta.keep_tail = keep_tail;
   obj_op.meta.appendable = true;
   //Add the append part number
   bufferlist cur_part_num_bl;

src/rgw/driver/rados/rgw_putobj_processor.h

@@ -259,6 +259,7 @@ class MultipartObjectProcessor : public ManifestObjectProcessor {
     uint64_t *cur_accounted_size;
     std::string cur_etag;
     const std::string unique_tag;
+    bool keep_tail;
 
     RGWObjManifest *cur_manifest;
 
@@ -276,7 +277,7 @@ class MultipartObjectProcessor : public ManifestObjectProcessor {
             : ManifestObjectProcessor(aio, store, bucket_info, ptail_placement_rule,
                                       owner, obj_ctx, _head_obj, dpp, y, trace),
               position(position), cur_size(0), cur_accounted_size(cur_accounted_size),
-              unique_tag(unique_tag), cur_manifest(nullptr)
+              unique_tag(unique_tag), keep_tail(false), cur_manifest(nullptr)
     {}
     int prepare(optional_yield y) override;
     int complete(size_t accounted_size, const std::string& etag,

src/rgw/driver/rados/rgw_rados.cc

@@ -3127,8 +3127,8 @@ int RGWRados::swift_versioning_restore(RGWObjectCtx& obj_ctx,
 
 int RGWRados::Object::Write::_do_write_meta(uint64_t size, uint64_t accounted_size,
                                            map<string, bufferlist>& attrs,
-                                           bool assume_noent, bool modify_tail,
-                                           void *_index_op, const req_context& rctx,
+                                           bool assume_noent, void *_index_op,
+                                           const req_context& rctx,
                                            jspan_context& trace, bool log_op)
 {
   RGWRados::Bucket::UpdateIndex *index_op = static_cast<RGWRados::Bucket::UpdateIndex *>(_index_op);
@@ -3172,7 +3172,7 @@ int RGWRados::Object::Write::_do_write_meta(uint64_t size, uint64_t accounted_si
   if (!ptag && !index_op->get_optag()->empty()) {
     ptag = index_op->get_optag();
   }
-  r = target->prepare_atomic_modification(rctx.dpp, op, reset_obj, ptag, meta.if_match, meta.if_nomatch, false, modify_tail, rctx.y);
+  r = target->prepare_atomic_modification(rctx.dpp, op, reset_obj, ptag, meta.if_match, meta.if_nomatch, false, meta.modify_tail, rctx.y);
   if (r < 0)
     return r;
 
@@ -3339,7 +3339,7 @@ int RGWRados::Object::Write::_do_write_meta(uint64_t size, uint64_t accounted_si
 
   poolid = ioctx.get_id();
 
-  r = target->complete_atomic_modification(rctx.dpp, rctx.y);
+  r = target->complete_atomic_modification(rctx.dpp, meta.keep_tail, rctx.y);
   if (r < 0) {
     ldpp_dout(rctx.dpp, 0) << "ERROR: complete_atomic_modification returned r=" << r << dendl;
   }
@@ -3457,13 +3457,13 @@ int RGWRados::Object::Write::write_meta(uint64_t size, uint64_t accounted_size,
   bool assume_noent = (meta.if_match == NULL && meta.if_nomatch == NULL);
   int r;
   if (assume_noent) {
-    r = _do_write_meta(size, accounted_size, attrs, assume_noent, meta.modify_tail, (void *)&index_op, rctx, trace, log_op);
+    r = _do_write_meta(size, accounted_size, attrs, assume_noent, (void *)&index_op, rctx, trace, log_op);
     if (r == -EEXIST) {
       assume_noent = false;
     }
   }
   if (!assume_noent) {
-    r = _do_write_meta(size, accounted_size, attrs, assume_noent, meta.modify_tail, (void *)&index_op, rctx, trace, log_op);
+    r = _do_write_meta(size, accounted_size, attrs, assume_noent, (void *)&index_op, rctx, trace, log_op);
   }
   return r;
 }
@@ -5011,8 +5011,6 @@ int RGWRados::copy_obj(RGWObjectCtx& src_obj_ctx,
     pmanifest = &manifest;
   } else {
     pmanifest = amanifest;
-    /* don't send the object's tail for garbage collection */
-    astate->keep_tail = true;
   }
 
   if (copy_first) {
@@ -5037,6 +5035,7 @@ int RGWRados::copy_obj(RGWObjectCtx& src_obj_ctx,
   write_op.meta.olh_epoch = olh_epoch;
   write_op.meta.delete_at = delete_at;
   write_op.meta.modify_tail = !copy_itself;
+  write_op.meta.keep_tail = copy_itself;
 
   ret = write_op.write_meta(obj_size, astate->accounted_size, attrs, rctx, trace);
   if (ret < 0) {
@@ -5659,9 +5658,9 @@ int RGWRados::bucket_suspended(const DoutPrefixProvider *dpp, rgw_bucket& bucket
   return 0;
 }
 
-int RGWRados::Object::complete_atomic_modification(const DoutPrefixProvider *dpp, optional_yield y)
+int RGWRados::Object::complete_atomic_modification(const DoutPrefixProvider *dpp, bool keep_tail, optional_yield y)
 {
-  if ((!manifest)|| state->keep_tail)
+  if ((!manifest) || keep_tail)
     return 0;
 
   cls_rgw_obj_chain chain;
@@ -6238,7 +6237,7 @@ int RGWRados::Object::Delete::delete_obj(optional_yield y, const DoutPrefixProvi
     }
     r = index_op.complete_del(dpp, poolid, epoch, state->mtime, params.remove_objs, y, log_op);
 
-    int ret = target->complete_atomic_modification(dpp, y);
+    int ret = target->complete_atomic_modification(dpp, false, y);
     if (ret < 0) {
       ldpp_dout(dpp, 0) << "ERROR: complete_atomic_modification returned ret=" << ret << dendl;
     }

src/rgw/driver/rados/rgw_rados.h

@@ -695,7 +695,7 @@ class RGWRados
 
     int prepare_atomic_modification(const DoutPrefixProvider *dpp, librados::ObjectWriteOperation& op, bool reset_obj, const std::string *ptag,
                                     const char *ifmatch, const char *ifnomatch, bool removal_op, bool modify_tail, optional_yield y);
-    int complete_atomic_modification(const DoutPrefixProvider *dpp, optional_yield y);
+    int complete_atomic_modification(const DoutPrefixProvider *dpp, bool keep_tail, optional_yield y);
 
   public:
     Object(RGWRados *_store, const RGWBucketInfo& _bucket_info, RGWObjectCtx& _ctx, const rgw_obj& _obj) : store(_store), bucket_info(_bucket_info),
@@ -813,22 +813,22 @@ class RGWRados
         const std::string *user_data;
         rgw_zone_set *zones_trace;
         bool modify_tail;
+        bool keep_tail;
         bool completeMultipart;
         bool appendable;
 
         MetaParams() : mtime(NULL), rmattrs(NULL), data(NULL), manifest(NULL), ptag(NULL),
                  remove_objs(NULL), category(RGWObjCategory::Main), flags(0),
                  if_match(NULL), if_nomatch(NULL), canceled(false), user_data(nullptr), zones_trace(nullptr),
-                 modify_tail(false),  completeMultipart(false), appendable(false) {}
+                 modify_tail(false), keep_tail(false), completeMultipart(false), appendable(false) {}
       } meta;
 
       explicit Write(RGWRados::Object *_target) : target(_target) {}
 
       int _do_write_meta(uint64_t size, uint64_t accounted_size,
                      std::map<std::string, bufferlist>& attrs,
-                     bool modify_tail, bool assume_noent,
-                     void *index_op, const req_context& rctx,
-                     jspan_context& trace,
+                     bool assume_noent, void *index_op,
+                     const req_context& rctx, jspan_context& trace,
                      bool log_op = true);
       int write_meta(uint64_t size, uint64_t accounted_size,
                      std::map<std::string, bufferlist>& attrs,

src/rgw/rgw_sal_store.h

@@ -37,7 +37,6 @@ struct RGWObjState {
   bool has_data{false};
   bufferlist data;
   bool prefetch_data{false};
-  bool keep_tail{false};
   bool is_olh{false};
   bufferlist olh_tag;
   uint64_t pg_ver{false};
@@ -73,7 +72,6 @@ struct RGWObjState {
       data = rhs.data;
     }
     prefetch_data = rhs.prefetch_data;
-    keep_tail = rhs.keep_tail;
     is_olh = rhs.is_olh;
     objv_tracker = rhs.objv_tracker;
     pg_ver = rhs.pg_ver;