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>
(cherry picked from commit e1ac09ec912ced1c7316c8a18dfad891423be30e)

diff --git a/src/rgw/driver/rados/rgw_rados.cc b/src/rgw/driver/rados/rgw_rados.cc
index 3b2f93b362396fb42609f1842133a1252fe53b4e..0b5818cae51f4959f6e29db55dfb4facb4531e0a 100644 (file)
--- a/src/rgw/driver/rados/rgw_rados.cc
+++ b/src/rgw/driver/rados/rgw_rados.cc
@@ -7440,7 +7440,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);
 
@@ -7559,6 +7572,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 3153b3007548c7a72b7ec291b46b03b67efd1792..4fc4dd06e8ee06e51bdda3ab2e3b6aa20d6e2be9 100644 (file)
--- a/src/rgw/driver/rados/rgw_sal_rados.cc
+++ b/src/rgw/driver/rados/rgw_sal_rados.cc
@@ -2845,8 +2845,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 8a0716eb4b320c3edc9e1f5dac0ea4a0c62d50a4..8cc004dfa345e08490664f31264249dc83b1836b 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