rgw: allow ordered bucket listing to work when many filtered out entries

A previous PR moved the much of the filtering that's part of bucket
listing to the CLS layer. One unanticipated result was that it is now
possible for a call to return 0 entries. In such a case we want to
retry the call with the marker moved forward (i.e., advanced),
repeatedly if necessary, in order to either retrieve some entries or
to hit the end of the entries. This PR adds that functionality.

Signed-off-by: J. Eric Ivancich <ivancich@redhat.com>
(cherry picked from commit 423c18308a35f0f2d6c7697539301cf7001d6329)

Conflicts:
	src/cls/rgw/cls_rgw_ops.h
s/ceph::buffer::list/bufferlist/g

diff --git a/src/cls/rgw/cls_rgw.cc b/src/cls/rgw/cls_rgw.cc
index 2472328257f3a7323bd222c22e986c9791986d41..320acb43ae2507629ec3f7cff204e9a460bf5c30 100644 (file)
--- a/src/cls/rgw/cls_rgw.cc
+++ b/src/cls/rgw/cls_rgw.cc
@@ -169,10 +169,10 @@ static int log_index_operation(cls_method_context_t hctx, cls_rgw_obj_key& obj_k
  * namespace".
  */
 static int get_obj_vals(cls_method_context_t hctx,
-                       const string& start,
-                       const string& filter_prefix,
+                       const std::string& start,
+                       const std::string& filter_prefix,
                         int num_entries,
-                       map<string, bufferlist> *pkeys,
+                       std::map<std::string, bufferlist> *pkeys,
                        bool *pmore)
 {
   int ret = cls_cxx_map_get_vals(hctx, start, filter_prefix,
@@ -185,7 +185,7 @@ static int get_obj_vals(cls_method_context_t hctx,
     return 0;
   }
 
-  auto last_element = pkeys->rbegin();
+  auto last_element = pkeys->crbegin();
   if ((unsigned char)last_element->first[0] < BI_PREFIX_CHAR) {
     /* if the first character of the last entry is less than the
      * prefix then all entries must preceed the "ugly namespace" and
@@ -194,11 +194,11 @@ static int get_obj_vals(cls_method_context_t hctx,
     return 0;
   }
 
-  auto first_element = pkeys->begin();
+  auto first_element = pkeys->cbegin();
   if ((unsigned char)first_element->first[0] > BI_PREFIX_CHAR) {
-    /* the first character of the last entry is in or after the "ugly
-     * namespace", so if the first character of the first entry
-     * follows the "ugly namespace" then all entries do and we're done
+    /* if the first character of the first entry is after the "ugly
+     * namespace" then all entries must follow the "ugly namespace"
+     * then all entries do and we're done
      */
     return 0;
   }
@@ -209,10 +209,10 @@ static int get_obj_vals(cls_method_context_t hctx,
    * outside the "ugly namespace"
    */
 
-  auto comp = [](const pair<string, bufferlist>& l, const string &r) {
+  auto comp = [](const pair<std::string, bufferlist>& l, const std::string &r) {
                return l.first < r;
              };
-  string new_start = {static_cast<char>(BI_PREFIX_CHAR + 1)};
+  std::string new_start = {static_cast<char>(BI_PREFIX_CHAR + 1)};
 
   auto lower = pkeys->lower_bound(string{static_cast<char>(BI_PREFIX_CHAR)});
   auto upper = std::lower_bound(lower, pkeys->end(), new_start, comp);
@@ -222,11 +222,11 @@ static int get_obj_vals(cls_method_context_t hctx,
     return 0;
   }
 
-  if (pkeys->size() && new_start < pkeys->rbegin()->first) {
+  if (pkeys->size() && new_start < pkeys->crbegin()->first) {
     new_start = pkeys->rbegin()->first;
   }
 
-  map<string, bufferlist> new_keys;
+  std::map<std::string, bufferlist> new_keys;
 
   /* now get some more keys */
   ret = cls_cxx_map_get_vals(hctx, new_start, filter_prefix,
@@ -480,29 +480,42 @@ int rgw_bucket_list(cls_method_context_t hctx, bufferlist *in, bufferlist *out)
     return rc;
   }
 
-  string start_after_key;   // key that we can start listing at, one of a)
-                            // sent in by caller, b) last item visited, or
-                            // c) when delimiter present, a key that will
-                            // move past the subdirectory
-  encode_list_index_key(hctx, op.start_obj, &start_after_key);
+  // some calls just want the header and request 0 entries
+  if (op.num_entries <= 0) {
+    ret.is_truncated = false;
+    encode(ret, *out);
+    return 0;
+  }
+
+  // key that we can start listing at, one of a) sent in by caller, b)
+  // last item visited, or c) when delimiter present, a key that will
+  // move past the subdirectory
+  std::string start_after_omap_key;
+  encode_list_index_key(hctx, op.start_obj, &start_after_omap_key);
+
+  // this is set whenenver start_after_omap_key is set to keep them in
+  // sync since this will be the returned marker when a marker is
+  // returned
+  cls_rgw_obj_key start_after_entry_key;
+
+  // last key stored in result, so if we have to call get_obj_vals
+  // multiple times, we do not add the overlap to result
+  std::string prev_omap_key;
 
-  string previous_key; // last key stored in result, so if we have to
-                      // call get_obj_vals multiple times, we do not
-                      // add the overlap to result
-  string previous_prefix_key; // last prefix_key stored in result, so
-                             // we can skip over entries with the
-                             // same prefix_key
+  // last prefix_key stored in result, so we can skip over entries
+  // with the same prefix_key
+  std::string prev_prefix_omap_key;
 
   bool done = false;   // whether we need to keep calling get_obj_vals
   bool more = true;    // output parameter of get_obj_vals
   bool has_delimiter = !op.delimiter.empty();
 
   if (has_delimiter &&
-      start_after_key > op.filter_prefix &&
-      boost::algorithm::ends_with(start_after_key, op.delimiter)) {
+      start_after_omap_key > op.filter_prefix &&
+      boost::algorithm::ends_with(start_after_omap_key, op.delimiter)) {
     // advance past all subdirectory entries if we start after a
     // subdirectory
-    start_after_key = cls_rgw_after_delim(start_after_key);
+    start_after_omap_key = cls_rgw_after_delim(start_after_omap_key);
   }
 
   for (int attempt = 0;
@@ -511,8 +524,12 @@ int rgw_bucket_list(cls_method_context_t hctx, bufferlist *in, bufferlist *out)
         !done &&
         name_entry_map.size() < op.num_entries;
        ++attempt) {
-    map<string, bufferlist> keys;
-    rc = get_obj_vals(hctx, start_after_key, op.filter_prefix,
+    std::map<std::string, bufferlist> keys;
+
+    // note: get_obj_vals skips past the "ugly namespace" (i.e.,
+    // entries that start with the BI_PREFIX_CHAR), so no need to
+    // check for such entries
+    rc = get_obj_vals(hctx, start_after_omap_key, op.filter_prefix,
                      op.num_entries - name_entry_map.size(),
                      &keys, &more);
     if (rc < 0) {
@@ -524,13 +541,6 @@ int rgw_bucket_list(cls_method_context_t hctx, bufferlist *in, bufferlist *out)
     done = keys.empty();
 
     for (auto kiter = keys.cbegin(); kiter != keys.cend(); ++kiter) {
-      if (!bi_is_plain_entry(kiter->first)) {
-       // we're done if we walked off the end of the objects area of
-       // the bucket index
-        done = true;
-        break;
-      }
-
       rgw_bucket_dir_entry entry;
       try {
        const bufferlist& entrybl = kiter->second;
@@ -542,7 +552,8 @@ int rgw_bucket_list(cls_method_context_t hctx, bufferlist *in, bufferlist *out)
         return -EINVAL;
       }
 
-      start_after_key = kiter->first;
+      start_after_omap_key = kiter->first;
+      start_after_entry_key = entry.key;
       CLS_LOG(20, "%s: working on key=%s len=%zu",
              __func__, kiter->first.c_str(), kiter->first.size());
 
@@ -577,10 +588,10 @@ int rgw_bucket_list(cls_method_context_t hctx, bufferlist *in, bufferlist *out)
           string prefix_key =
            key.name.substr(0, delim_pos + op.delimiter.length());
 
-         if (prefix_key == previous_prefix_key) {
+         if (prefix_key == prev_prefix_omap_key) {
            continue; // we've already added this;
          } else {
-           previous_prefix_key = prefix_key;
+           prev_prefix_omap_key = prefix_key;
          }
 
          if (name_entry_map.size() < op.num_entries) {
@@ -598,11 +609,12 @@ int rgw_bucket_list(cls_method_context_t hctx, bufferlist *in, bufferlist *out)
          // make sure that if this is the last item added to the
          // result from this call to get_obj_vals, the next call will
          // skip past rest of "subdirectory"
-         start_after_key = cls_rgw_after_delim(prefix_key);
+         start_after_omap_key = cls_rgw_after_delim(prefix_key);
+         start_after_entry_key.set(start_after_omap_key);
 
-         // advance to past this subdirectory, but then back up one,
+         // advance past this subdirectory, but then back up one,
          // so the loop increment will put us in the right place
-         kiter = keys.lower_bound(start_after_key);
+         kiter = keys.lower_bound(start_after_omap_key);
          --kiter;
 
           continue;
@@ -613,9 +625,9 @@ int rgw_bucket_list(cls_method_context_t hctx, bufferlist *in, bufferlist *out)
       }
 
       if (name_entry_map.size() < op.num_entries &&
-         kiter->first != previous_key) {
+         kiter->first != prev_omap_key) {
         name_entry_map[kiter->first] = entry;
-       previous_key = kiter->first;
+       prev_omap_key = kiter->first;
        CLS_LOG(20, "%s: got object entry %s[%s] num entries=%d\n",
                __func__, key.name.c_str(), key.instance.c_str(),
                int(name_entry_map.size()));
@@ -624,12 +636,22 @@ int rgw_bucket_list(cls_method_context_t hctx, bufferlist *in, bufferlist *out)
   } // for (int attempt...
 
   ret.is_truncated = more && !done;
+  if (ret.is_truncated) {
+    ret.marker = start_after_entry_key;
+  }
   CLS_LOG(20, "%s: normal exit returning %ld entries, is_truncated=%d\n",
          __func__, ret.dir.m.size(), ret.is_truncated);
   encode(ret, *out);
-  return 0;
+
+  if (ret.is_truncated && name_entry_map.size() == 0) {
+    CLS_LOG(5, "%s: returning value RGWBIAdvanceAndRetryError\n", __func__);
+    return RGWBIAdvanceAndRetryError;
+  } else {
+    return 0;
+  }
 } // rgw_bucket_list
 
+
 static int check_index(cls_method_context_t hctx,
                       rgw_bucket_dir_header *existing_header,
                       rgw_bucket_dir_header *calc_header)

diff --git a/src/cls/rgw/cls_rgw_client.cc b/src/cls/rgw/cls_rgw_client.cc
index 380afee40c9f2a2832395fc218208eee968c5b95..aeda1a947ca9c7ec5ec8905f01d02d3b7a9197a2 100644 (file)
--- a/src/cls/rgw/cls_rgw_client.cc
+++ b/src/cls/rgw/cls_rgw_client.cc
@@ -340,12 +340,34 @@ static bool issue_bucket_list_op(librados::IoCtx& io_ctx,
 
 int CLSRGWIssueBucketList::issue_op(const int shard_id, const string& oid)
 {
+  // set the marker depending on whether we've already queried this
+  // shard and gotten a RGWBIAdvanceAndRetryError (defined
+  // constant) return value; if we have use the marker in the return
+  // to advance the search, otherwise use the marker passed in by the
+  // caller
+  cls_rgw_obj_key marker;
+  auto iter = result.find(shard_id);
+  if (iter != result.end()) {
+    marker = iter->second.marker;
+  } else {
+    marker = start_obj;
+  }
+
   return issue_bucket_list_op(io_ctx, shard_id, oid,
-                             start_obj, filter_prefix, delimiter,
+                             marker, filter_prefix, delimiter,
                              num_entries, list_versions, &manager,
                              &result[shard_id]);
 }
 
+
+void CLSRGWIssueBucketList::reset_container(std::map<int, std::string>& objs)
+{
+  objs_container.swap(objs);
+  iter = objs_container.begin();
+  objs.clear();
+}
+
+
 void cls_rgw_remove_obj(librados::ObjectWriteOperation& o, list<string>& keep_attr_prefixes)
 {
   bufferlist in;

diff --git a/src/cls/rgw/cls_rgw_client.h b/src/cls/rgw/cls_rgw_client.h
index cfaab7c389d4262c39b1426200dd51d9f1e435e9..ba16061d862ba104e83ae88d964a3d8c32c32997 100644 (file)
--- a/src/cls/rgw/cls_rgw_client.h
+++ b/src/cls/rgw/cls_rgw_client.h
@@ -14,6 +14,7 @@
 #include "common/ceph_time.h"
 #include "common/ceph_mutex.h"
 
+
 // Forward declaration
 class BucketIndexAioManager;
 /*
@@ -424,6 +425,7 @@ class CLSRGWIssueBucketList : public CLSRGWConcurrentIO {
 
 protected:
   int issue_op(int shard_id, const std::string& oid) override;
+  void reset_container(std::map<int, std::string>& objs) override;
 
 public:
   CLSRGWIssueBucketList(librados::IoCtx& io_ctx,

diff --git a/src/cls/rgw/cls_rgw_ops.h b/src/cls/rgw/cls_rgw_ops.h
index 3c248865b8f7f945183a8425e6c23a5ccd4555cb..5510c49f36c90218517950f860f63ebf8287dca9 100644 (file)
--- a/src/cls/rgw/cls_rgw_ops.h
+++ b/src/cls/rgw/cls_rgw_ops.h
@@ -428,6 +428,12 @@ struct rgw_cls_list_ret {
   rgw_bucket_dir dir;
   bool is_truncated;
 
+  // if is_truncated is true, starting marker for next iteration; this
+  // is necessary as it's possible after maximum number of tries we
+  // still might have zero entries to return, in which case we have to
+  // at least move the ball foward
+  cls_rgw_obj_key marker;
+
   // cls_filtered is not transmitted; it is assumed true for versions
   // on/after 3 and false for prior versions; this allows the rgw
   // layer to know when an older osd (cls) does not do the filtering
@@ -439,16 +445,20 @@ struct rgw_cls_list_ret {
   {}
 
   void encode(bufferlist &bl) const {
-    ENCODE_START(3, 2, bl);
+    ENCODE_START(4, 2, bl);
     encode(dir, bl);
     encode(is_truncated, bl);
+    encode(marker, bl);
     ENCODE_FINISH(bl);
   }
   void decode(bufferlist::const_iterator &bl) {
-    DECODE_START_LEGACY_COMPAT_LEN(3, 2, 2, bl);
+    DECODE_START_LEGACY_COMPAT_LEN(4, 2, 2, bl);
     decode(dir, bl);
     decode(is_truncated, bl);
     cls_filtered = struct_v >= 3;
+    if (struct_v >= 4) {
+      decode(marker, bl);
+    }
     DECODE_FINISH(bl);
   }
   void dump(Formatter *f) const;

diff --git a/src/cls/rgw/cls_rgw_types.h b/src/cls/rgw/cls_rgw_types.h
index 3f5b41a4753f33a343fc7ceb28d5832d41ad9dbe..0f646c049c35680fc1d63468ed78e6b3f2838d37 100644 (file)
--- a/src/cls/rgw/cls_rgw_types.h
+++ b/src/cls/rgw/cls_rgw_types.h
@@ -338,6 +338,7 @@ struct rgw_bucket_entry_ver {
 };
 WRITE_CLASS_ENCODER(rgw_bucket_entry_ver)
 
+
 struct cls_rgw_obj_key {
   string name;
   string instance;
@@ -356,6 +357,7 @@ struct cls_rgw_obj_key {
 
   void set(const std::string& _name) {
     name = _name;
+    instance.clear();
   }
 
   bool operator==(const cls_rgw_obj_key& k) const {

diff --git a/src/rgw/rgw_rados.cc b/src/rgw/rgw_rados.cc
index 9035cc0aa5feadfd85f9fc7d889a5a9b53517097..4fe0068fc9836c7e421efb0c9499b25c75e3c0e8 100644 (file)
--- a/src/rgw/rgw_rados.cc
+++ b/src/rgw/rgw_rados.cc
@@ -8425,7 +8425,7 @@ int RGWRados::cls_bucket_list_ordered(RGWBucketInfo& bucket_info,
   // (key=candidate, value=index into results_trackers); as we consume
   // entries from shards, we replace them with the next entries in the
   // shards until we run out
-  map<string, size_t> candidates;
+  std::map<std::string, size_t> candidates;
   size_t tracker_idx = 0;
   for (auto& t : results_trackers) {
     // it's important that the values in the map refer to the index