Merge pull request ceph#49438 from cfsnyder/wip-58274-bluestore-collection-list-bounds

os/bluestore: set rocksdb iterator bounds for Bluestore::_collection_list()

Reviewed-by: Igor Fedotov <[email protected]>
Reviewed-by: Adam Kupczyk <[email protected]>

Author: yuriw

src/os/bluestore/BlueStore.cc

@@ -12676,16 +12676,14 @@ int BlueStore::_collection_list(
   Collection *c, const ghobject_t& start, const ghobject_t& end, int max,
   bool legacy, vector<ghobject_t> *ls, ghobject_t *pnext)
 {
-
   if (!c->exists)
     return -ENOENT;
 
   ghobject_t static_next;
   std::unique_ptr<CollectionListIterator> it;
   ghobject_t coll_range_temp_start, coll_range_temp_end;
   ghobject_t coll_range_start, coll_range_end;
-  ghobject_t pend;
-  bool temp;
+  std::vector<std::tuple<ghobject_t, ghobject_t>> ranges;
 
   if (!pnext)
     pnext = &static_next;
@@ -12719,82 +12717,59 @@ int BlueStore::_collection_list(
     << " and " << coll_range_start
     << " to " << coll_range_end
     << " start " << start << dendl;
-  if (legacy) {
-    it = std::make_unique<SimpleCollectionListIterator>(
-      cct, db->get_iterator(PREFIX_OBJ));
-  } else {
-    it = std::make_unique<SortedCollectionListIterator>(
-      db->get_iterator(PREFIX_OBJ));
-  }
-  if (start == ghobject_t() ||
-    start.hobj == hobject_t() ||
-    start == c->cid.get_min_hobj()) {
-    it->upper_bound(coll_range_temp_start);
-    temp = true;
-  } else {
-    if (start.hobj.is_temp()) {
-      temp = true;
-      ceph_assert(start >= coll_range_temp_start && start < coll_range_temp_end);
-    } else {
-      temp = false;
-      ceph_assert(start >= coll_range_start && start < coll_range_end);
-    }
-    dout(20) << __func__ << " temp=" << (int)temp << dendl;
-    it->lower_bound(start);
+
+  // if specified start is not specifically in the pg normal range, we should start with temp iter
+ if ((start == ghobject_t() ||
+      start.hobj == hobject_t() ||
+      start == c->cid.get_min_hobj() ||
+      start.hobj.is_temp())
+    && coll_range_temp_start != coll_range_temp_end) {
+    ranges.push_back(std::tuple(std::move(coll_range_temp_start), std::move(coll_range_temp_end)));
   }
-  if (end.hobj.is_max()) {
-    pend = temp ? coll_range_temp_end : coll_range_end;
-  } else {
-    if (end.hobj.is_temp()) {
-      if (temp) {
-        pend = end;
-      } else {
-        *pnext = ghobject_t::get_max();
-        return 0;
-      }
+  // if end param is in temp section, then we do not need to proceed to the normal section
+  if (!end.hobj.is_temp()) {
+    ranges.push_back(std::tuple(std::move(coll_range_start), std::move(coll_range_end)));
+  }
+
+  for (const auto & [cur_range_start, cur_range_end] : ranges) {
+    dout(30) << __func__ << " cur_range " << cur_range_start << " to " << cur_range_end << dendl;
+
+    const ghobject_t low = start > cur_range_start ? start : cur_range_start;
+    const ghobject_t high = end < cur_range_end ? end : cur_range_end;
+    if (low >= high) {
+      continue;
+    }
+
+    std::string kv_low_key, kv_high_key;
+    _key_encode_prefix(low, &kv_low_key);
+    _key_encode_prefix(high, &kv_high_key);
+    kv_high_key.push_back('\xff');
+    dout(30) << __func__ << " kv_low_key: " << kv_low_key << " kv_high_key: " << kv_high_key << dendl;
+    const KeyValueDB::IteratorBounds bounds = KeyValueDB::IteratorBounds{std::move(kv_low_key), std::move(kv_high_key)};
+    if (legacy) {
+      it = std::make_unique<SimpleCollectionListIterator>(
+              cct, db->get_iterator(PREFIX_OBJ, 0, std::move(bounds)));
     } else {
-      pend = temp ? coll_range_temp_end : end;
+      it = std::make_unique<SortedCollectionListIterator>(
+              db->get_iterator(PREFIX_OBJ, 0, std::move(bounds)));
     }
-  }
-  dout(20) << __func__ << " pend " << pend << dendl;
-  while (true) {
-    if (!it->valid() || it->is_ge(pend)) {
-      if (!it->valid())
-	dout(20) << __func__ << " iterator not valid (end of db?)" << dendl;
-      else
-	dout(20) << __func__ << " oid " << it->oid() << " >= " << pend << dendl;
-      if (temp) {
-	if (end.hobj.is_temp()) {
-          if (it->valid() && it->is_lt(coll_range_temp_end)) {
-            *pnext = it->oid();
-            return 0;
-          }
-	  break;
-	}
-	dout(30) << __func__ << " switch to non-temp namespace" << dendl;
-	temp = false;
-	it->upper_bound(coll_range_start);
-        if (end.hobj.is_max())
-          pend = coll_range_end;
-        else
-          pend = end;
-	dout(30) << __func__ << " pend " << pend << dendl;
-	continue;
+    it->lower_bound(low);
+    while (it->valid()) {
+      if (it->oid() < low) {
+        it->next();
+        continue;
+      }
+      if (it->oid() > high) {
+        break;
       }
-      if (it->valid() && it->is_lt(coll_range_end)) {
+      if (ls->size() >= (unsigned)max || it->oid() == high) {
         *pnext = it->oid();
         return 0;
       }
-      break;
-    }
-    dout(20) << __func__ << " oid " << it->oid() << " end " << end << dendl;
-    if (ls->size() >= (unsigned)max) {
-      dout(20) << __func__ << " reached max " << max << dendl;
-      *pnext = it->oid();
-      return 0;
+      dout(20) << __func__ << " oid " << it->oid() << dendl;
+      ls->push_back(it->oid());
+      it->next();
     }
-    ls->push_back(it->oid());
-    it->next();
   }
   *pnext = ghobject_t::get_max();
   return 0;