rgw: radosgw-admin errors if marker not specified on data/mdlog trim

Check that a marker was specified and trim if we don't have one.

Also: In a world where we're parsing for generation, it doesn't really
make sense to have a 'no marker specified' as separate from a marker
that is just an empty string.

Also: Successful datalog trim returns zero, not
-ENODATA, and radosgw-admin should expect this.

Fixes: https://tracker.ceph.com/issues/51712
Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

diff --git a/src/rgw/rgw_admin.cc b/src/rgw/rgw_admin.cc
index c4dfd771fe71f6036b7deae024f6ff802b07b277..3f0cae87907d30fafdc9446b7ede68e19c257d37 100644 (file)
--- a/src/rgw/rgw_admin.cc
+++ b/src/rgw/rgw_admin.cc
@@ -7737,6 +7737,11 @@ next:
       return EINVAL;
     }
 
+    if (marker.empty()) {
+      cerr << "ERROR: marker must be specified for trim operation" << std::endl;
+      return EINVAL;
+    }
+
     if (period_id.empty()) {
       std::cerr << "missing --period argument" << std::endl;
       return EINVAL;
@@ -8714,10 +8719,7 @@ next:
       std::vector<rgw_data_change_log_entry> entries;
       if (specified_shard_id) {
         ret = datalog_svc->list_entries(dpp(), shard_id, max_entries - count,
-                                       entries,
-                                       marker.empty() ?
-                                       std::nullopt :
-                                       std::make_optional(marker),
+                                       entries, marker,
                                        &marker, &truncated);
       } else {
         ret = datalog_svc->list_entries(dpp(), max_entries - count, entries,
@@ -8809,11 +8811,13 @@ next:
       return EINVAL;
     }
 
-    // loop until -ENODATA
-    do {
-      auto datalog = static_cast<rgw::sal::RadosStore*>(store)->svc()->datalog_rados;
-      ret = datalog->trim_entries(dpp(), shard_id, marker);
-    } while (ret == 0);
+    if (marker.empty()) {
+      cerr << "ERROR: requires a --marker" << std::endl;
+      return EINVAL;
+    }
+
+    auto datalog = static_cast<rgw::sal::RadosStore*>(store)->svc()->datalog_rados;
+    ret = datalog->trim_entries(dpp(), shard_id, marker);
 
     if (ret < 0 && ret != -ENODATA) {
       cerr << "ERROR: trim_entries(): " << cpp_strerror(-ret) << std::endl;

diff --git a/src/rgw/rgw_datalog.cc b/src/rgw/rgw_datalog.cc
index ddc85802b30e0a02a4ac5df2142194a150da85ac..ce2f4134bab10afe4e5617c75f068888cef416b5 100644 (file)
--- a/src/rgw/rgw_datalog.cc
+++ b/src/rgw/rgw_datalog.cc
@@ -703,10 +703,11 @@ int RGWDataChangesLog::add_entry(const DoutPrefixProvider *dpp, const RGWBucketI
 
 int DataLogBackends::list(const DoutPrefixProvider *dpp, int shard, int max_entries,
                          std::vector<rgw_data_change_log_entry>& entries,
-                         std::optional<std::string_view> marker,
-                         std::string* out_marker, bool* truncated)
+                         std::string_view marker,
+                         std::string* out_marker,
+                         bool* truncated)
 {
-  const auto [start_id, start_cursor] = cursorgeno(marker);
+  const auto [start_id, start_cursor] = cursorgen(marker);
   auto gen_id = start_id;
   std::string out_cursor;
   while (max_entries > 0) {
@@ -743,7 +744,7 @@ int DataLogBackends::list(const DoutPrefixProvider *dpp, int shard, int max_entr
 
 int RGWDataChangesLog::list_entries(const DoutPrefixProvider *dpp, int shard, int max_entries,
                                    std::vector<rgw_data_change_log_entry>& entries,
-                                   std::optional<std::string_view> marker,
+                                   std::string_view marker,
                                    std::string* out_marker, bool* truncated)
 {
   assert(shard < num_shards);
@@ -757,7 +758,7 @@ int RGWDataChangesLog::list_entries(const DoutPrefixProvider *dpp, int max_entri
   bool truncated;
   entries.clear();
   for (; marker.shard < num_shards && int(entries.size()) < max_entries;
-       marker.shard++, marker.marker.reset()) {
+       marker.shard++, marker.marker.clear()) {
     int ret = list_entries(dpp, marker.shard, max_entries - entries.size(),
                           entries, marker.marker, NULL, &truncated);
     if (ret == -ENOENT) {

diff --git a/src/rgw/rgw_datalog.h b/src/rgw/rgw_datalog.h
index 79872bb1571697b99fc235e84e7f861202083b2c..cdf4102f16c710f1e21c58bd7661f6af44febac6 100644 (file)
--- a/src/rgw/rgw_datalog.h
+++ b/src/rgw/rgw_datalog.h
@@ -113,7 +113,7 @@ struct RGWDataChangesLogInfo {
 
 struct RGWDataChangesLogMarker {
   int shard = 0;
-  std::optional<std::string> marker;
+  std::string marker;
 
   RGWDataChangesLogMarker() = default;
 };
@@ -148,7 +148,7 @@ public:
   }
   int list(const DoutPrefixProvider *dpp, int shard, int max_entries,
           std::vector<rgw_data_change_log_entry>& entries,
-          std::optional<std::string_view> marker,
+          std::string_view marker,
           std::string* out_marker, bool* truncated);
   int trim_entries(const DoutPrefixProvider *dpp, int shard_id, std::string_view marker);
   void trim_entries(const DoutPrefixProvider *dpp, int shard_id, std::string_view marker,
@@ -232,7 +232,7 @@ public:
   int get_log_shard_id(rgw_bucket& bucket, int shard_id);
   int list_entries(const DoutPrefixProvider *dpp, int shard, int max_entries,
                   std::vector<rgw_data_change_log_entry>& entries,
-                  std::optional<std::string_view> marker,
+                  std::string_view marker,
                   std::string* out_marker, bool* truncated);
   int trim_entries(const DoutPrefixProvider *dpp, int shard_id, std::string_view marker);
   int trim_entries(const DoutPrefixProvider *dpp, int shard_id, std::string_view marker,

diff --git a/src/rgw/rgw_log_backing.h b/src/rgw/rgw_log_backing.h
index 29b315b21642b9ca87b2a6d386974f8a3a550e22..599ea4bd1030b4d34e38ee75aa5df6fc86169378 100644 (file)
--- a/src/rgw/rgw_log_backing.h
+++ b/src/rgw/rgw_log_backing.h
@@ -241,6 +241,9 @@ inline std::string gencursor(uint64_t gen_id, std::string_view cursor) {
 
 inline std::pair<uint64_t, std::string_view>
 cursorgen(std::string_view cursor_) {
+  if (cursor_.empty()) {
+    return { 0, ""sv };
+  }
   std::string_view cursor = cursor_;
   if (cursor[0] != 'G') {
     return { 0, cursor };
@@ -254,15 +257,6 @@ cursorgen(std::string_view cursor_) {
   return { *gen_id, cursor };
 }
 
-inline std::pair<uint64_t, std::string_view>
-cursorgeno(std::optional<std::string_view> cursor) {
-  if (cursor && !cursor->empty()) {
-    return cursorgen(*cursor);
-  } else {
-    return { 0, ""s };
-  }
-}
-
 class LazyFIFO {
   librados::IoCtx& ioctx;
   std::string oid;