From: Adam C. Emerson Date: Fri, 16 Jul 2021 15:20:39 +0000 (-0400) Subject: rgw: radosgw-admin errors if marker not specified on data/mdlog trim X-Git-Tag: v16.2.6~100^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=9671dac475346e509f242ba979028ef23a687ca0;p=ceph.git 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 (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 a57e2cacd7be..9f807dbfd84e 100644 --- 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 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 f962c76c2a71..140ef54cb3e3 100644 --- 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& entries, - std::optional 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& entries, - std::optional 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 1ff714873367..f609a02bf365 100644 --- 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 marker; + std::string marker; RGWDataChangesLogMarker() = default; }; @@ -148,7 +148,7 @@ public: } int list(const DoutPrefixProvider *dpp, int shard, int max_entries, std::vector& entries, - std::optional 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& entries, - std::optional 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 29b315b21642..599ea4bd1030 100644 --- 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 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 -cursorgeno(std::optional cursor) { - if (cursor && !cursor->empty()) { - return cursorgen(*cursor); - } else { - return { 0, ""s }; - } -} - class LazyFIFO { librados::IoCtx& ioctx; std::string oid;