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

Conflicts:
	src/rgw/rgw_admin.cc

Cherry-pick notes:
- static_cast for RadosStore not needed in Pacific

diff --git a/src/rgw/rgw_admin.cc b/src/rgw/rgw_admin.cc
index a57e2cacd7bed8ea932f2bf70413e3e89c5d7a6c..9f807dbfd84e1980a5a5d7fd6bac999185c20685 100644 (file)
--- a/src/rgw/rgw_admin.cc
+++ b/src/rgw/rgw_admin.cc
@@ -7781,6 +7781,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;
@@ -8770,10 +8775,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,
@@ -8865,11 +8867,13 @@ next:
       return EINVAL;
     }
 
-    // loop until -ENODATA
-    do {
-      auto datalog = 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 = 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 f962c76c2a71ad4113e871f7ebb09e5cea401c8a..140ef54cb3e3d3b4b0f686d1b5d84b0bbb48dacf 100644 (file)
--- a/src/rgw/rgw_datalog.cc
+++ b/src/rgw/rgw_datalog.cc
@@ -704,10 +704,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) {
@@ -744,7 +745,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);
@@ -758,7 +759,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 1ff714873367eb2e1359a11d49f2500e5dcbaa17..f609a02bf365c2d88fc7f2df6284516c357f997c 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;