os/bluestore: No garbage collection for uncompressed blobs

Signed-off-by: Roushan Ali<roushan.ali@sandisk.com>

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index 3f81a132a8b982a686150a5b2b3429d230205e7a..e1fd1f8b32113814b6d47a9536f74b0c65e2e4a4 100644 (file)
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -2209,18 +2209,20 @@ bool BlueStore::ExtentMap::do_write_check_depth(
   auto hp = seek_lextent(start_offset);
   if (hp != extent_map.end() &&
     hp->logical_offset < start_offset &&
-    start_offset < hp->logical_offset + hp->length) {
-    depth = hp->blob_depth;
-    head_overlap = true;
+    start_offset < (hp->logical_offset + hp->length) &&
+    hp->blob->get_blob().is_compressed()) {
+      depth = hp->blob_depth;
+      head_overlap = true;
   }
 
   auto tp = seek_lextent(end_offset);
   if (tp != extent_map.end() &&
     tp->logical_offset < end_offset &&
-    end_offset < tp->logical_offset + tp->length) {
-    tail_overlap = true;
-    if (depth < tp->blob_depth) {
-      depth = tp->blob_depth;
+    end_offset < (tp->logical_offset + tp->length) &&
+    tp->blob->get_blob().is_compressed()) {
+      tail_overlap = true;
+      if (depth < tp->blob_depth) {
+        depth = tp->blob_depth;
     }
   }
 

diff --git a/src/test/objectstore/store_test.cc b/src/test/objectstore/store_test.cc
index f219566ef7bb3fe01d9ff45e30228b9de2e1d57c..77149c8e2cbabcaa330204a54a20d1a0fd69d01e 100644 (file)
--- a/src/test/objectstore/store_test.cc
+++ b/src/test/objectstore/store_test.cc
@@ -937,10 +937,19 @@ TEST_P(StoreTest, garbageCollection) {
   int64_t waste1, waste2;
   coll_t cid;
   int buf_len = 256 * 1024;
+  int overlap_offset = 64 * 1024;
+  int write_offset = buf_len;
   if (string(GetParam()) != "bluestore")
     return;
 
-  g_conf->set_val("bluestore_compression", "force");
+#define WRITE_AT(offset, length) {\
+      ObjectStore::Transaction t;\
+      t.write(cid, hoid, offset, length, bl);\
+      r = apply_transaction(store, &osr, std::move(t));\
+      ASSERT_EQ(r, 0);\
+  }
+  g_conf->set_val("bluestore_compression", "none");
+  //g_conf->set_val("bluestore_compression", "force");
   g_conf->set_val("bluestore_merge_gc_data", "true"); 
   g_ceph_context->_conf->apply_changes(NULL);
 
@@ -981,33 +990,18 @@ TEST_P(StoreTest, garbageCollection) {
       data[i] = 'R';
 
     bl.append(data);
+
+    WRITE_AT(0, buf_len);
+    WRITE_AT(write_offset - 3 * overlap_offset, buf_len);
+    WRITE_AT(write_offset - 2 * overlap_offset, buf_len);
     {
-      ObjectStore::Transaction t;
-      t.write(cid, hoid, 0, bl.length(), bl);
-      r = apply_transaction(store, &osr, std::move(t));
-      ASSERT_EQ(r, 0);
-    }
-    {
-      ObjectStore::Transaction t;
-      t.write(cid, hoid, buf_len - 4096, bl.length(), bl);
-      r = apply_transaction(store, &osr, std::move(t));
-      ASSERT_EQ(r, 0);
-    }
-    {
-      ObjectStore::Transaction t;
-      t.write(cid, hoid, 2 * (buf_len - 4096), bl.length(), bl);
-      r = apply_transaction(store, &osr, std::move(t));
-      ASSERT_EQ(r, 0);
       struct store_statfs_t statfs;
       int r = store->statfs(&statfs);
       ASSERT_EQ(r, 0);
       waste1 = statfs.allocated - statfs.stored;
     }
+    WRITE_AT(write_offset - overlap_offset, buf_len);
     {
-      ObjectStore::Transaction t;
-      t.write(cid, hoid, 3 * (buf_len - 4096), bl.length(), bl);
-      r = apply_transaction(store, &osr, std::move(t));
-      ASSERT_EQ(r, 0);
       struct store_statfs_t statfs;
       int r = store->statfs(&statfs);
       ASSERT_EQ(r, 0);
@@ -1042,36 +1036,16 @@ TEST_P(StoreTest, garbageCollection) {
       data[i] = i  % 256;
     bl.append(data);
 
-    {
-      ObjectStore::Transaction t;
-      t.write(cid, hoid, 3 * (buf_len - 4096), bl.length(), bl);
-      r = apply_transaction(store, &osr, std::move(t));
-      ASSERT_EQ(r, 0);
-    }
-    {
-      ObjectStore::Transaction t;
-      t.write(cid, hoid, 2 * (buf_len - 4096), bl.length(), bl);
-      r = apply_transaction(store, &osr, std::move(t));
-      ASSERT_EQ(r, 0);
-    }
-    {
-      ObjectStore::Transaction t;
-      t.write(cid, hoid, buf_len - 4096, bl.length(), bl);
-      r = apply_transaction(store, &osr, std::move(t));
-      ASSERT_EQ(r, 0);
-    }
+    WRITE_AT(write_offset - overlap_offset, buf_len);
+    WRITE_AT(write_offset - 2 * overlap_offset, buf_len);
+    WRITE_AT(write_offset - 3 * overlap_offset, buf_len);
     {
       struct store_statfs_t statfs;
       int r = store->statfs(&statfs);
       ASSERT_EQ(r, 0);
       waste1 = statfs.allocated - statfs.stored;
     }
-    {
-      ObjectStore::Transaction t;
-      t.write(cid, hoid, 50 * 1024, bl.length(), bl);
-      r = apply_transaction(store, &osr, std::move(t));
-      ASSERT_EQ(r, 0);
-    }
+    WRITE_AT(0, buf_len);
     {
       struct store_statfs_t statfs;
       int r = store->statfs(&statfs);
@@ -1106,48 +1080,10 @@ TEST_P(StoreTest, garbageCollection) {
       data[i] = i  % 256;
     bl.append(data);
 
-    {
-      ObjectStore::Transaction t;
-      t.write(cid, hoid, 5 * (buf_len - 4096), bl.length(), bl);
-      r = apply_transaction(store, &osr, std::move(t));
-      ASSERT_EQ(r, 0);
-    }
-    {
-      ObjectStore::Transaction t;
-      t.write(cid, hoid, 4 * (buf_len - 4096), bl.length(), bl);
-      r = apply_transaction(store, &osr, std::move(t));
-      ASSERT_EQ(r, 0);
-    }
-    {
-      ObjectStore::Transaction t;
-      t.write(cid, hoid, 3 * (buf_len - 4096), bl.length(), bl);
-      r = apply_transaction(store, &osr, std::move(t));
-      ASSERT_EQ(r, 0);
-    }
-    {
-      ObjectStore::Transaction t;
-      t.write(cid, hoid, buf_len - 4096, bl.length(), bl);
-      r = apply_transaction(store, &osr, std::move(t));
-      ASSERT_EQ(r, 0);
-    }
-    {
-      ObjectStore::Transaction t;
-      t.write(cid, hoid, 40 * 1024, bl.length(), bl);
-      r = apply_transaction(store, &osr, std::move(t));
-      ASSERT_EQ(r, 0);
-    }
-    {
-      struct store_statfs_t statfs;
-      int r = store->statfs(&statfs);
-      ASSERT_EQ(r, 0);
-      waste1 = statfs.allocated - statfs.stored;
-    }
-    {
-      ObjectStore::Transaction t;
-      t.write(cid, hoid, 5 * (buf_len - 3 * 4096), bl.length(), bl);
-      r = apply_transaction(store, &osr, std::move(t));
-      ASSERT_EQ(r, 0);
-    }
+    WRITE_AT(2 * write_offset - 5 * overlap_offset, buf_len);
+    WRITE_AT(2 * write_offset - 4 * overlap_offset, buf_len);
+    WRITE_AT(2 * write_offset - 3 * overlap_offset, buf_len);
+    WRITE_AT(2 * overlap_offset, buf_len);
     {
       struct store_statfs_t statfs;
       int r = store->statfs(&statfs);