Merge pull request ceph#35405 from ivancich/wip-fix-truncated-bucket-…

…listing

rgw: bucket list/stats truncates for user w/ >1000 buckets

 Reviewed-by: Matt Benjamin <[email protected]>

src/common/options.cc

@@ -6557,8 +6557,9 @@ std::vector<Option> get_rgw_options() {
     .set_default(1000)
     .set_description("Max number of buckets per user")
     .set_long_description(
-        "A user can create this many buckets. Zero means unlimited, negative number means "
-        "user cannot create any buckets (although user will retain buckets already created."),
+      "A user can create at most this number of buckets. Zero means "
+      "no limit; a negative value means users cannot create any new "
+      "buckets, although users will retain buckets already created."),
 
     Option("rgw_objexp_gc_interval", Option::TYPE_UINT, Option::LEVEL_ADVANCED)
     .set_default(10_min)

src/rgw/rgw_auth.cc

@@ -524,7 +524,8 @@ void rgw::auth::RemoteApplier::create_account(const DoutPrefixProvider* dpp,
   user_info.user_id = new_acct_user;
   user_info.display_name = info.acct_name;
 
-  user_info.max_buckets = cct->_conf->rgw_user_max_buckets;
+  user_info.max_buckets =
+    cct->_conf.get_val<int64_t>("rgw_user_max_buckets");
   rgw_apply_default_bucket_quota(user_info.bucket_quota, cct->_conf);
   rgw_apply_default_user_quota(user_info.user_quota, cct->_conf);
 

src/rgw/rgw_bucket.cc

@@ -1400,22 +1400,30 @@ int RGWBucketAdminOp::sync_bucket(rgw::sal::RGWRadosStore *store, RGWBucketAdmin
   return bucket.sync(op_state, &attrs, err_msg);
 }
 
-static int bucket_stats(rgw::sal::RGWRadosStore *store, const std::string& tenant_name, std::string&  bucket_name, Formatter *formatter)
+static int bucket_stats(rgw::sal::RGWRadosStore *store,
+			const std::string& tenant_name,
+			const std::string& bucket_name,
+			Formatter *formatter)
 {
   RGWBucketInfo bucket_info;
   map<RGWObjCategory, RGWStorageStats> stats;
   map<string, bufferlist> attrs;
 
   real_time mtime;
-  int r = store->getRados()->get_bucket_info(store->svc(), tenant_name, bucket_name, bucket_info, &mtime, null_yield, &attrs);
-  if (r < 0)
+  int r = store->getRados()->get_bucket_info(store->svc(),
+					     tenant_name, bucket_name, bucket_info,
+					     &mtime, null_yield, &attrs);
+  if (r < 0) {
     return r;
+  }
 
   rgw_bucket& bucket = bucket_info.bucket;
 
   string bucket_ver, master_ver;
   string max_marker;
-  int ret = store->getRados()->get_bucket_stats(bucket_info, RGW_NO_SHARD, &bucket_ver, &master_ver, stats, &max_marker);
+  int ret = store->getRados()->get_bucket_stats(bucket_info, RGW_NO_SHARD,
+						&bucket_ver, &master_ver, stats,
+						&max_marker);
   if (ret < 0) {
     cerr << "error getting bucket stats ret=" << ret << std::endl;
     return ret;
@@ -1426,7 +1434,8 @@ static int bucket_stats(rgw::sal::RGWRadosStore *store, const std::string& tenan
 
   formatter->open_object_section("stats");
   formatter->dump_string("bucket", bucket.name);
-  formatter->dump_int("num_shards", bucket_info.layout.current_index.layout.normal.num_shards);
+  formatter->dump_int("num_shards",
+		      bucket_info.layout.current_index.layout.normal.num_shards);
   formatter->dump_string("tenant", bucket.tenant);
   formatter->dump_string("zonegroup", bucket_info.zonegroup);
   formatter->dump_string("placement_rule", bucket_info.placement_rule.to_str());
@@ -1584,52 +1593,61 @@ int RGWBucketAdminOp::limit_check(rgw::sal::RGWRadosStore *store,
   return ret;
 } /* RGWBucketAdminOp::limit_check */
 
-int RGWBucketAdminOp::info(rgw::sal::RGWRadosStore *store, RGWBucketAdminOpState& op_state,
-                  RGWFormatterFlusher& flusher)
+int RGWBucketAdminOp::info(rgw::sal::RGWRadosStore *store,
+			   RGWBucketAdminOpState& op_state,
+			   RGWFormatterFlusher& flusher)
 {
   int ret = 0;
-  string bucket_name = op_state.get_bucket_name();
+  const std::string& bucket_name = op_state.get_bucket_name();
   Formatter *formatter = flusher.get_formatter();
   flusher.start(0);
 
   CephContext *cct = store->ctx();
 
   const size_t max_entries = cct->_conf->rgw_list_buckets_max_chunk;
 
-  bool show_stats = op_state.will_fetch_stats();
-  rgw_user user_id = op_state.get_user_id();
+  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()) {
     formatter->open_array_section("buckets");
 
     rgw::sal::RGWBucketList buckets;
     rgw::sal::RGWRadosUser user(store, op_state.get_user_id());
-    string marker;
-    bool is_truncated = false;
+    std::string marker;
+    const std::string empty_end_marker;
+    constexpr bool no_need_stats = false; // set need_stats to false
 
     do {
-      ret = user.list_buckets(marker, string(), max_entries, false, buckets);
-      if (ret < 0)
+      buckets.clear();
+      ret = user.list_buckets(marker, empty_end_marker, max_entries,
+			      no_need_stats, buckets);
+      if (ret < 0) {
         return ret;
+      }
 
+      const std::string* marker_cursor = nullptr;
       map<string, rgw::sal::RGWBucket*>& m = buckets.get_buckets();
-      map<string, rgw::sal::RGWBucket*>::iterator iter;
 
-      for (iter = m.begin(); iter != m.end(); ++iter) {
-        std::string obj_name = iter->first;
+      for (const auto& i : m) {
+        const std::string& obj_name = i.first;
         if (!bucket_name.empty() && bucket_name != obj_name) {
           continue;
         }
 
-        if (show_stats)
+        if (show_stats) {
           bucket_stats(store, user_id.tenant, obj_name, formatter);
-        else
+	} else {
           formatter->dump_string("bucket", obj_name);
+	}
 
-        marker = obj_name;
+        marker_cursor = &obj_name;
+      } // for loop
+      if (marker_cursor) {
+	marker = *marker_cursor;
       }
 
       flusher.flush();
-    } while (is_truncated);
+    } while (buckets.is_truncated());
 
     formatter->close_section();
   } else if (!bucket_name.empty()) {
@@ -1645,14 +1663,15 @@ int RGWBucketAdminOp::info(rgw::sal::RGWRadosStore *store, RGWBucketAdminOpState
     ret = store->ctl()->meta.mgr->list_keys_init("bucket", &handle);
     while (ret == 0 && truncated) {
       std::list<std::string> buckets;
-      const int max_keys = 1000;
+      constexpr int max_keys = 1000;
       ret = store->ctl()->meta.mgr->list_keys_next(handle, max_keys, buckets,
-                                            &truncated);
+						   &truncated);
       for (auto& bucket_name : buckets) {
-        if (show_stats)
+        if (show_stats) {
           bucket_stats(store, user_id.tenant, bucket_name, formatter);
-        else
+	} else {
           formatter->dump_string("bucket", bucket_name);
+	}
       }
     }
     store->ctl()->meta.mgr->list_keys_complete(handle);

src/rgw/rgw_cr_tools.cc

@@ -20,7 +20,8 @@ int RGWUserCreateCR::Request::_send_request()
 {
   CephContext *cct = store->ctx();
 
-  int32_t default_max_buckets = cct->_conf->rgw_user_max_buckets;
+  const int32_t default_max_buckets =
+    cct->_conf.get_val<int64_t>("rgw_user_max_buckets");
 
   RGWUserAdminOpState op_state;
 

src/rgw/rgw_rest_user.cc

@@ -125,7 +125,8 @@ void RGWOp_User_Create::execute()
   bool exclusive;
 
   int32_t max_buckets;
-  int32_t default_max_buckets = s->cct->_conf->rgw_user_max_buckets;
+  const int32_t default_max_buckets =
+    s->cct->_conf.get_val<int64_t>("rgw_user_max_buckets");
 
   RGWUserAdminOpState op_state;
 

src/rgw/rgw_sal.h

@@ -154,8 +154,8 @@ class RGWBucketList {
     buckets[bucket->ent.bucket.name] = bucket;
   }
   size_t count() const { return buckets.size(); }
-
-};
+  void clear() { buckets.clear(); truncated = false; }
+}; // class RGWBucketList
 
 class RGWObject {
   protected:

src/rgw/rgw_user.cc

@@ -1743,7 +1743,8 @@ int RGWUser::execute_add(RGWUserAdminOpState& op_state, std::string *err_msg)
   if (op_state.max_buckets_specified) {
     user_info.max_buckets = op_state.get_max_buckets();
   } else {
-    user_info.max_buckets = cct->_conf->rgw_user_max_buckets;
+    user_info.max_buckets =
+      cct->_conf.get_val<int64_t>("rgw_user_max_buckets");
   }
 
   user_info.suspended = op_state.get_suspension_status();