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>

diff --git a/src/cls/rgw/cls_rgw.cc b/src/cls/rgw/cls_rgw.cc
index 1cca30be4e184aa2476330c50d24e0613bea41a5..d545dada42706305a71b871b907c7ecbe33bdfcc 100644 (file)
--- a/src/cls/rgw/cls_rgw.cc
+++ b/src/cls/rgw/cls_rgw.cc
@@ -183,10 +183,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,
@@ -199,7 +199,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
@@ -208,11 +208,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;
   }
@@ -223,10 +223,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);
@@ -236,11 +236,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,
@@ -496,29 +496,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;
@@ -527,8 +540,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) {
@@ -540,13 +557,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;
@@ -558,7 +568,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());
 
@@ -593,10 +604,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) {
@@ -614,11 +625,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;
@@ -629,9 +641,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()));
@@ -640,12 +652,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 2f1a9164f33b3d615b8d4ccb063e500793dea707..da0fd9d0d572ef3f8b2183649ddcaa451411c771 100644 (file)
--- a/src/cls/rgw/cls_rgw_client.cc
+++ b/src/cls/rgw/cls_rgw_client.cc
@@ -348,12 +348,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 44905671c1372400a6442e33a0e977ebac2f064b..67508b21f277e3e0ad9d331ca587794f91921a26 100644 (file)
--- a/src/cls/rgw/cls_rgw_client.h
+++ b/src/cls/rgw/cls_rgw_client.h
@@ -13,6 +13,7 @@
 #include "common/ceph_time.h"
 #include "common/ceph_mutex.h"
 
+
 // Forward declaration
 class BucketIndexAioManager;
 /*
@@ -425,6 +426,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 38b4fb4b9bb2149b6d692a5c1e05a8c7940bdbc8..a90c5e103a97a1e6b2ed83c543f72d01039d3b68 100644 (file)
--- a/src/cls/rgw/cls_rgw_ops.h
+++ b/src/cls/rgw/cls_rgw_ops.h
@@ -427,6 +427,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
@@ -438,16 +444,20 @@ struct rgw_cls_list_ret {
   {}
 
   void encode(ceph::buffer::list &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(ceph::buffer::list::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(ceph::Formatter *f) const;

diff --git a/src/cls/rgw/cls_rgw_types.h b/src/cls/rgw/cls_rgw_types.h
index ccf19c1824ecff586892bc0a80337900c141b116..2b374355dd21b7c8a3d7974bd3e365c7d0869b4e 100644 (file)
--- a/src/cls/rgw/cls_rgw_types.h
+++ b/src/cls/rgw/cls_rgw_types.h
@@ -331,6 +331,7 @@ struct rgw_bucket_entry_ver {
 };
 WRITE_CLASS_ENCODER(rgw_bucket_entry_ver)
 
+
 struct cls_rgw_obj_key {
   std::string name;
   std::string instance;
@@ -341,6 +342,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 717a34c7a4a48d632b3306981ec7c69c9ccf62be..f2e555f122339958fc67e03074da9517d5c44c11 100644 (file)
--- a/src/rgw/rgw_rados.cc
+++ b/src/rgw/rgw_rados.cc
@@ -8505,7 +8505,7 @@ int RGWRados::cls_bucket_list_ordered(const DoutPrefixProvider *dpp,
   // (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