From: Casey Bodley Date: Tue, 21 Mar 2023 17:36:56 +0000 (-0400) Subject: rgw/sal: list_buckets() returns RGWBucketEnts X-Git-Tag: v19.0.0~427^2~1 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=50836121a60f7b64dbd41aaceae134588d067701;p=ceph-ci.git rgw/sal: list_buckets() returns RGWBucketEnts `sal::User::list_buckets()` no longer returns a map of `sal::Bucket` handles. it now uses `std::span` for input and output. `RGWBucketEnt` contains all of the information we need to satisfy ListBuckets requests, and also stores the `rgw_bucket` key for use with `Driver::get_bucket()` where a `sal::Bucket` handle is necessary `sal::BucketList` contains the span of results and the `next_marker`. the `is_truncated` flag was removed in favor of `!next_marker.empty()` the checks for `user->get_max_buckets()` on bucket creation now use a paginated `check_user_max_buckets()` helper function that limits the number of allocated entries to `rgw_list_buckets_max_chunk` Signed-off-by: Casey Bodley --- diff --git a/src/rgw/driver/posix/rgw_sal_posix.cc b/src/rgw/driver/posix/rgw_sal_posix.cc index bb7617e8b9b..bd6d8db352b 100644 --- a/src/rgw/driver/posix/rgw_sal_posix.cc +++ b/src/rgw/driver/posix/rgw_sal_posix.cc @@ -510,16 +510,17 @@ int POSIXDriver::close() return 0; } +// TODO: marker and other params int POSIXUser::list_buckets(const DoutPrefixProvider* dpp, const std::string& marker, const std::string& end_marker, uint64_t max, - bool need_stats, BucketList &buckets, optional_yield y) + bool need_stats, BucketList &result, optional_yield y) { DIR* dir; struct dirent* entry; int dfd; int ret; - buckets.clear(); + result.buckets.clear(); /* it's not sufficient to dup(root_fd), as as the new fd would share * the file position of root_fd */ @@ -557,7 +558,6 @@ int POSIXUser::list_buckets(const DoutPrefixProvider* dpp, const std::string& ma ret = errno; ldpp_dout(dpp, 0) << "ERROR: could not stat object " << entry->d_name << ": " << cpp_strerror(ret) << dendl; - buckets.clear(); return -ret; } @@ -572,25 +572,12 @@ int POSIXUser::list_buckets(const DoutPrefixProvider* dpp, const std::string& ma continue; } - /* TODO Use stat_to_ent */ - //RGWBucketEnt ent; - //ent.bucket.name = decode_name(entry->d_name); - //bucket_statx_save(stx, ent, mtime); - RGWBucketInfo info; - info.bucket.name = url_decode(entry->d_name); - info.owner.id = std::to_string(stx.stx_uid); // TODO convert to owner name - info.creation_time = from_statx_timestamp(stx.stx_btime); + RGWBucketEnt ent; + ent.bucket.name = url_decode(entry->d_name); + ent.creation_time = ceph::real_clock::from_time_t(stx.stx_btime.tv_sec); + // TODO: ent.size and ent.count - std::unique_ptr bucket; - ret = driver->get_bucket(this, info, &bucket); - if (ret < 0) { - ldpp_dout(dpp, 0) << "ERROR: could not get bucket " << info.bucket << ": " - << cpp_strerror(ret) << dendl; - buckets.clear(); - return -ret; - } - - buckets.add(std::move(bucket)); + result.buckets.push_back(std::move(ent)); errno = 0; } @@ -598,7 +585,6 @@ int POSIXUser::list_buckets(const DoutPrefixProvider* dpp, const std::string& ma if (ret != 0) { ldpp_dout(dpp, 0) << "ERROR: could not list buckets for " << get_display_name() << ": " << cpp_strerror(ret) << dendl; - buckets.clear(); return -ret; } diff --git a/src/rgw/driver/rados/rgw_bucket.cc b/src/rgw/driver/rados/rgw_bucket.cc index a23e657b62c..7603bf73b84 100644 --- a/src/rgw/driver/rados/rgw_bucket.cc +++ b/src/rgw/driver/rados/rgw_bucket.cc @@ -87,51 +87,39 @@ void check_bad_user_bucket_mapping(rgw::sal::Driver* driver, rgw::sal::User& use optional_yield y, const DoutPrefixProvider *dpp) { - rgw::sal::BucketList user_buckets; - string marker; - - CephContext *cct = driver->ctx(); - - size_t max_entries = cct->_conf->rgw_list_buckets_max_chunk; + size_t max_entries = dpp->get_cct()->_conf->rgw_list_buckets_max_chunk; + rgw::sal::BucketList listing; do { - int ret = user.list_buckets(dpp, marker, string(), max_entries, false, user_buckets, y); + int ret = user.list_buckets(dpp, listing.next_marker, string(), + max_entries, false, listing, y); if (ret < 0) { - ldout(driver->ctx(), 0) << "failed to read user buckets: " - << cpp_strerror(-ret) << dendl; + ldpp_dout(dpp, 0) << "failed to read user buckets: " + << cpp_strerror(-ret) << dendl; return; } - map>& buckets = user_buckets.get_buckets(); - for (auto i = buckets.begin(); - i != buckets.end(); - ++i) { - marker = i->first; - - auto& bucket = i->second; - - std::unique_ptr actual_bucket; - int r = driver->get_bucket(dpp, &user, user.get_tenant(), bucket->get_name(), &actual_bucket, y); + for (const auto& ent : listing.buckets) { + std::unique_ptr bucket; + int r = driver->get_bucket(dpp, &user, user.get_tenant(), ent.bucket.name, &bucket, y); if (r < 0) { - ldout(driver->ctx(), 0) << "could not get bucket info for bucket=" << bucket << dendl; + ldpp_dout(dpp, 0) << "could not get bucket info for bucket=" << bucket << dendl; continue; } - if (actual_bucket->get_name().compare(bucket->get_name()) != 0 || - actual_bucket->get_tenant().compare(bucket->get_tenant()) != 0 || - actual_bucket->get_marker().compare(bucket->get_marker()) != 0 || - actual_bucket->get_bucket_id().compare(bucket->get_bucket_id()) != 0) { - cout << "bucket info mismatch: expected " << actual_bucket << " got " << bucket << std::endl; + if (ent.bucket != bucket->get_key()) { + cout << "bucket info mismatch: expected " << ent.bucket + << " got " << bucket << std::endl; if (fix) { cout << "fixing" << std::endl; - r = actual_bucket->chown(dpp, user, y); + r = bucket->chown(dpp, user, y); if (r < 0) { cerr << "failed to fix bucket: " << cpp_strerror(-r) << std::endl; } } } } - } while (user_buckets.is_truncated()); + } while (!listing.next_marker.empty()); } // returns true if entry is in the empty namespace. note: function @@ -969,32 +957,24 @@ int RGWBucketAdminOp::limit_check(rgw::sal::Driver* driver, formatter->open_array_section("users"); for (const auto& user_id : user_ids) { + std::unique_ptr user = driver->get_user(rgw_user(user_id)); formatter->open_object_section("user"); formatter->dump_string("user_id", user_id); formatter->open_array_section("buckets"); - string marker; - rgw::sal::BucketList buckets; + rgw::sal::BucketList listing; do { - std::unique_ptr user = driver->get_user(rgw_user(user_id)); - - ret = user->list_buckets(dpp, marker, string(), max_entries, false, buckets, y); - + ret = user->list_buckets(dpp, listing.next_marker, string(), + max_entries, false, listing, y); if (ret < 0) return ret; - map>& m_buckets = buckets.get_buckets(); - - for (const auto& iter : m_buckets) { - auto& bucket = iter.second; + for (const auto& ent : listing.buckets) { uint64_t num_objects = 0; - marker = bucket->get_name(); /* Casey's location for marker update, - * as we may now not reach the end of - * the loop body */ - - ret = bucket->load_bucket(dpp, y); + std::unique_ptr bucket; + ret = driver->get_bucket(dpp, user.get(), ent.bucket, &bucket, y); if (ret < 0) continue; @@ -1046,7 +1026,7 @@ int RGWBucketAdminOp::limit_check(rgw::sal::Driver* driver, } } formatter->flush(cout); - } while (buckets.is_truncated()); /* foreach: bucket */ + } while (!listing.next_marker.empty()); /* foreach: bucket */ formatter->close_section(); formatter->close_section(); @@ -1080,58 +1060,42 @@ int RGWBucketAdminOp::info(rgw::sal::Driver* driver, Formatter *formatter = flusher.get_formatter(); flusher.start(0); - CephContext *cct = driver->ctx(); - - const size_t max_entries = cct->_conf->rgw_list_buckets_max_chunk; - const bool show_stats = op_state.will_fetch_stats(); const rgw_user& user_id = op_state.get_user_id(); - if (op_state.is_user_op()) { + if (!bucket_name.empty()) { + ret = bucket_stats(driver, user_id.tenant, bucket_name, formatter, dpp, y); + if (ret < 0) { + return ret; + } + } else if (op_state.is_user_op()) { formatter->open_array_section("buckets"); - rgw::sal::BucketList buckets; std::unique_ptr user = driver->get_user(op_state.get_user_id()); - std::string marker = op_state.marker; const std::string empty_end_marker; + const size_t max_entries = dpp->get_cct()->_conf->rgw_list_buckets_max_chunk; constexpr bool no_need_stats = false; // set need_stats to false + rgw::sal::BucketList listing; + listing.next_marker = op_state.marker; do { - ret = user->list_buckets(dpp, marker, empty_end_marker, max_entries, - no_need_stats, buckets, y); + ret = user->list_buckets(dpp, listing.next_marker, empty_end_marker, + max_entries, no_need_stats, listing, y); if (ret < 0) { return ret; } - const std::string* marker_cursor = nullptr; - map>& m = buckets.get_buckets(); - - for (const auto& i : m) { - const std::string& obj_name = i.first; - if (!bucket_name.empty() && bucket_name != obj_name) { - continue; - } - + for (const auto& ent : listing.buckets) { if (show_stats) { - bucket_stats(driver, user_id.tenant, obj_name, formatter, dpp, y); + bucket_stats(driver, user_id.tenant, ent.bucket.name, formatter, dpp, y); } else { - formatter->dump_string("bucket", obj_name); + formatter->dump_string("bucket", ent.bucket.name); } - - marker_cursor = &obj_name; } // for loop - if (marker_cursor) { - marker = *marker_cursor; - } flusher.flush(); - } while (buckets.is_truncated()); + } while (!listing.next_marker.empty()); formatter->close_section(); - } else if (!bucket_name.empty()) { - ret = bucket_stats(driver, user_id.tenant, bucket_name, formatter, dpp, y); - if (ret < 0) { - return ret; - } } else { void *handle = nullptr; bool truncated = true; diff --git a/src/rgw/driver/rados/rgw_sal_rados.cc b/src/rgw/driver/rados/rgw_sal_rados.cc index 8eafadc905c..465a6e064d4 100644 --- a/src/rgw/driver/rados/rgw_sal_rados.cc +++ b/src/rgw/driver/rados/rgw_sal_rados.cc @@ -140,23 +140,28 @@ static int drain_aio(std::list& handles) int RadosUser::list_buckets(const DoutPrefixProvider* dpp, const std::string& marker, const std::string& end_marker, uint64_t max, bool need_stats, - BucketList &buckets, optional_yield y) + BucketList &result, optional_yield y) { RGWUserBuckets ulist; bool is_truncated = false; - int ret; - buckets.clear(); - ret = store->ctl()->user->list_buckets(dpp, info.user_id, marker, end_marker, max, - need_stats, &ulist, &is_truncated, y); + int ret = store->ctl()->user->list_buckets(dpp, get_id(), marker, end_marker, + max, need_stats, &ulist, + &is_truncated, y); if (ret < 0) return ret; - buckets.set_truncated(is_truncated); - for (const auto& ent : ulist.get_buckets()) { - buckets.add(std::unique_ptr(new RadosBucket(this->store, ent.second, this))); + result.buckets.clear(); + + for (auto& ent : ulist.get_buckets()) { + result.buckets.push_back(std::move(ent.second)); } + if (is_truncated && !result.buckets.empty()) { + result.next_marker = result.buckets.back().bucket.name; + } else { + result.next_marker.clear(); + } return 0; } diff --git a/src/rgw/driver/rados/rgw_user.cc b/src/rgw/driver/rados/rgw_user.cc index b0d11631823..4e70c855222 100644 --- a/src/rgw/driver/rados/rgw_user.cc +++ b/src/rgw/driver/rados/rgw_user.cc @@ -1556,25 +1556,20 @@ int RGWUser::execute_rename(const DoutPrefixProvider *dpp, RGWUserAdminOpState& policy_instance.create_default(new_user->get_id(), old_user->get_display_name()); //unlink and link buckets to new user - string marker; - CephContext *cct = driver->ctx(); - size_t max_buckets = cct->_conf->rgw_list_buckets_max_chunk; - rgw::sal::BucketList buckets; + size_t max_entries = dpp->get_cct()->_conf->rgw_list_buckets_max_chunk; + rgw::sal::BucketList listing; do { - ret = old_user->list_buckets(dpp, marker, "", max_buckets, false, buckets, y); + ret = old_user->list_buckets(dpp, listing.next_marker, "", + max_entries, false, listing, y); if (ret < 0) { set_err_msg(err_msg, "unable to list user buckets"); return ret; } - auto& m = buckets.get_buckets(); - - for (auto it = m.begin(); it != m.end(); ++it) { - auto& bucket = it->second; - marker = it->first; - - ret = bucket->load_bucket(dpp, y); + for (const auto& ent : listing.buckets) { + std::unique_ptr bucket; + ret = driver->get_bucket(dpp, old_user.get(), ent.bucket, &bucket, y); if (ret < 0) { set_err_msg(err_msg, "failed to fetch bucket info for bucket=" + bucket->get_name()); return ret; @@ -1594,7 +1589,7 @@ int RGWUser::execute_rename(const DoutPrefixProvider *dpp, RGWUserAdminOpState& } } - } while (buckets.is_truncated()); + } while (!listing.next_marker.empty()); // update the 'stub user' with all of the other fields and rewrite all of the // associated index objects @@ -1762,34 +1757,37 @@ int RGWUser::execute_remove(const DoutPrefixProvider *dpp, RGWUserAdminOpState& return -ENOENT; } - rgw::sal::BucketList buckets; - string marker; - CephContext *cct = driver->ctx(); - size_t max_buckets = cct->_conf->rgw_list_buckets_max_chunk; + size_t max_buckets = dpp->get_cct()->_conf->rgw_list_buckets_max_chunk; + + rgw::sal::BucketList listing; do { - ret = user->list_buckets(dpp, marker, string(), max_buckets, false, buckets, y); + ret = user->list_buckets(dpp, listing.next_marker, string(), + max_buckets, false, listing, y); if (ret < 0) { - set_err_msg(err_msg, "unable to read user bucket info"); + set_err_msg(err_msg, "unable to list user buckets"); return ret; } - auto& m = buckets.get_buckets(); - if (!m.empty() && !purge_data) { + if (!listing.buckets.empty() && !purge_data) { set_err_msg(err_msg, "must specify purge data to remove user with buckets"); return -EEXIST; // change to code that maps to 409: conflict } - for (auto it = m.begin(); it != m.end(); ++it) { - ret = it->second->remove_bucket(dpp, true, false, nullptr, y); + for (const auto& ent : listing.buckets) { + std::unique_ptr bucket; + ret = driver->get_bucket(dpp, user, ent.bucket, &bucket, y); if (ret < 0) { - set_err_msg(err_msg, "unable to delete user data"); + set_err_msg(err_msg, "unable to load bucket " + ent.bucket.name); return ret; } - marker = it->first; + ret = bucket->remove_bucket(dpp, true, false, nullptr, y); + if (ret < 0) { + set_err_msg(err_msg, "unable to delete user data"); + return ret; + } } - - } while (buckets.is_truncated()); + } while (!listing.next_marker.empty()); ret = user->remove_user(dpp, y); if (ret < 0) { @@ -1908,32 +1906,27 @@ int RGWUser::execute_modify(const DoutPrefixProvider *dpp, RGWUserAdminOpState& __u8 suspended = op_state.get_suspension_status(); user_info.suspended = suspended; - rgw::sal::BucketList buckets; if (user_id.empty()) { set_err_msg(err_msg, "empty user id passed...aborting"); return -EINVAL; } - - string marker; - CephContext *cct = driver->ctx(); - size_t max_buckets = cct->_conf->rgw_list_buckets_max_chunk; std::unique_ptr user = driver->get_user(user_id); + + size_t max_buckets = dpp->get_cct()->_conf->rgw_list_buckets_max_chunk; + + rgw::sal::BucketList listing; do { - ret = user->list_buckets(dpp, marker, string(), max_buckets, false, buckets, y); + ret = user->list_buckets(dpp, listing.next_marker, string(), + max_buckets, false, listing, y); if (ret < 0) { set_err_msg(err_msg, "could not get buckets for uid: " + user_id.to_str()); return ret; } - auto& m = buckets.get_buckets(); - - vector bucket_names; - for (auto iter = m.begin(); iter != m.end(); ++iter) { - auto& bucket = iter->second; - bucket_names.push_back(bucket->get_key()); - - marker = iter->first; + std::vector bucket_names; + for (auto& ent : listing.buckets) { + bucket_names.push_back(std::move(ent.bucket)); } ret = driver->set_buckets_enabled(dpp, bucket_names, !suspended, y); @@ -1942,7 +1935,7 @@ int RGWUser::execute_modify(const DoutPrefixProvider *dpp, RGWUserAdminOpState& return ret; } - } while (buckets.is_truncated()); + } while (!listing.next_marker.empty()); } if (op_state.mfa_ids_specified) { diff --git a/src/rgw/rgw_file_int.h b/src/rgw/rgw_file_int.h index 6f71f754ebe..c695aba46cb 100644 --- a/src/rgw/rgw_file_int.h +++ b/src/rgw/rgw_file_int.h @@ -1405,17 +1405,14 @@ public: sent_data = true; } - void send_response_data(rgw::sal::BucketList& buckets) override { + void send_response_data(std::span buckets) override { if (!sent_data) return; - auto& m = buckets.get_buckets(); - for (const auto& iter : m) { - std::string_view marker{iter.first}; - auto& ent = iter.second; - if (! this->operator()(ent->get_name(), marker)) { + for (const auto& ent : buckets) { + if (! this->operator()(ent.bucket.name, ent.bucket.name)) { /* caller cannot accept more */ lsubdout(cct, rgw, 5) << "ListBuckets rcb failed" - << " dirent=" << ent->get_name() + << " dirent=" << ent.bucket.name << " call count=" << ix << dendl; rcb_eof = true; diff --git a/src/rgw/rgw_op.cc b/src/rgw/rgw_op.cc index 585c37f3ab5..8607f058ac7 100644 --- a/src/rgw/rgw_op.cc +++ b/src/rgw/rgw_op.cc @@ -2489,29 +2489,46 @@ void RGWListBuckets::execute(optional_yield y) const uint64_t max_buckets = s->cct->_conf->rgw_list_buckets_max_chunk; + auto g = make_scope_guard([this, &started] { + if (!started) { + send_response_begin(false); + } + send_response_end(); + }); + op_ret = get_params(y); if (op_ret < 0) { - goto send_end; + return; } if (supports_account_metadata()) { op_ret = s->user->read_attrs(this, s->yield); if (op_ret < 0) { - goto send_end; + return; } } - is_truncated = false; + /* We need to have stats for all our policies - even if a given policy + * isn't actually used in a given account. In such situation its usage + * stats would be simply full of zeros. */ + std::set targets; + driver->get_zone()->get_zonegroup().get_placement_target_names(targets); + for (const auto& policy : targets) { + policies_stats[policy] = {}; + } + + rgw::sal::BucketList listing; do { - rgw::sal::BucketList buckets; uint64_t read_count; - if (limit >= 0) { + if (limit == 0) { + break; + } else if (limit > 0) { read_count = min(limit - total_count, max_buckets); } else { read_count = max_buckets; } - op_ret = s->user->list_buckets(this, marker, end_marker, read_count, should_get_stats(), buckets, y); + op_ret = s->user->list_buckets(this, marker, end_marker, read_count, should_get_stats(), listing, y); if (op_ret < 0) { /* hmm.. something wrong here.. the user was authenticated, so it @@ -2521,57 +2538,33 @@ void RGWListBuckets::execute(optional_yield y) break; } - is_truncated = buckets.is_truncated(); + marker = listing.next_marker; - /* We need to have stats for all our policies - even if a given policy - * isn't actually used in a given account. In such situation its usage - * stats would be simply full of zeros. */ - std::set targets; - driver->get_zone()->get_zonegroup().get_placement_target_names(targets); - for (const auto& policy : targets) { - policies_stats.emplace(policy, decltype(policies_stats)::mapped_type()); - } - - std::map>& m = buckets.get_buckets(); - for (const auto& kv : m) { - const auto& bucket = kv.second; - - global_stats.bytes_used += bucket->get_size(); - global_stats.bytes_used_rounded += bucket->get_size_rounded(); - global_stats.objects_count += bucket->get_count(); + for (const auto& ent : listing.buckets) { + global_stats.bytes_used += ent.size; + global_stats.bytes_used_rounded += ent.size_rounded; + global_stats.objects_count += ent.count; /* operator[] still can create a new entry for storage policy seen * for first time. */ - auto& policy_stats = policies_stats[bucket->get_placement_rule().to_str()]; - policy_stats.bytes_used += bucket->get_size(); - policy_stats.bytes_used_rounded += bucket->get_size_rounded(); + auto& policy_stats = policies_stats[ent.placement_rule.to_str()]; + policy_stats.bytes_used += ent.size; + policy_stats.bytes_used_rounded += ent.size_rounded; policy_stats.buckets_count++; - policy_stats.objects_count += bucket->get_count(); + policy_stats.objects_count += ent.count; } - global_stats.buckets_count += m.size(); - total_count += m.size(); + global_stats.buckets_count += listing.buckets.size(); + total_count += listing.buckets.size(); - done = (m.size() < read_count || (limit >= 0 && total_count >= (uint64_t)limit)); + done = (limit >= 0 && std::cmp_greater_equal(total_count, limit)); if (!started) { - send_response_begin(buckets.count() > 0); + send_response_begin(!listing.buckets.empty()); started = true; } - if (read_count > 0 && - !m.empty()) { - auto riter = m.rbegin(); - marker = riter->first; - - handle_listing_chunk(std::move(buckets)); - } - } while (is_truncated && !done); - -send_end: - if (!started) { - send_response_begin(false); - } - send_response_end(); + handle_listing_chunk(listing.buckets); + } while (!marker.empty() && !done); } void RGWGetUsage::execute(optional_yield y) @@ -2649,58 +2642,44 @@ int RGWStatAccount::verify_permission(optional_yield y) void RGWStatAccount::execute(optional_yield y) { - string marker; - rgw::sal::BucketList buckets; uint64_t max_buckets = s->cct->_conf->rgw_list_buckets_max_chunk; - const string *lastmarker; - do { + /* We need to have stats for all our policies - even if a given policy + * isn't actually used in a given account. In such situation its usage + * stats would be simply full of zeros. */ + std::set names; + driver->get_zone()->get_zonegroup().get_placement_target_names(names); + for (const auto& policy : names) { + policies_stats.emplace(policy, decltype(policies_stats)::mapped_type()); + } - lastmarker = nullptr; - op_ret = s->user->list_buckets(this, marker, string(), max_buckets, true, buckets, y); + rgw::sal::BucketList listing; + do { + op_ret = s->user->list_buckets(this, listing.next_marker, string(), + max_buckets, true, listing, y); if (op_ret < 0) { /* hmm.. something wrong here.. the user was authenticated, so it should exist */ ldpp_dout(this, 10) << "WARNING: failed on list_buckets uid=" << s->user->get_id() << " ret=" << op_ret << dendl; - break; - } else { - /* We need to have stats for all our policies - even if a given policy - * isn't actually used in a given account. In such situation its usage - * stats would be simply full of zeros. */ - std::set names; - driver->get_zone()->get_zonegroup().get_placement_target_names(names); - for (const auto& policy : names) { - policies_stats.emplace(policy, decltype(policies_stats)::mapped_type()); - } + return; + } - std::map>& m = buckets.get_buckets(); - for (const auto& kv : m) { - const auto& bucket = kv.second; - lastmarker = &kv.first; - - global_stats.bytes_used += bucket->get_size(); - global_stats.bytes_used_rounded += bucket->get_size_rounded(); - global_stats.objects_count += bucket->get_count(); - - /* operator[] still can create a new entry for storage policy seen - * for first time. */ - auto& policy_stats = policies_stats[bucket->get_placement_rule().to_str()]; - policy_stats.bytes_used += bucket->get_size(); - policy_stats.bytes_used_rounded += bucket->get_size_rounded(); - policy_stats.buckets_count++; - policy_stats.objects_count += bucket->get_count(); - } - global_stats.buckets_count += m.size(); + for (const auto& ent : listing.buckets) { + global_stats.bytes_used += ent.size; + global_stats.bytes_used_rounded += ent.size_rounded; + global_stats.objects_count += ent.count; + /* operator[] still can create a new entry for storage policy seen + * for first time. */ + auto& policy_stats = policies_stats[ent.placement_rule.to_str()]; + policy_stats.bytes_used += ent.size; + policy_stats.bytes_used_rounded += ent.size_rounded; + policy_stats.buckets_count++; + policy_stats.objects_count += ent.count; } - if (!lastmarker) { - ldpp_dout(this, -1) << "ERROR: rgw_read_user_buckets, stasis at marker=" - << marker << " uid=" << s->user->get_id() << dendl; - break; - } - marker = *lastmarker; - } while (buckets.is_truncated()); + global_stats.buckets_count += listing.buckets.size(); + } while (!listing.next_marker.empty()); } int RGWGetBucketVersioning::verify_permission(optional_yield y) @@ -3090,6 +3069,36 @@ int RGWGetBucketLocation::verify_permission(optional_yield y) return verify_bucket_owner_or_policy(s, rgw::IAM::s3GetBucketLocation); } +// list the user's buckets to check whether they're at their maximum +static int check_user_max_buckets(const DoutPrefixProvider* dpp, + rgw::sal::User& user, optional_yield y) +{ + int32_t remaining = user.get_max_buckets(); + if (!remaining) { // unlimited + return 0; + } + + uint64_t max_buckets = dpp->get_cct()->_conf->rgw_list_buckets_max_chunk; + + rgw::sal::BucketList listing; + do { + size_t to_read = std::max(max_buckets, remaining); + + int ret = user.list_buckets(dpp, listing.next_marker, string(), + to_read, false, listing, y); + if (ret < 0) { + return ret; + } + + remaining -= listing.buckets.size(); + if (remaining <= 0) { + return -ERR_TOO_MANY_BUCKETS; + } + } while (!listing.next_marker.empty()); + + return 0; +} + int RGWCreateBucket::verify_permission(optional_yield y) { /* This check is mostly needed for S3 that doesn't support account ACL. @@ -3122,21 +3131,7 @@ int RGWCreateBucket::verify_permission(optional_yield y) return -EPERM; } - if (s->user->get_max_buckets()) { - rgw::sal::BucketList buckets; - string marker; - op_ret = s->user->list_buckets(this, marker, string(), s->user->get_max_buckets(), - false, buckets, y); - if (op_ret < 0) { - return op_ret; - } - - if ((int)buckets.count() >= s->user->get_max_buckets()) { - return -ERR_TOO_MANY_BUCKETS; - } - } - - return 0; + return check_user_max_buckets(this, *s->user, y); } void RGWCreateBucket::pre_exec() @@ -7478,21 +7473,7 @@ RGWBulkUploadOp::handle_upload_path(req_state *s) int RGWBulkUploadOp::handle_dir_verify_permission(optional_yield y) { - if (s->user->get_max_buckets() > 0) { - rgw::sal::BucketList buckets; - std::string marker; - op_ret = s->user->list_buckets(this, marker, std::string(), s->user->get_max_buckets(), - false, buckets, y); - if (op_ret < 0) { - return op_ret; - } - - if (buckets.count() >= static_cast(s->user->get_max_buckets())) { - return -ERR_TOO_MANY_BUCKETS; - } - } - - return 0; + return check_user_max_buckets(this, *s->user, y); } static void forward_req_info(const DoutPrefixProvider *dpp, CephContext *cct, req_info& info, const std::string& bucket_name) diff --git a/src/rgw/rgw_op.h b/src/rgw/rgw_op.h index 0dcdd456d7d..b5ce737147f 100644 --- a/src/rgw/rgw_op.h +++ b/src/rgw/rgw_op.h @@ -16,6 +16,7 @@ #include #include +#include #include #include #include @@ -835,7 +836,7 @@ public: void execute(optional_yield y) override; virtual int get_params(optional_yield y) = 0; - virtual void handle_listing_chunk(rgw::sal::BucketList&& buckets) { + virtual void handle_listing_chunk(std::span buckets) { /* The default implementation, used by e.g. S3, just generates a new * part of listing and sends it client immediately. Swift can behave * differently: when the reverse option is requested, all incoming @@ -843,7 +844,7 @@ public: return send_response_data(buckets); } virtual void send_response_begin(bool has_buckets) = 0; - virtual void send_response_data(rgw::sal::BucketList& buckets) = 0; + virtual void send_response_data(std::span buckets) = 0; virtual void send_response_end() = 0; void send_response() override {} diff --git a/src/rgw/rgw_rest_s3.cc b/src/rgw/rgw_rest_s3.cc index 76876faeaea..6cda26df462 100644 --- a/src/rgw/rgw_rest_s3.cc +++ b/src/rgw/rgw_rest_s3.cc @@ -87,11 +87,11 @@ void list_all_buckets_end(req_state *s) s->formatter->close_section(); } -void dump_bucket(req_state *s, rgw::sal::Bucket& obj) +void dump_bucket(req_state *s, const RGWBucketEnt& ent) { s->formatter->open_object_section("Bucket"); - s->formatter->dump_string("Name", obj.get_name()); - dump_time(s, "CreationDate", obj.get_creation_time()); + s->formatter->dump_string("Name", ent.bucket.name); + dump_time(s, "CreationDate", ent.creation_time); s->formatter->close_section(); } @@ -1454,16 +1454,13 @@ void RGWListBuckets_ObjStore_S3::send_response_begin(bool has_buckets) } } -void RGWListBuckets_ObjStore_S3::send_response_data(rgw::sal::BucketList& buckets) +void RGWListBuckets_ObjStore_S3::send_response_data(std::span buckets) { if (!sent_data) return; - auto& m = buckets.get_buckets(); - - for (auto iter = m.begin(); iter != m.end(); ++iter) { - auto& bucket = iter->second; - dump_bucket(s, *bucket); + for (const auto& ent : buckets) { + dump_bucket(s, ent); } rgw_flush_formatter(s, s->formatter); } @@ -2338,10 +2335,12 @@ void RGWGetBucketWebsite_ObjStore_S3::send_response() rgw_flush_formatter_and_reset(s, s->formatter); } -static void dump_bucket_metadata(req_state *s, rgw::sal::Bucket* bucket) +static void dump_bucket_metadata(req_state *s, rgw::sal::Bucket* bucket, + RGWStorageStats& stats) { - dump_header(s, "X-RGW-Object-Count", static_cast(bucket->get_count())); - dump_header(s, "X-RGW-Bytes-Used", static_cast(bucket->get_size())); + dump_header(s, "X-RGW-Object-Count", static_cast(stats.num_objects)); + dump_header(s, "X-RGW-Bytes-Used", static_cast(stats.size)); + // only bucket's owner is allowed to get the quota settings of the account if (bucket->is_owner(s->user.get())) { auto user_info = s->user->get_info(); @@ -2357,7 +2356,7 @@ static void dump_bucket_metadata(req_state *s, rgw::sal::Bucket* bucket) void RGWStatBucket_ObjStore_S3::send_response() { if (op_ret >= 0) { - dump_bucket_metadata(s, bucket.get()); + dump_bucket_metadata(s, bucket.get(), stats); } set_req_state_err(s, op_ret); diff --git a/src/rgw/rgw_rest_s3.h b/src/rgw/rgw_rest_s3.h index 20237166ba6..925155fcfeb 100644 --- a/src/rgw/rgw_rest_s3.h +++ b/src/rgw/rgw_rest_s3.h @@ -135,7 +135,7 @@ public: return 0; } void send_response_begin(bool has_buckets) override; - void send_response_data(rgw::sal::BucketList& buckets) override; + void send_response_data(std::span buckets) override; void send_response_end() override; }; diff --git a/src/rgw/rgw_rest_swift.cc b/src/rgw/rgw_rest_swift.cc index ff04b98a993..1f965711159 100644 --- a/src/rgw/rgw_rest_swift.cc +++ b/src/rgw/rgw_rest_swift.cc @@ -203,18 +203,36 @@ void RGWListBuckets_ObjStore_SWIFT::send_response_begin(bool has_buckets) } } -void RGWListBuckets_ObjStore_SWIFT::handle_listing_chunk(rgw::sal::BucketList&& buckets) +static bool bucket_prefix_less(const RGWBucketEnt& e, std::string_view p) { - if (wants_reversed) { - /* Just store in the reversal buffer. Its content will be handled later, - * in send_response_end(). */ - reverse_buffer.emplace(std::begin(reverse_buffer), std::move(buckets)); - } else { + return e.bucket.name < p; +} + +void RGWListBuckets_ObjStore_SWIFT::handle_listing_chunk(std::span buckets) +{ + if (!wants_reversed) { return send_response_data(buckets); } + + /* Just store in the reversal buffer. Its content will be handled later, + * in send_response_end(). */ + if (prefix.empty()) { + reverse_buffer.insert(reverse_buffer.begin(), + std::make_move_iterator(buckets.rbegin()), + std::make_move_iterator(buckets.rend())); + return; + } + + // only keep the entries that match the prefix + auto i = std::lower_bound(buckets.begin(), buckets.end(), + prefix, bucket_prefix_less); + for (; i != buckets.end() && boost::algorithm::starts_with(i->bucket.name, prefix); + ++i) { + reverse_buffer.push_front(std::move(*i)); + } } -void RGWListBuckets_ObjStore_SWIFT::send_response_data(rgw::sal::BucketList& buckets) +void RGWListBuckets_ObjStore_SWIFT::send_response_data(std::span buckets) { if (! sent_data) { return; @@ -224,22 +242,22 @@ void RGWListBuckets_ObjStore_SWIFT::send_response_data(rgw::sal::BucketList& buc * in applying the filter earlier as we really need to go through all * entries regardless of it (the headers like X-Account-Container-Count * aren't affected by specifying prefix). */ - const auto& m = buckets.get_buckets(); - for (auto iter = m.lower_bound(prefix); - iter != m.end() && boost::algorithm::starts_with(iter->first, prefix); - ++iter) { - dump_bucket_entry(*iter->second); + auto i = std::lower_bound(buckets.begin(), buckets.end(), + prefix, bucket_prefix_less); + for (; i != buckets.end() && boost::algorithm::starts_with(i->bucket.name, prefix); + ++i) { + dump_bucket_entry(*i); } } -void RGWListBuckets_ObjStore_SWIFT::dump_bucket_entry(const rgw::sal::Bucket& bucket) +void RGWListBuckets_ObjStore_SWIFT::dump_bucket_entry(const RGWBucketEnt& ent) { s->formatter->open_object_section("container"); - s->formatter->dump_string("name", bucket.get_name()); + s->formatter->dump_string("name", ent.bucket.name); if (need_stats) { - s->formatter->dump_int("count", bucket.get_count()); - s->formatter->dump_int("bytes", bucket.get_size()); + s->formatter->dump_int("count", ent.count); + s->formatter->dump_int("bytes", ent.size); } s->formatter->close_section(); @@ -249,37 +267,11 @@ void RGWListBuckets_ObjStore_SWIFT::dump_bucket_entry(const rgw::sal::Bucket& bu } } -void RGWListBuckets_ObjStore_SWIFT::send_response_data_reversed(rgw::sal::BucketList& buckets) -{ - if (! sent_data) { - return; - } - - /* Take care of the prefix parameter of Swift API. There is no business - * in applying the filter earlier as we really need to go through all - * entries regardless of it (the headers like X-Account-Container-Count - * aren't affected by specifying prefix). */ - auto& m = buckets.get_buckets(); - - auto iter = m.rbegin(); - for (/* initialized above */; - iter != m.rend() && !boost::algorithm::starts_with(iter->first, prefix); - ++iter) { - /* NOP */; - } - - for (/* iter carried */; - iter != m.rend() && boost::algorithm::starts_with(iter->first, prefix); - ++iter) { - dump_bucket_entry(*iter->second); - } -} - void RGWListBuckets_ObjStore_SWIFT::send_response_end() { if (wants_reversed) { - for (auto& buckets : reverse_buffer) { - send_response_data_reversed(buckets); + for (const auto& ent : reverse_buffer) { + dump_bucket_entry(ent); } } diff --git a/src/rgw/rgw_rest_swift.h b/src/rgw/rgw_rest_swift.h index 89873131cfb..25b20a9700d 100644 --- a/src/rgw/rgw_rest_swift.h +++ b/src/rgw/rgw_rest_swift.h @@ -36,28 +36,21 @@ public: }; class RGWListBuckets_ObjStore_SWIFT : public RGWListBuckets_ObjStore { - bool need_stats; - bool wants_reversed; + bool need_stats{true}; + bool wants_reversed{false}; std::string prefix; - std::vector reverse_buffer; + std::deque reverse_buffer; uint64_t get_default_max() const override { return 0; } public: - RGWListBuckets_ObjStore_SWIFT() - : need_stats(true), - wants_reversed(false) { - } - ~RGWListBuckets_ObjStore_SWIFT() override {} - int get_params(optional_yield y) override; - void handle_listing_chunk(rgw::sal::BucketList&& buckets) override; + void handle_listing_chunk(std::span buckets) override; void send_response_begin(bool has_buckets) override; - void send_response_data(rgw::sal::BucketList& buckets) override; - void send_response_data_reversed(rgw::sal::BucketList& buckets); - void dump_bucket_entry(const rgw::sal::Bucket& obj); + void send_response_data(std::span buckets) override; + void dump_bucket_entry(const RGWBucketEnt& ent); void send_response_end() override; bool should_get_stats() override { return need_stats; } diff --git a/src/rgw/rgw_sal.h b/src/rgw/rgw_sal.h index 6754324ea7e..b8f27db0b8a 100644 --- a/src/rgw/rgw_sal.h +++ b/src/rgw/rgw_sal.h @@ -453,6 +453,18 @@ class Driver { virtual void register_admin_apis(RGWRESTMgr* mgr) = 0; }; +/** + * @brief A list of buckets + * + * This is the result from a bucket listing operation. + */ +struct BucketList { + /// The list of results, sorted by bucket name + std::vector buckets; + /// The next marker to resume listing, or empty + std::string next_marker; +}; + /** * @brief User abstraction * @@ -814,51 +826,6 @@ class Bucket { virtual bool operator==(const Bucket& b) const = 0; virtual bool operator!=(const Bucket& b) const = 0; - - friend class BucketList; -}; - -/** - * @brief A list of buckets - * - * This is the result from a bucket listing operation. - */ -class BucketList { - std::map> buckets; - bool truncated; - -public: - BucketList() : buckets(), truncated(false) {} - BucketList(BucketList&& _bl) : - buckets(std::move(_bl.buckets)), - truncated(_bl.truncated) - { } - BucketList& operator=(const BucketList&) = delete; - BucketList& operator=(BucketList&& _bl) { - for (auto& ent : _bl.buckets) { - buckets.emplace(ent.first, std::move(ent.second)); - } - truncated = _bl.truncated; - return *this; - }; - - /** Get the list of buckets. The list is a map of pairs. */ - std::map>& get_buckets() { return buckets; } - /** True if the list is truncated (that is, there are more buckets to list) */ - bool is_truncated(void) const { return truncated; } - /** Set the truncated state of the list */ - void set_truncated(bool trunc) { truncated = trunc; } - /** Add a bucket to the list. Takes ownership of the bucket */ - void add(std::unique_ptr bucket) { - buckets.emplace(bucket->get_name(), std::move(bucket)); - } - /** The number of buckets in this list */ - size_t count() const { return buckets.size(); } - /** Clear the list */ - void clear(void) { - buckets.clear(); - truncated = false; - } }; /** diff --git a/src/rgw/rgw_sal_dbstore.cc b/src/rgw/rgw_sal_dbstore.cc index 2465b03e586..5b18986a902 100644 --- a/src/rgw/rgw_sal_dbstore.cc +++ b/src/rgw/rgw_sal_dbstore.cc @@ -34,23 +34,27 @@ namespace rgw::sal { int DBUser::list_buckets(const DoutPrefixProvider *dpp, const string& marker, const string& end_marker, uint64_t max, bool need_stats, - BucketList &buckets, optional_yield y) + BucketList &result, optional_yield y) { RGWUserBuckets ulist; bool is_truncated = false; - int ret; - buckets.clear(); - ret = store->getDB()->list_buckets(dpp, "", info.user_id, marker, end_marker, max, - need_stats, &ulist, &is_truncated); + int ret = store->getDB()->list_buckets(dpp, "", info.user_id, marker, + end_marker, max, need_stats, &ulist, &is_truncated); if (ret < 0) return ret; - buckets.set_truncated(is_truncated); - for (const auto& ent : ulist.get_buckets()) { - buckets.add(std::make_unique(this->store, ent.second, this)); + result.buckets.clear(); + + for (auto& ent : ulist.get_buckets()) { + result.buckets.push_back(std::move(ent.second)); } + if (is_truncated && !result.buckets.empty()) { + result.next_marker = result.buckets.back().bucket.name; + } else { + result.next_marker.clear(); + } return 0; } diff --git a/src/rgw/rgw_sal_filter.cc b/src/rgw/rgw_sal_filter.cc index 78604774003..32bdaa89346 100644 --- a/src/rgw/rgw_sal_filter.cc +++ b/src/rgw/rgw_sal_filter.cc @@ -536,20 +536,8 @@ int FilterUser::list_buckets(const DoutPrefixProvider* dpp, const std::string& m const std::string& end_marker, uint64_t max, bool need_stats, BucketList &buckets, optional_yield y) { - BucketList bl; - int ret; - - buckets.clear(); - ret = next->list_buckets(dpp, marker, end_marker, max, need_stats, bl, y); - if (ret < 0) - return ret; - - buckets.set_truncated(bl.is_truncated()); - for (auto& ent : bl.get_buckets()) { - buckets.add(std::make_unique(std::move(ent.second), this)); - } - - return 0; + return next->list_buckets(dpp, marker, end_marker, max, + need_stats, buckets, y); } int FilterUser::create_bucket(const DoutPrefixProvider* dpp, diff --git a/src/rgw/rgw_user.cc b/src/rgw/rgw_user.cc index 3604cad799c..cdaa08756df 100644 --- a/src/rgw/rgw_user.cc +++ b/src/rgw/rgw_user.cc @@ -16,33 +16,28 @@ using namespace std; int rgw_user_sync_all_stats(const DoutPrefixProvider *dpp, rgw::sal::Driver* driver, rgw::sal::User* user, optional_yield y) { - rgw::sal::BucketList user_buckets; - - CephContext *cct = driver->ctx(); - size_t max_entries = cct->_conf->rgw_list_buckets_max_chunk; - string marker; - int ret; + size_t max_entries = dpp->get_cct()->_conf->rgw_list_buckets_max_chunk; + rgw::sal::BucketList listing; + int ret = 0; do { - ret = user->list_buckets(dpp, marker, string(), max_entries, false, user_buckets, y); + ret = user->list_buckets(dpp, listing.next_marker, string(), + max_entries, false, listing, y); if (ret < 0) { ldpp_dout(dpp, 0) << "failed to read user buckets: ret=" << ret << dendl; return ret; } - auto& buckets = user_buckets.get_buckets(); - for (auto i = buckets.begin(); i != buckets.end(); ++i) { - marker = i->first; - - auto& bucket = i->second; - ret = bucket->load_bucket(dpp, y); + for (auto& ent : listing.buckets) { + std::unique_ptr bucket; + ret = driver->get_bucket(dpp, user, ent.bucket, &bucket, y); if (ret < 0) { ldpp_dout(dpp, 0) << "ERROR: could not read bucket info: bucket=" << bucket << " ret=" << ret << dendl; continue; } ret = bucket->sync_user_stats(dpp, y); if (ret < 0) { - ldout(cct, 0) << "ERROR: could not sync bucket stats: ret=" << ret << dendl; + ldpp_dout(dpp, 0) << "ERROR: could not sync bucket stats: ret=" << ret << dendl; return ret; } ret = bucket->check_bucket_shards(dpp, y); @@ -50,7 +45,7 @@ int rgw_user_sync_all_stats(const DoutPrefixProvider *dpp, rgw::sal::Driver* dri ldpp_dout(dpp, 0) << "ERROR in check_bucket_shards: " << cpp_strerror(-ret)<< dendl; } } - } while (user_buckets.is_truncated()); + } while (!listing.next_marker.empty()); ret = user->complete_flush_stats(dpp, y); if (ret < 0) { @@ -67,38 +62,26 @@ int rgw_user_get_all_buckets_stats(const DoutPrefixProvider *dpp, map& buckets_usage_map, optional_yield y) { - CephContext *cct = driver->ctx(); - size_t max_entries = cct->_conf->rgw_list_buckets_max_chunk; - bool done; - string marker; - int ret; + size_t max_entries = dpp->get_cct()->_conf->rgw_list_buckets_max_chunk; + rgw::sal::BucketList listing; do { - rgw::sal::BucketList buckets; - ret = user->list_buckets(dpp, marker, string(), max_entries, false, buckets, y); + int ret = user->list_buckets(dpp, listing.next_marker, string(), + max_entries, false, listing, y); if (ret < 0) { ldpp_dout(dpp, 0) << "failed to read user buckets: ret=" << ret << dendl; return ret; } - auto& m = buckets.get_buckets(); - for (const auto& i : m) { - marker = i.first; - auto& bucket_ent = i.second; - ret = bucket_ent->load_bucket(dpp, y, true /* load user stats */); - if (ret < 0) { - ldpp_dout(dpp, 0) << "ERROR: could not get bucket stats: ret=" << ret << dendl; - return ret; - } + for (const auto& ent : listing.buckets) { bucket_meta_entry entry; - entry.size = bucket_ent->get_size(); - entry.size_rounded = bucket_ent->get_size_rounded(); - entry.creation_time = bucket_ent->get_creation_time(); - entry.count = bucket_ent->get_count(); - buckets_usage_map.emplace(bucket_ent->get_name(), entry); + entry.size = ent.size; + entry.size_rounded = ent.size_rounded; + entry.creation_time = ent.creation_time; + entry.count = ent.count; + buckets_usage_map.emplace(ent.bucket.name, entry); } - done = (buckets.count() < max_entries); - } while (!done); + } while (!listing.next_marker.empty()); return 0; } diff --git a/src/test/rgw/test_rgw_lua.cc b/src/test/rgw/test_rgw_lua.cc index ea721afd5b2..80df413cca1 100644 --- a/src/test/rgw/test_rgw_lua.cc +++ b/src/test/rgw/test_rgw_lua.cc @@ -74,7 +74,7 @@ public: return std::unique_ptr(new TestUser(*this)); } - virtual int list_buckets(const DoutPrefixProvider *dpp, const string&, const string&, uint64_t, bool, sal::BucketList&, optional_yield y) override { + virtual int list_buckets(const DoutPrefixProvider *dpp, const string&, const string&, uint64_t, bool, sal::BucketList& results, optional_yield y) { return 0; }