From: J. Eric Ivancich Date: Mon, 2 Aug 2021 19:46:47 +0000 (-0400) Subject: rgw: clean up some code X-Git-Tag: v17.1.0~1192^2 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=44d3f86a7deb0c643835c171f98389b17f4505e1;p=ceph-ci.git rgw: clean up some code In rgw_admin.cc a variable `f` is used frequently. However due to the sheer size of the file and variable scopes involved, finding the declaration of `f`, which is a common character, can be challenging. So it has been renamed `stream_flusher`, which better indicates its purpose and makes it easier to find the corresponding declaration. Additionally some unneeded variables in RGWBucket::check_object_index were removed. rgw::sal::Bucket::ListParams initializes parameters as one would hope/expect. Signed-off-by: J. Eric Ivancich --- diff --git a/src/rgw/rgw_admin.cc b/src/rgw/rgw_admin.cc index 3a4546b32fe..5e9ae500914 100644 --- a/src/rgw/rgw_admin.cc +++ b/src/rgw/rgw_admin.cc @@ -3912,7 +3912,7 @@ int main(int argc, const char **argv) zone_name = g_conf()->rgw_zone; zonegroup_name = g_conf()->rgw_zonegroup; - RGWStreamFlusher f(formatter.get(), cout); + RGWStreamFlusher stream_flusher(formatter.get(), cout); RGWUserAdminOpState user_op(store); if (!user_email.empty()) { @@ -6013,7 +6013,7 @@ int main(int argc, const char **argv) return -ret; } } else { - int ret = RGWBucketAdminOp::get_policy(store, bucket_op, f, dpp()); + int ret = RGWBucketAdminOp::get_policy(store, bucket_op, stream_flusher, dpp()); if (ret < 0) { cerr << "ERROR: failed to get policy: " << cpp_strerror(-ret) << std::endl; return -ret; @@ -6032,7 +6032,7 @@ int main(int argc, const char **argv) if (!rgw::sal::User::empty(user)) { user_ids.push_back(user->get_id().id); ret = - RGWBucketAdminOp::limit_check(store, bucket_op, user_ids, f, + RGWBucketAdminOp::limit_check(store, bucket_op, user_ids, stream_flusher, null_yield, dpp(), warnings_only); } else { /* list users in groups of max-keys, then perform user-bucket @@ -6054,7 +6054,7 @@ int main(int argc, const char **argv) } else { /* ok, do the limit checks for this group */ ret = - RGWBucketAdminOp::limit_check(store, bucket_op, user_ids, f, + RGWBucketAdminOp::limit_check(store, bucket_op, user_ids, stream_flusher, null_yield, dpp(), warnings_only); if (ret < 0) break; @@ -6074,7 +6074,7 @@ int main(int argc, const char **argv) return -ENOENT; } } - RGWBucketAdminOp::info(store, bucket_op, f, null_yield, dpp()); + RGWBucketAdminOp::info(store, bucket_op, stream_flusher, null_yield, dpp()); } else { int ret = init_bucket(user.get(), tenant, bucket_name, bucket_id, &bucket); if (ret < 0) { @@ -6169,7 +6169,7 @@ int main(int argc, const char **argv) } bucket_op.set_fetch_stats(true); - int r = RGWBucketAdminOp::info(store, bucket_op, f, null_yield, dpp()); + int r = RGWBucketAdminOp::info(store, bucket_op, stream_flusher, null_yield, dpp()); if (r < 0) { cerr << "failure: " << cpp_strerror(-r) << ": " << err << std::endl; return -r; @@ -6419,7 +6419,7 @@ next: } ret = RGWUsage::show(dpp(), store, user.get(), bucket.get(), start_epoch, end_epoch, show_log_entries, show_log_sum, &categories, - f); + stream_flusher); if (ret < 0) { cerr << "ERROR: failed to show usage" << std::endl; return 1; @@ -6789,7 +6789,7 @@ next: } if (opt_cmd == OPT::OBJECTS_EXPIRE_STALE_LIST) { - ret = RGWBucketAdminOp::fix_obj_expiry(store, bucket_op, f, dpp(), true); + ret = RGWBucketAdminOp::fix_obj_expiry(store, bucket_op, stream_flusher, dpp(), true); if (ret < 0) { cerr << "ERROR: listing returned " << cpp_strerror(-ret) << std::endl; return -ret; @@ -6797,7 +6797,7 @@ next: } if (opt_cmd == OPT::OBJECTS_EXPIRE_STALE_RM) { - ret = RGWBucketAdminOp::fix_obj_expiry(store, bucket_op, f, dpp(), false); + ret = RGWBucketAdminOp::fix_obj_expiry(store, bucket_op, stream_flusher, dpp(), false); if (ret < 0) { cerr << "ERROR: removing returned " << cpp_strerror(-ret) << std::endl; return -ret; @@ -7178,7 +7178,7 @@ next: } do_check_object_locator(tenant, bucket_name, fix, remove_bad, formatter.get()); } else { - RGWBucketAdminOp::check_index(store, bucket_op, f, null_yield, dpp()); + RGWBucketAdminOp::check_index(store, bucket_op, stream_flusher, null_yield, dpp()); } } @@ -7319,7 +7319,7 @@ next: if (opt_cmd == OPT::LC_RESHARD_FIX) { - ret = RGWBucketAdminOp::fix_lc_shards(store, bucket_op, f, dpp()); + ret = RGWBucketAdminOp::fix_lc_shards(store, bucket_op, stream_flusher, dpp()); if (ret < 0) { cerr << "ERROR: listing stale instances" << cpp_strerror(-ret) << std::endl; } @@ -9130,7 +9130,7 @@ next: return EINVAL; } - ret = RGWBucketAdminOp::list_stale_instances(store, bucket_op, f, dpp()); + ret = RGWBucketAdminOp::list_stale_instances(store, bucket_op, stream_flusher, dpp()); if (ret < 0) { cerr << "ERROR: listing stale instances" << cpp_strerror(-ret) << std::endl; } @@ -9142,7 +9142,7 @@ next: return EINVAL; } - ret = RGWBucketAdminOp::clear_stale_instances(store, bucket_op,f, dpp()); + ret = RGWBucketAdminOp::clear_stale_instances(store, bucket_op, stream_flusher, dpp()); if (ret < 0) { cerr << "ERROR: deleting stale instances" << cpp_strerror(-ret) << std::endl; } diff --git a/src/rgw/rgw_bucket.cc b/src/rgw/rgw_bucket.cc index 87cdd054ec8..b020ac21205 100644 --- a/src/rgw/rgw_bucket.cc +++ b/src/rgw/rgw_bucket.cc @@ -594,8 +594,6 @@ int RGWBucket::check_object_index(const DoutPrefixProvider *dpp, bucket->set_tag_timeout(dpp, BUCKET_TAG_TIMEOUT); - string prefix; - string empty_delimiter; rgw::sal::Bucket::ListResults results; results.is_truncated = true; @@ -603,15 +601,13 @@ int RGWBucket::check_object_index(const DoutPrefixProvider *dpp, formatter->open_object_section("objects"); while (results.is_truncated) { rgw::sal::Bucket::ListParams params; - params.marker = results.next_marker; - params.prefix = prefix; int r = bucket->list(dpp, params, listing_max_entries, results, y); if (r == -ENOENT) { break; - } else if (r < 0 && r != -ENOENT) { + } else if (r < 0) { set_err_msg(err_msg, "ERROR: failed operation r=" + cpp_strerror(-r)); } diff --git a/src/rgw/rgw_bucket.h b/src/rgw/rgw_bucket.h index d09abfd9181..30e1fd76942 100644 --- a/src/rgw/rgw_bucket.h +++ b/src/rgw/rgw_bucket.h @@ -1,8 +1,7 @@ // -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- // vim: ts=8 sw=2 smarttab ft=cpp -#ifndef CEPH_RGW_BUCKET_H -#define CEPH_RGW_BUCKET_H +#pragma once #include #include @@ -124,8 +123,7 @@ public: /** * Store a list of the user's buckets, with associated functinos. */ -class RGWUserBuckets -{ +class RGWUserBuckets { std::map buckets; public: @@ -310,12 +308,11 @@ struct RGWBucketAdminOpState { bucket_stored(false), sync_bucket(true) {} }; + /* * A simple wrapper class for administrative bucket operations */ - -class RGWBucket -{ +class RGWBucket { RGWUserBuckets buckets; rgw::sal::Store* store; RGWAccessHandle handle; @@ -362,8 +359,7 @@ public: const RGWBucketInfo& get_bucket_info() const { return bucket->get_info(); } }; -class RGWBucketAdminOp -{ +class RGWBucketAdminOp { public: static int get_policy(rgw::sal::Store* store, RGWBucketAdminOpState& op_state, RGWFormatterFlusher& flusher, const DoutPrefixProvider *dpp); @@ -411,8 +407,7 @@ struct rgw_ep_info { : ep(ep), attrs(attrs) {} }; -class RGWBucketCtl -{ +class RGWBucketCtl { CephContext *cct; struct Svc { @@ -762,5 +757,3 @@ private: bool rgw_find_bucket_by_id(const DoutPrefixProvider *dpp, CephContext *cct, rgw::sal::Store* store, const string& marker, const string& bucket_id, rgw_bucket* bucket_out); - -#endif diff --git a/src/rgw/rgw_rados.cc b/src/rgw/rgw_rados.cc index 717a34c7a4a..59a887910bd 100644 --- a/src/rgw/rgw_rados.cc +++ b/src/rgw/rgw_rados.cc @@ -4920,30 +4920,30 @@ int RGWRados::bucket_check_index(const DoutPrefixProvider *dpp, RGWBucketInfo& b map *calculated_stats) { RGWSI_RADOS::Pool index_pool; + // key - bucket index object id // value - bucket index check OP returned result with the given bucket index object (shard) map oids; - map bucket_objs_ret; - int ret = svc.bi_rados->open_bucket_index(dpp, bucket_info, std::nullopt, &index_pool, &oids, nullptr); if (ret < 0) { - return ret; + return ret; } + // declare and pre-populate + map bucket_objs_ret; for (auto& iter : oids) { - bucket_objs_ret[iter.first] = rgw_cls_check_index_ret(); + bucket_objs_ret.emplace(iter.first, rgw_cls_check_index_ret()); } ret = CLSRGWIssueBucketCheck(index_pool.ioctx(), oids, bucket_objs_ret, cct->_conf->rgw_bucket_index_max_aio)(); if (ret < 0) { - return ret; + return ret; } - // Aggregate results (from different shards if there is any) - map::iterator iter; - for (iter = bucket_objs_ret.begin(); iter != bucket_objs_ret.end(); ++iter) { - accumulate_raw_stats(iter->second.existing_header, *existing_stats); - accumulate_raw_stats(iter->second.calculated_header, *calculated_stats); + // aggregate results (from different shards if there are any) + for (const auto& iter : bucket_objs_ret) { + accumulate_raw_stats(iter.second.existing_header, *existing_stats); + accumulate_raw_stats(iter.second.calculated_header, *calculated_stats); } return 0;