Merge pull request ceph#63761 from aclamk/aclamk-bs-bluefs-various-fixes

os/bluestore: Set of various fixes to bluefs

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

Author: tchaikov

src/os/bluestore/BlueFS.cc

@@ -4695,7 +4695,6 @@ void BlueFS::_drain_writer(FileWriter *h)
     if (bdev[i]) {
       if (h->iocv[i]) {
 	h->iocv[i]->aio_wait();
-	delete h->iocv[i];
       }
     }
   }

src/os/bluestore/BlueFS.h

@@ -434,6 +434,9 @@ class BlueFS {
     // NOTE: caller must call BlueFS::close_writer()
     ~FileWriter() {
       --file->num_writers;
+      for (unsigned i = 0; i < MAX_BDEV; ++i) {
+        delete iocv[i];
+      }
     }
 
     // note: BlueRocksEnv uses this append exclusively, so it's safe

src/os/bluestore/bluefs_types.cc

@@ -190,9 +190,6 @@ void bluefs_super_t::encode(bufferlist& bl) const
   encode(block_size, bl);
   encode(log_fnode, bl);
   encode(memorized_layout, bl);
-  if (_version >= 3) {
-    encode(wal_version, bl);
-  }
   ENCODE_FINISH(bl);
 }
 
@@ -208,9 +205,6 @@ void bluefs_super_t::decode(bufferlist::const_iterator& p)
   if (struct_v >= 2) {
     decode(memorized_layout, p);
   }
-  if (struct_v >= 3) {
-    decode(wal_version, p);
-  }
   DECODE_FINISH(p);
 }
 

src/os/bluestore/bluefs_types.h

@@ -328,8 +328,6 @@ struct bluefs_super_t {
 
   std::optional<bluefs_layout_t> memorized_layout;
 
-  uint8_t wal_version;
-
   bluefs_super_t();
 
   uint64_t block_mask() const {

src/test/objectstore/test_bluefs.cc

@@ -1137,7 +1137,7 @@ TEST(BlueFS, test_wal_migrate) {
   gen_debugable(70000, bl1);
   fs.append_try_flush(writer, bl1.c_str(), bl1.length());
   fs.fsync(writer);
-
+  fs.close_writer(writer);
   // WAL files don't update internal extents while writing to save memory, only on _replay
   fs.umount();
   fs.mount();
@@ -1435,6 +1435,7 @@ TEST(BlueFS, test_wal_read_after_rollback_to_v1) {
   gen_debugable(70000, bl1);
   fs.append_try_flush(writer, bl1.c_str(), bl1.length());
   fs.fsync(writer);
+  fs.close_writer(writer);
 
   g_ceph_context->_conf.set_val("bluefs_wal_envelope_mode", "false");
   fs.umount();
@@ -1454,6 +1455,7 @@ TEST(BlueFS, test_wal_read_after_rollback_to_v1) {
     ASSERT_EQ(0, fs.open_for_write(dir_db, wal_file, &writer, false));
     ASSERT_NE(nullptr, writer);
     ASSERT_EQ(writer->file->fnode.encoding, bluefs_node_encoding::PLAIN);
+    fs.close_writer(writer);
   }
   fs.umount();
 }
@@ -1671,6 +1673,7 @@ TEST(BlueFS, truncate_fsync) {
         fs.fsync(h);
         fs.close_writer(h);
       }
+      fs.umount();
     }
     {
       //this was broken due to https://tracker.ceph.com/issues/55307
@@ -1703,7 +1706,7 @@ TEST(BlueFS, test_shared_alloc) {
                       size, shared_alloc_unit, "test shared allocator"),
     shared_alloc_unit);
   shared_alloc.a->init_add_free(0, size);
-
+  auto sg = make_scope_guard([&shared_alloc] { delete shared_alloc.a; });
   BlueFS fs(g_ceph_context);
   // DB device is fully utilized
   ASSERT_EQ(0, fs.add_block_device(BlueFS::BDEV_DB, bdev_db.path, false));
@@ -1787,6 +1790,7 @@ TEST(BlueFS, test_shared_alloc_sparse) {
   for(uint64_t i = 5 * bluefs_alloc_unit; i < size; i += 2 * main_unit) {
     shared_alloc.a->init_add_free(i, main_unit);
   }
+  auto sg = make_scope_guard([&shared_alloc] { delete shared_alloc.a; });
 
   BlueFS fs(g_ceph_context);
   ASSERT_EQ(0, fs.add_block_device(BlueFS::BDEV_DB, bdev_slow.path, false,
@@ -1863,6 +1867,7 @@ TEST(BlueFS, test_4k_shared_alloc) {
                       size, main_unit, "test shared allocator"),
     main_unit);
   shared_alloc.a->init_add_free(bluefs_alloc_unit, size - bluefs_alloc_unit);
+  auto sg = make_scope_guard([&shared_alloc] { delete shared_alloc.a; });
 
   BlueFS fs(g_ceph_context);
   ASSERT_EQ(0, fs.add_block_device(BlueFS::BDEV_DB, bdev_slow.path, false,
@@ -2125,6 +2130,7 @@ TEST(BlueFS, test_log_runway) {
   utime_t mtime;
   fs.stat("dir", "file", &file_size, &mtime);
   ASSERT_EQ(file_size, 9);
+  fs.umount();
 }
 
 TEST(BlueFS, test_log_runway_2) {
@@ -2182,6 +2188,7 @@ TEST(BlueFS, test_log_runway_2) {
   std::vector<std::string> ls_longdir;
   fs.readdir(longdir, &ls_longdir);
   ASSERT_EQ(ls_longdir.front(), longfile);
+  fs.umount();
 }
 
 TEST(BlueFS, test_log_runway_3) {
@@ -2242,6 +2249,7 @@ TEST(BlueFS, test_log_runway_3) {
     fs.stat(longdir, longfile, &file_size, &mtime);
     ASSERT_EQ(file_size, 6);
   }
+  fs.umount();
 }
 
 TEST(BlueFS, test_log_runway_advance_seq) {
@@ -2266,6 +2274,7 @@ TEST(BlueFS, test_log_runway_advance_seq) {
   std::string longdir(max_log_runway*2, 'A');
   ASSERT_EQ(fs.mkdir(longdir), 0);
   fs.compact_log();
+  fs.umount();
 }
 
 TEST(BlueFS, test_69481_truncate_corrupts_log) {