rgw/logging: allow committing empty objects

Fixes: https://tracker.ceph.com/issues/72542
Signed-off-by: Yuval Lifshitz <ylifshit@ibm.com>

diff --git a/doc/radosgw/s3/bucketops.rst b/doc/radosgw/s3/bucketops.rst
index 0ae9c60dcc37001530583d755fa031a076a780e8..21f834afdf5dd3e8d487f681ef9f51d17dec97a0 100644 (file)
--- a/doc/radosgw/s3/bucketops.rst
+++ b/doc/radosgw/s3/bucketops.rst
@@ -919,7 +919,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 fe8094316fba0de2ecb02fed8dcd17e847eb12d1..fcc8f631e0edfe84ce123b8a3d7f63caf510426d 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 377665ef5b344849fcce213fa3fd904cbe4d5462..c7fc57d50f46adbf18c32e172e1c7456366480bb 100644 (file)
--- a/src/rgw/radosgw-admin/radosgw-admin.cc
+++ b/src/rgw/radosgw-admin/radosgw-admin.cc
@@ -7812,22 +7812,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 b681924aa56764dda067552521a2134fce4bc67e..c511e044facd77b4806a81681d078f5e11bfe9a1 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 89f785582802a6d64cec28c5a983bd338b93255b..28294119456db51b773ba38f33fb2f9a1a59b2c2 100644 (file)
--- a/src/rgw/rgw_rest_bucket_logging.cc
+++ b/src/rgw/rgw_rest_bucket_logging.cc
@@ -384,23 +384,22 @@ class RGWPostBucketLoggingOp : public RGWDefaultResponseOp {
     std::string obj_name;
     RGWObjVersionTracker objv_tracker;
     op_ret = target_bucket->get_logging_object_name(obj_name, configuration.target_prefix, y, 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;
+    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, y, 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