Merge pull request ceph#55162 from cfsnyder/wip-64014-cfsnyder-pacific

rgw: fix issue with concurrent versioned deletes leaving behind olh entries

Reviewed-by: J. Eric Ivancich <[email protected]>

Author: cbodley

qa/workunits/rgw/test_rgw_versioning.py

@@ -5,6 +5,7 @@
 import uuid
 import botocore
 import time
+import threading
 from common import exec_cmd, create_user, boto_connect
 from botocore.config import Config
 
@@ -100,7 +101,33 @@ def main():
         exec_cmd('ceph config rm client rgw_debug_inject_set_olh_err')
     get_resp = bucket.Object(key).get()
     assert put_resp.e_tag == get_resp['ETag'], 'get did not return null version with correct etag'
-        
+
+    # TESTCASE 'verify that concurrent delete requests do not leave behind olh entries'
+    log.debug('TEST: verify that concurrent delete requests do not leave behind olh entries\n')
+    bucket.object_versions.all().delete()
+    
+    key = 'concurrent-delete'
+    # create a delete marker
+    resp = bucket.Object(key).delete()
+    version_id = resp['ResponseMetadata']['HTTPHeaders']['x-amz-version-id']
+    try:
+        exec_cmd('ceph config set client rgw_debug_inject_latency_bi_unlink 2')
+        time.sleep(1)
+
+        def do_delete():
+            connection.ObjectVersion(bucket.name, key, version_id).delete()
+            
+        t2 = threading.Thread(target=do_delete)
+        t2.start()
+        do_delete()
+        t2.join()
+    finally:
+        exec_cmd('ceph config rm client rgw_debug_inject_latency_bi_unlink')
+    out = exec_cmd(f'radosgw-admin bucket check olh --bucket {bucket.name} --dump-keys')
+    num_leftover_olh_entries = len(json.loads(out))
+    assert num_leftover_olh_entries == 0, \
+      'Found leftover olh entries after concurrent deletes'
+
     # Clean up
     log.debug("Deleting bucket {}".format(BUCKET_NAME))
     bucket.object_versions.all().delete()

src/cls/rgw/cls_rgw.cc

@@ -1906,11 +1906,10 @@ static int rgw_bucket_unlink_instance(cls_method_context_t hctx, bufferlist *in,
   BIOLHEntry olh(hctx, dest_key);
 
   int ret = obj.init();
-  if (ret == -ENOENT) {
-    return 0; /* already removed */
-  }
   if (ret < 0) {
-    CLS_LOG(0, "ERROR: obj.init() returned ret=%d", ret);
+    if (ret != -ENOENT) {
+      CLS_LOG(0, "ERROR: obj.init() returned ret=%d", ret);
+    }
     return ret;
   }
 

src/common/options/rgw.yaml.in

@@ -2642,6 +2642,16 @@ options:
   - rgw
   - rgw
   min: 30
+- name: rgw_debug_inject_latency_bi_unlink
+  type: uint
+  level: dev
+  desc: Latency (in seconds) injected before rgw bucket index unlink op calls to simulate
+    queueing latency and validate behavior of simultaneuous delete requests which
+    target the same object.
+  default: 0
+  with_legacy: true
+  services:
+  - rgw
 - name: rgw_debug_inject_set_olh_err
   type: uint
   level: dev

src/rgw/driver/rados/rgw_rados.cc

@@ -8387,6 +8387,12 @@ int RGWRados::unlink_obj_instance(const DoutPrefixProvider *dpp, RGWObjectCtx& o
     }
 
     string olh_tag(state->olh_tag.c_str(), state->olh_tag.length());
+    
+    if (cct->_conf->rgw_debug_inject_latency_bi_unlink) {
+      // simulates queue latency for unlink ops to validate behavior with
+      // concurrent delete requests for the same object version instance
+      std::this_thread::sleep_for(cct->_conf->rgw_debug_inject_latency_bi_unlink * std::chrono::seconds{1});
+    }
 
     ret = bucket_index_unlink_instance(dpp, bucket_info, target_obj, op_tag, olh_tag, olh_epoch, y, zones_trace, log_op);
     if (ret < 0) {