Issue 45: Volume Destroy Implementation and its test case (#71)

* Issue 45: Volume Destroy Implementation and its test case

Author: yamingk

conanfile.py

@@ -9,7 +9,7 @@
 
 class HomeBlocksConan(ConanFile):
     name = "homeblocks"
-    version = "0.0.6"
+    version = "0.0.7"
 
     homepage = "https://github.com/eBay/HomeBlocks"
     description = "Block Store built on HomeStore"
@@ -44,9 +44,9 @@ def build_requirements(self):
         self.test_requires("gtest/1.14.0")
 
     def requirements(self):
-        self.requires("homestore/[^6.5]@oss/master")
+        self.requires("homestore/[~6.9.0]@oss/master")
         self.requires("iomgr/[^11.3]@oss/master")
-        self.requires("sisl/[^12.3, include_prerelease=True]@oss/master", transitive_headers=True)
+        self.requires("sisl/[^12.2]@oss/master", transitive_headers=True)
         self.requires("lz4/1.9.4", override=True)
 
     def validate(self):

src/lib/listener.cpp

@@ -28,8 +28,9 @@ bool HBListener::on_pre_commit(int64_t lsn, const sisl::blob& header, const sisl
 void HBListener::on_error(homestore::ReplServiceError error, const sisl::blob& header, const sisl::blob& key,
                           cintrusive< homestore::repl_req_ctx >& ctx) {}
 
-homestore::ReplResult< homestore::blk_alloc_hints > HBListener::get_blk_alloc_hints(sisl::blob const& header,
-                                                                                    uint32_t data_size) {
+homestore::ReplResult< homestore::blk_alloc_hints >
+HBListener::get_blk_alloc_hints(sisl::blob const& header, uint32_t data_size,
+                                cintrusive< homestore::repl_req_ctx >& hs_ctx) {
     return homestore::blk_alloc_hints();
 }
 

src/lib/listener.hpp

@@ -38,8 +38,9 @@ class HBListener : public homestore::ReplDevListener {
     void on_error(homestore::ReplServiceError error, const sisl::blob& header, const sisl::blob& key,
                   cintrusive< homestore::repl_req_ctx >& ctx) override;
 
-    homestore::ReplResult< homestore::blk_alloc_hints > get_blk_alloc_hints(sisl::blob const& header,
-                                                                            uint32_t data_size) override;
+    homestore::ReplResult< homestore::blk_alloc_hints >
+    get_blk_alloc_hints(sisl::blob const& header, uint32_t data_size,
+                        cintrusive< homestore::repl_req_ctx >& hs_ctx) override;
     // group_id is the uuid generated by HomeBlocks when create_volume->create_repl_dev(gid) is called;
     // when volume is being destroyed, on_destroy is going to be triggered to listener that this volume's gid is
     // destroyed;

src/lib/volume/tests/test_volume.cpp

@@ -48,6 +48,38 @@ class VolumeTest : public ::testing::Test {
     }
 };
 
+TEST_F(VolumeTest, CreateDestroyVolume) {
+    std::vector< volume_id_t > vol_ids;
+    {
+        auto hb = g_helper->inst();
+        auto vol_mgr = hb->volume_manager();
+
+        auto num_vols = SISL_OPTIONS["num_vols"].as< uint32_t >();
+
+        for (uint32_t i = 0; i < num_vols; ++i) {
+            auto vinfo = gen_vol_info(i);
+            auto id = vinfo.id;
+            vol_ids.emplace_back(id);
+            auto ret = vol_mgr->create_volume(std::move(vinfo)).get();
+            ASSERT_TRUE(ret);
+
+            auto vinfo_ptr = vol_mgr->lookup_volume(id);
+            // verify the volume is there
+            ASSERT_TRUE(vinfo_ptr != nullptr);
+        }
+
+        for (uint32_t i = 0; i < num_vols; ++i) {
+            auto id = vol_ids[i];
+            auto ret = vol_mgr->remove_volume(id).get();
+            ASSERT_TRUE(ret);
+
+            auto vinfo_ptr = vol_mgr->lookup_volume(id);
+            // verify the volume is not there
+            ASSERT_TRUE(vinfo_ptr == nullptr);
+        }
+    }
+}
+
 TEST_F(VolumeTest, CreateVolumeThenRecover) {
     std::vector< volume_id_t > vol_ids;
     {

src/lib/volume/volume.cpp

@@ -84,4 +84,24 @@ bool Volume::init(bool is_recovery) {
     return true;
 }
 
+void Volume::destroy() {
+    // destroy the repl dev;
+    if (rd_) {
+        LOGI("Destroying repl dev for volume: {}, uuid: {}", vol_info_->name, boost::uuids::to_string(id()));
+        homestore::hs()->repl_service().remove_repl_dev(id()).get();
+        rd_ = nullptr;
+    }
+
+    // destroy the index table;
+    if (indx_tbl_) {
+        LOGI("Destroying index table for volume: {}, uuid: {}", vol_info_->name, boost::uuids::to_string(id()));
+        homestore::hs()->index_service().remove_index_table(indx_tbl_);
+        indx_tbl_->destroy();
+        indx_tbl_ = nullptr;
+    }
+
+    // destroy the superblock which will remove sb from meta svc;
+    sb_.destroy();
+}
+
 } // namespace homeblocks

src/lib/volume/volume.hpp

@@ -71,7 +71,6 @@ class Volume {
     Volume& operator=(Volume const& volume) = delete;
     Volume& operator=(Volume&& volume) = default;
 
-    // TODO: volume destructor should remove volume meta block;
     virtual ~Volume() = default;
 
     // static APIs exposed to HomeBlks Implementation Layer;
@@ -100,6 +99,8 @@ class Volume {
     //
     shared< VolumeIndexTable > init_index_table(bool is_recovery, shared< VolumeIndexTable > tbl = nullptr);
 
+    void destroy();
+
 private:
     //
     // this API will be called to initialize volume in both volume creation and volume recovery;

src/lib/volume_mgr.cpp

@@ -80,7 +80,38 @@ VolumeManager::NullAsyncResult HomeBlocksImpl::create_volume(VolumeInfo&& vol_in
     return folly::Unit();
 }
 
-VolumeManager::NullAsyncResult HomeBlocksImpl::remove_volume(const volume_id_t& id) { return folly::Unit(); }
+VolumeManager::NullAsyncResult HomeBlocksImpl::remove_volume(const volume_id_t& id) {
+    LOGI("remove_volume with input id: {}", boost::uuids::to_string(id));
+
+    std::string vol_id_str;
+    // 1. remove destroy volume and remove volume from vol_map;
+    {
+        auto lg = std::scoped_lock(vol_lock_);
+        if (auto it = vol_map_.find(id); it != vol_map_.end()) {
+            auto vol_ptr = it->second;
+            vol_id_str = vol_ptr->id_str();
+            vol_map_.erase(it);
+            vol_ptr->destroy();
+            LOGI("Volume {} removed successfully", boost::uuids::to_string(id));
+        } else {
+            LOGW("remove_volume with input id: {} not found", boost::uuids::to_string(id));
+            return folly::makeUnexpected(VolumeError::INVALID_ARG);
+        }
+    }
+
+    // 2. remove index handler from index_map if it exists;
+    {
+        auto lg = std::scoped_lock(index_lock_);
+        auto it = idx_tbl_map_.find(vol_id_str);
+        if (it != idx_tbl_map_.end()) {
+            idx_tbl_map_.erase(it);
+            LOGI("Volume {} index table removed successfully", boost::uuids::to_string(id));
+        }
+    }
+
+    // Volume Destructor will be called after vol_ptr goes out of scope;
+    return folly::Unit();
+}
 
 VolumeInfoPtr HomeBlocksImpl::lookup_volume(const volume_id_t& id) {
     auto lg = std::shared_lock(vol_lock_);