rgw/logging: allow committing empty objects

Fixes: https://tracker.ceph.com/issues/72542
Resolves: rhbz#2394062

Signed-off-by: Yuval Lifshitz <ylifshit@ibm.com>
(cherry picked from commit 62fed9946937cbdda4b6e100a50fc05e9d94ab47)

Conflicts:
	src/rgw/rgw_rest_bucket_logging.cc

diff --git a/doc/radosgw/s3/bucketops.rst b/doc/radosgw/s3/bucketops.rst
index da0fc29c1919cfbd90dd3ad89c0f51a507e9db83..b9a3973e841de2353aed15b8d840eb33daa0fed5 100644 (file)
--- a/doc/radosgw/s3/bucketops.rst
+++ b/doc/radosgw/s3/bucketops.rst
@@ -917,7 +917,7 @@ Flush Bucket Logging
 --------------------
 
 Flushes logging object for a given source bucket (if not flushed, the logging objects are written lazily to the log bucket).
-Returns the name of the object that was flushed. An empty name will be returned if no object needs to be flushed.
+Returns the name of the object that was flushed. Flushing will happen even if the logging object is empty.
 
 Syntax
 ~~~~~~

diff --git a/src/rgw/driver/rados/rgw_sal_rados.cc b/src/rgw/driver/rados/rgw_sal_rados.cc
index ebffb1f014e652b7786c1cee5a368c2795089c62..3153b3007548c7a72b7ec291b46b03b67efd1792 100644 (file)
--- a/src/rgw/driver/rados/rgw_sal_rados.cc
+++ b/src/rgw/driver/rados/rgw_sal_rados.cc
@@ -1138,7 +1138,7 @@ int RadosBucket::get_logging_object_name(std::string& obj_name,
     return -EIO;
   }
   bufferlist bl;
