os/bluestore: be smarter about picking a zone to clean

Base our selection on best "value" (benefit / cost).

Signed-off-by: Sage Weil <sage@newdream.net>

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index c0dfcd36ea4848f9095f7583e68ed9b69d6a01e9..db29a98b3390b77986a2e94944b1631edb798e7b 100644 (file)
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -13070,7 +13070,7 @@ void BlueStore::_zoned_cleaner_thread()
   auto f = dynamic_cast<ZonedFreelistManager*>(fm);
   ceph_assert(f);
   while (true) {
-    auto zone_to_clean = a->pick_zone_to_clean();
+    auto zone_to_clean = a->pick_zone_to_clean(.1, zone_size / 16); // FIXME
     if (zone_to_clean < 0) {
       if (zoned_cleaner_stop) {
        break;

diff --git a/src/os/bluestore/ZonedAllocator.cc b/src/os/bluestore/ZonedAllocator.cc
index 8438c65c643cd0e725da8b0ceeb4317aa243a9da..7feb6f3e014a1d4d85cb15db58355f75945cb885 100644 (file)
--- a/src/os/bluestore/ZonedAllocator.cc
+++ b/src/os/bluestore/ZonedAllocator.cc
@@ -167,25 +167,48 @@ void ZonedAllocator::init_from_zone_pointers(
                 << dendl;
 }
 
-int64_t ZonedAllocator::pick_zone_to_clean(void)
+int64_t ZonedAllocator::pick_zone_to_clean(float min_score, uint64_t min_saved)
 {
+  std::lock_guard l(lock);
   int32_t best = -1;
-  int64_t best_score = 0;
+  float best_score = 0.0;
   for (size_t i = first_seq_zone_num; i < num_zones; ++i) {
-    int64_t score = zone_states[i].num_dead_bytes;
-    // discount by remaining space so we will tend to clean full zones
-    score -= (zone_size - zone_states[i].write_pointer) / 2;
-    if (score > 0 && (best < 0 || score > best_score)) {
+    // value (score) = benefit / cost
+    //    benefit = how much net free space we'll get (dead bytes)
+    //    cost = how many bytes we'll have to rewrite (live bytes)
+    // avoid divide by zero on a zone with no live bytes
+    float score =
+      (float)zone_states[i].num_dead_bytes /
+      (float)(zone_states[i].get_num_live_bytes() + 1);
+    if (score > 0) {
+      ldout(cct, 20) << " zone 0x" << std::hex << i
+                    << " dead 0x" << zone_states[i].num_dead_bytes
+                    << " score " << score
+                    << dendl;
+    }
+    if (zone_states[i].num_dead_bytes < min_saved) {
+      continue;
+    }
+    if (best < 0 || score > best_score) {
       best = i;
       best_score = score;
     }
   }
-  if (best >= 0) {
-    ldout(cct, 10) << " zone 0x" << std::hex << best << " with score 0x" << best_score
+  if (best_score >= min_score) {
+    ldout(cct, 10) << " zone 0x" << std::hex << best << " with score " << best_score
                   << ": 0x" << zone_states[best].num_dead_bytes
                   << " dead and 0x"
                   << zone_states[best].write_pointer - zone_states[best].num_dead_bytes
                   << " live bytes" << std::dec << dendl;
+  } else if (best > 0) {
+    ldout(cct, 10) << " zone 0x" << std::hex << best << " with score " << best_score
+                  << ": 0x" << zone_states[best].num_dead_bytes
+                  << " dead and 0x"
+                  << zone_states[best].write_pointer - zone_states[best].num_dead_bytes
+                  << " live bytes" << std::dec
+                  << " but below min_score " << min_score
+                  << dendl;
+    best = -1;
   } else {
     ldout(cct, 10) << " no zones found that are good cleaning candidates" << dendl;
   }

diff --git a/src/os/bluestore/ZonedAllocator.h b/src/os/bluestore/ZonedAllocator.h
index ab4cc27707a5cf8ecb7847d026907960b178dd8d..9f5b632eb93a5318c8f20c8621154b1d27311736 100644 (file)
--- a/src/os/bluestore/ZonedAllocator.h
+++ b/src/os/bluestore/ZonedAllocator.h
@@ -79,6 +79,9 @@ public:
   uint64_t get_dead_bytes(uint32_t zone) {
     return zone_states[zone].num_dead_bytes;
   }
+  uint64_t get_live_bytes(uint32_t zone) {
+    return zone_states[zone].write_pointer - zone_states[zone].num_dead_bytes;
+  }
 
   int64_t allocate(
     uint64_t want_size, uint64_t alloc_unit, uint64_t max_alloc_size,
@@ -92,7 +95,7 @@ public:
   void dump(std::function<void(uint64_t offset,
                                uint64_t length)> notify) override;
 
-  int64_t pick_zone_to_clean(void);
+  int64_t pick_zone_to_clean(float min_score, uint64_t min_saved);
   void set_cleaning_zone(uint32_t zone) {
     cleaning_zone = zone;
   }

diff --git a/src/os/bluestore/zoned_types.h b/src/os/bluestore/zoned_types.h
index 93d3138399a0c0d7f965a91224bb6f8140a906d3..5507e64c7c58c908562a06a2ef0ab7dbb09ba5d4 100644 (file)
--- a/src/os/bluestore/zoned_types.h
+++ b/src/os/bluestore/zoned_types.h
@@ -32,6 +32,10 @@ struct zone_state_t {
     return num_dead_bytes;
   }
 
+  uint64_t get_num_live_bytes() const {
+    return write_pointer - num_dead_bytes;
+  }
+
   uint64_t get_write_pointer() const {
     return write_pointer;
   }