rgw/multisite: reset RGW_ATTR_OBJ_REPLICATION_TRACE during object attr changes.

otherwise, if a zone receives request for any s3 object api requests like PutObjectAcl, PutObjectTagging etc. and this zone
was originally the source zone for the object put request, then such subsequent sync ops will fail. this is because the
zone id was added to the replication trace to ensure that we don't sync the object back to it.
for example in a put/delete race during full sync(https://tracker.ceph.com/issues/58911)
so, if the same zone ever becomes the destination for subsequent sync requests on the same object, we compare this zone as
the destination zone against the zone entries in replication trace and because it's entry is already present in the trace,
the sync operation returns -ERR_NOT_MODIFIED.

Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>

diff --git a/src/rgw/driver/rados/rgw_rados.cc b/src/rgw/driver/rados/rgw_rados.cc
index 3ad3c46e74c159e7c219884f84d0bc90cb0fadbd..94242301624f935a4cbe3b6f0199214faa93f840 100644 (file)
--- a/src/rgw/driver/rados/rgw_rados.cc
+++ b/src/rgw/driver/rados/rgw_rados.cc
@@ -7421,7 +7421,20 @@ int RGWRados::set_attrs(const DoutPrefixProvider *dpp, RGWObjectCtx* octx, RGWBu
   if (!op.size())
     return 0;
 
+  // remove replication-trace attr to be able to re-replicate an object when metadata changes
   bufferlist bl;
+  const string replication_trace = RGW_ATTR_OBJ_REPLICATION_TRACE;
+  bool removed_attr{false};
+  r = state->get_attr(replication_trace, bl);
+  if (r < 0) {
+    ldpp_dout(dpp, 10) << "ERROR: cannot remove attr " << replication_trace.c_str() << dendl;
+  } else {
+    op.rmxattr(replication_trace.c_str());
+    removed_attr = true;
+  }
+
+  bl.clear();
+
   RGWRados::Bucket bop(this, bucket_info);
   RGWRados::Bucket::UpdateIndex index_op(&bop, obj);
 
@@ -7540,6 +7553,10 @@ int RGWRados::set_attrs(const DoutPrefixProvider *dpp, RGWObjectCtx* octx, RGWBu
       }
     }
 
+    if (removed_attr) {
+        state->attrset.erase(replication_trace);
+    }
+
     for (iter = attrs.begin(); iter != attrs.end(); ++iter) {
       state->attrset[iter->first] = iter->second;
     }

diff --git a/src/rgw/driver/rados/rgw_sal_rados.cc b/src/rgw/driver/rados/rgw_sal_rados.cc
index b18434f6460e09b5931f5f558c975eb744387621..983d09326c7eda79e4075334662c6437a5b9cb22 100644 (file)
--- a/src/rgw/driver/rados/rgw_sal_rados.cc
+++ b/src/rgw/driver/rados/rgw_sal_rados.cc
@@ -2848,8 +2848,10 @@ int RadosObject::modify_obj_attrs(const char* attr_name, bufferlist& attr_val, o
   /* Temporarily set target */
   state.obj = target;
   set_atomic(true);
-  state.attrset[attr_name] = attr_val;
-  r = set_obj_attrs(dpp, &state.attrset, nullptr, y, flags);
+
+  Attrs mattr;
+  mattr[attr_name] = attr_val;
+  r = set_obj_attrs(dpp, &mattr, nullptr, y, flags);
   /* Restore target */
   state.obj = save;
 

diff --git a/src/test/rgw/rgw_multi/tests.py b/src/test/rgw/rgw_multi/tests.py
index c457fc3ca9c30e0488c4ad2c89923cd72068a864..b70fc22aa2441025165088a458ae427af23bc1d2 100644 (file)
--- a/src/test/rgw/rgw_multi/tests.py
+++ b/src/test/rgw/rgw_multi/tests.py
@@ -2317,14 +2317,14 @@ def test_object_acl():
     before_set_acl = bucket2.get_acl(k)
     assert(len(before_set_acl.acl.grants) == 1)
 
-    #set object acl on primary and wait for sync.
-    bucket.set_canned_acl('public-read', key_name=k)
-    log.debug('set acl=%s', bucket.name)
+    #set object acl on secondary and wait for sync.
+    bucket2.set_canned_acl('public-read', key_name=k)
+    log.debug('set acl=%s', bucket2.name)
     zonegroup_data_checkpoint(zonegroup_conns)
-    zonegroup_bucket_checkpoint(zonegroup_conns, bucket.name)
+    zonegroup_bucket_checkpoint(zonegroup_conns, bucket2.name)
 
-    #check object secondary after setacl
-    bucket2 = get_bucket(secondary, bucket.name)
+    #check object on primary after setacl
+    bucket2 = get_bucket(primary, bucket.name)
     after_set_acl = bucket2.get_acl(k)
     assert(len(after_set_acl.acl.grants) == 2) # read grant added on AllUsers