-  const int ret = rgw_get_system_obj(store->svc()->sysobj,
+  if (const int ret = rgw_get_system_obj(store->svc()->sysobj,
                                data_pool,
                                obj_name_oid,
                                bl,
@@ -1147,13 +1147,12 @@ int RadosBucket::get_logging_object_name(std::string& obj_name,
                                y,
                                dpp,
                                nullptr,
-                               nullptr);
-  if (ret < 0) {
-    if (ret == -ENOENT) {
-      ldpp_dout(dpp, 20) << "INFO: logging object name '" << obj_name_oid << "' not found. ret = " << ret << dendl;
-      return ret;
+                               nullptr); ret < 0) {
+    if (ret != -ENOENT) {
+      ldpp_dout(dpp, 1) << "ERROR: failed to get logging object name from '" << obj_name_oid << "'. ret = " << ret << dendl;
+    } else {
+      ldpp_dout(dpp, 20) << "INFO: logging object name does not exist at '" << obj_name_oid << "'" << dendl;
     }
-    ldpp_dout(dpp, 1) << "ERROR: failed to get logging object name from '" << obj_name_oid << "'. ret = " << ret << dendl;
     return ret;
   }
   obj_name = bl.to_str();
@@ -1274,7 +1273,7 @@ int RadosBucket::commit_logging_object(const std::string& obj_name,
   std::map<string, bufferlist> obj_attrs;
   ceph::real_time mtime;
   bufferlist bl_data;
-  if (const auto ret = rgw_get_system_obj(store->svc()->sysobj,
+  if (auto ret = rgw_get_system_obj(store->svc()->sysobj,
                      data_pool,
                      temp_obj_name,
                      bl_data,
@@ -1284,13 +1283,28 @@ int RadosBucket::commit_logging_object(const std::string& obj_name,
                      dpp,
                      &obj_attrs,
                      nullptr); ret < 0) {
-    if (ret == -ENOENT) {
-      ldpp_dout(dpp, 5) << "WARNING: temporary logging object '" << temp_obj_name << "' does not exists" << dendl;
-    } else {
+    if (ret != -ENOENT) {
       ldpp_dout(dpp, 1) << "ERROR: failed to read logging data when committing object '" << temp_obj_name
         << ". error: " << ret << dendl;
+      return ret;
+    }
+    ldpp_dout(dpp, 20) << "INFO: temporary logging object '" << temp_obj_name << "' does not exist. committing it empty" << dendl;
+    // creating an empty object
+    if (ret = rgw_put_system_obj(dpp, store->svc()->sysobj,
+                   data_pool,
+                   temp_obj_name,
+                   bl_data, // empty bufferlist
+                   true, // exclusive
+                   nullptr,
+                   ceph::real_time::clock::now(),
+                   y); ret < 0) {
+      if (ret == -EEXIST) {
+        ldpp_dout(dpp, 5) << "WARNING: race detected in committing an empty logging object '" << temp_obj_name << dendl;
+      } else {
+        ldpp_dout(dpp, 1) << "ERROR: failed to commit empty logging object '" << temp_obj_name << "'. error: " << ret << dendl;
+      }
+      return ret;
     }
-    return ret;
   }
 
   uint64_t size = bl_data.length();

diff --git a/src/rgw/radosgw-admin/radosgw-admin.cc b/src/rgw/radosgw-admin/radosgw-admin.cc
index c7d4ac0edbe884b4cc965feb56e3ef4f2e9cd60c..029c8a48240f89f353e60db8b55f1338e09d01e3 100644 (file)
--- a/src/rgw/radosgw-admin/radosgw-admin.cc
+++ b/src/rgw/radosgw-admin/radosgw-admin.cc
@@ -7865,22 +7865,18 @@ int main(int argc, const char **argv)
     std::string obj_name;
     RGWObjVersionTracker objv_tracker;
     ret = target_bucket->get_logging_object_name(obj_name, configuration.target_prefix, null_yield, dpp(), &objv_tracker);
-    if (ret < 0) {
-      cerr << "ERROR: failed to get pending logging object name from target bucket '" << configuration.target_bucket << "'" << std::endl;
+    if (ret < 0 && ret != -ENOENT) {
+      cerr << "ERROR: failed to get pending logging object name from target bucket '" << configuration.target_bucket <<
+        "'. error: " << cpp_strerror(-ret) << std::endl;
       return -ret;
     }
     std::string old_obj;
     const auto region = driver->get_zone()->get_zonegroup().get_api_name();
     ret = rgw::bucketlogging::rollover_logging_object(configuration, target_bucket, obj_name, dpp(), region, bucket, null_yield, true, &objv_tracker, &old_obj);
     if (ret < 0) {
-      if (ret == -ENOENT) {
-        cerr << "WARNING: no pending logging object '" << obj_name << "'. nothing to flush";
-        ret = 0;
-      } else {
-        cerr << "ERROR: failed flush pending logging object '" << obj_name << "'";
-      }
-      cerr << " to target bucket '" << configuration.target_bucket << "'. "
-        << " last committed object is '" << old_obj << "'" << std::endl;
+      cerr << "ERROR: failed to flush pending logging object '" << obj_name << "' to target bucket '" << configuration.target_bucket << "'. "
+        << " last committed object is '" << old_obj <<
+        "'. error: " << cpp_strerror(-ret) << std::endl;
       return -ret;
     }
     cout << "flushed pending logging object '" << old_obj

diff --git a/src/rgw/rgw_bucket_logging.cc b/src/rgw/rgw_bucket_logging.cc
index 0b82b47fd3f6d9bbfc193082e7906cb4b90cef73..599d23b0e56e3c559353c1fda55165deef342976 100644 (file)
--- a/src/rgw/rgw_bucket_logging.cc
+++ b/src/rgw/rgw_bucket_logging.cc
@@ -368,25 +368,40 @@ int rollover_logging_object(const configuration& conf,
       "', bucket= '" << target_bucket->get_key() << "'" << dendl;
     return -EINVAL;
   }
-  const auto old_obj = obj_name;
-  const int ret = new_logging_object(conf, target_bucket, obj_name, dpp, region, source_bucket, y, old_obj, objv_tracker);
-  if (ret == -ECANCELED) {
-    ldpp_dout(dpp, 20) << "INFO: rollover already performed for object '" << old_obj <<  "' to logging bucket '" <<
-      target_bucket->get_key() << "'. ret = " << ret << dendl;
-    return 0;
-  } else if (ret < 0) {
-    ldpp_dout(dpp, 1) << "ERROR: failed to rollover object '" << old_obj << "' to logging bucket '" <<
-      target_bucket->get_key() << "'. ret = " << ret << dendl;
+
+  auto old_obj = obj_name.empty() ? std::nullopt : std::optional<std::string>(obj_name);
+
+  auto handle_error = [&dpp, &old_obj, &target_bucket](int ret) {
+    if (ret == -ECANCELED) {
+      ldpp_dout(dpp, 20) << "INFO: rollover already performed for logging object '" << old_obj <<  "' to logging bucket '" <<
+        target_bucket->get_key() << "'. ret = " << ret << dendl;
+      return 0;
+    }
+    if (ret < 0) {
+      ldpp_dout(dpp, 1) << "ERROR: failed to rollover logging object '" << old_obj << "' to logging bucket '" <<
+        target_bucket->get_key() << "'. ret = " << ret << dendl;
+    }
+    return ret;
+  };
+
+  if (const int ret = handle_error(new_logging_object(conf, target_bucket, obj_name, dpp, region, source_bucket, y, old_obj, objv_tracker)); ret < 0) {
     return ret;
   }
-  if (const int ret = target_bucket->commit_logging_object(old_obj, y, dpp, conf.target_prefix, last_committed); ret < 0) {
+  if (!old_obj) {
+    // first time logging old == new
+    old_obj = obj_name;
+    if (const int ret = handle_error(new_logging_object(conf, target_bucket, obj_name, dpp, region, source_bucket, y, old_obj, objv_tracker)); ret < 0) {
+      return ret;
+    }
+  }
+  if (const int ret = target_bucket->commit_logging_object(*old_obj, y, dpp, conf.target_prefix, last_committed); ret < 0) {
     if (must_commit) {
       return ret;
     }
     ldpp_dout(dpp, 5) << "WARNING: failed to commit object '" << old_obj << "' to logging bucket '" <<
       target_bucket->get_key() << "'. ret = " << ret << dendl;
     // we still want to write the new records to the new object even if commit failed
-    // will try to commit again next time
+    // TODO: should add commit retry mechanism
   }
   return 0;
 }

diff --git a/src/rgw/rgw_rest_bucket_logging.cc b/src/rgw/rgw_rest_bucket_logging.cc
index e8bb5e643ebf1c00c5bf9b51947bce11d0fa2c39..7d55dfd3fcc80c1701051d32fdbd7c0e9e2884cf 100644 (file)
--- a/src/rgw/rgw_rest_bucket_logging.cc
+++ b/src/rgw/rgw_rest_bucket_logging.cc
@@ -383,24 +383,23 @@ class RGWPostBucketLoggingOp : public RGWDefaultResponseOp {
     const auto& target_bucket_id = target_bucket->get_key();
     std::string obj_name;
     RGWObjVersionTracker objv_tracker;
-    op_ret = target_bucket->get_logging_object_name(obj_name, configuration.target_prefix, null_yield, this, &objv_tracker);
-    if (op_ret < 0) {
-      ldpp_dout(this, 1) << "ERROR: failed to get pending logging object name from target bucket '" << target_bucket_id << "'" << dendl;
+    op_ret = target_bucket->get_logging_object_name(obj_name, configuration.target_prefix, y, this, &objv_tracker);
+    if (op_ret < 0 && op_ret != -ENOENT) {
+      ldpp_dout(this, 1) << "ERROR: failed to get pending logging object name from target bucket '" << target_bucket_id <<
+        "'. error: " << op_ret << dendl;
       return;
     }
+    if (op_ret == -ENOENT) {
+      // no pending object - nothing to flush
+      ldpp_dout(this, 5) << "INFO: no pending logging object in target bucket '" << target_bucket_id << "'. new object should be created" << dendl;
+    }
     const auto region = driver->get_zone()->get_zonegroup().get_api_name();
     op_ret = rgw::bucketlogging::rollover_logging_object(configuration, target_bucket, obj_name, this, region, source_bucket, null_yield, true, &objv_tracker, &old_obj);
     if (op_ret < 0) {
-      if (op_ret == -ENOENT) {
-        ldpp_dout(this, 5) << "WARNING: no pending logging object '" << obj_name << "'. nothing to flush"
-            << " to target bucket '" << target_bucket_id << "'. "
-            << " last committed object is '" << old_obj << "'" << dendl;
-        op_ret = 0;
-      } else {
-        ldpp_dout(this, 1) << "ERROR: failed flush pending logging object '" << obj_name << "'"
-            << " to target bucket '" << target_bucket_id << "'. "
-            << " last committed object is '" << old_obj << "'" << dendl;
-      }
+      ldpp_dout(this, 1) << "ERROR: failed to flush pending logging object '" << obj_name << "'"
+          << " to target bucket '" << target_bucket_id << "'. "
+          << " last committed object is '" << old_obj <<
+          "'. error: " << op_ret << dendl;
       return;
     }
     ldpp_dout(this, 20) << "INFO: flushed pending logging object '" << old_obj