fix tests

Signed-off-by: Wu, Jiayu <[email protected]>

engine/backup_log.hpp

@@ -275,7 +275,7 @@ class BackupLog {
   std::string file_name_{};
   std::string delta_{};
   void* log_file_{nullptr};
-  ssize_t file_size_{0};
+  uint64_t file_size_{0};
   int fd_{-1};
   BackupStage stage_{BackupStage::NotFinished};
 };

engine/hash_collection/hash_list.cpp

@@ -123,7 +123,7 @@ HashList::WriteResult HashList::Modify(const StringView key,
 
 HashWriteArgs HashList::InitWriteArgs(const StringView& key,
                                       const StringView& value, WriteOp op) {
-  HashWriteArgs args = {};
+  HashWriteArgs args{};
   args.key = key;
   args.value = value;
   args.op = op;

engine/hash_collection/rebuilder.hpp

@@ -73,7 +73,7 @@ class HashListRebuilder {
     }
     std::vector<std::future<Status>> fs;
     size_t i = 0;
-    for (auto& hlist : rebuild_hlists_) {
+    for (auto hlist : rebuild_hlists_) {
       i++;
       fs.push_back(std::async(&HashListRebuilder::rebuildIndex, this,
                               hlist.second.get()));

engine/hash_table.hpp

@@ -256,7 +256,6 @@ class HashTable {
   Array<Slot> slots_;
   std::vector<uint64_t> hash_bucket_entries_;
   Array<HashBucket> hash_buckets_;
-  // void* main_buckets_;
 };
 
 // Iterator all hash entries in a hash table bucket

engine/kv_engine.cpp

@@ -1166,6 +1166,7 @@ Status KVEngine::batchWriteRollbackLogs() {
                        strerror(errno));
     return Status::IOError;
   }
+  defer(closedir(dir));
   dirent* entry;
   while ((entry = readdir(dir)) != NULL) {
     std::string fname = std::string{entry->d_name};
@@ -1180,7 +1181,6 @@ Status KVEngine::batchWriteRollbackLogs() {
     if (addr == NULL) {
       GlobalLogger.Error("Fail to Rollback BatchLog file. %s\n",
                          strerror(errno));
-      closedir(dir);
       return Status::PMemMapFileError;
     }
     kvdk_assert(is_pmem != 0 && mapped_len >= BatchWriteLog::MaxBytes(), "");
@@ -1193,43 +1193,37 @@ Status KVEngine::batchWriteRollbackLogs() {
          ++iter) {
       s = listRollback(*iter);
       if (s != Status::Ok) {
-        closedir(dir);
         return s;
       }
     }
     for (auto iter = log.HashLogs().rbegin(); iter != log.HashLogs().rend();
          ++iter) {
       s = hashListRollback(*iter);
       if (s != Status::Ok) {
-        closedir(dir);
         return s;
       }
     }
     for (auto iter = log.SortedLogs().rbegin(); iter != log.SortedLogs().rend();
          ++iter) {
       s = sortedRollback(*iter);
       if (s != Status::Ok) {
-        closedir(dir);
         return s;
       }
     }
     for (auto iter = log.StringLogs().rbegin(); iter != log.StringLogs().rend();
          ++iter) {
       s = stringRollback(log.Timestamp(), *iter);
       if (s != Status::Ok) {
-        closedir(dir);
         return s;
       }
     }
     log.MarkInitializing(static_cast<char*>(addr));
     if (pmem_unmap(addr, mapped_len) != 0) {
       GlobalLogger.Error("Fail to Rollback BatchLog file. %s\n",
                          strerror(errno));
-      closedir(dir);
       return Status::PMemMapFileError;
     }
   }
-  closedir(dir);
   std::string cmd{"rm -rf " + batch_log_dir_ + "*"};
   [[gnu::unused]] int ret = system(cmd.c_str());
 

engine/kv_engine.hpp

@@ -356,10 +356,11 @@ class KVEngine : public Engine {
                   "Invalid type!");
     return std::is_same<CollectionType, Skiplist>::value
                ? RecordType::SortedRecord
-           : std::is_same<CollectionType, List>::value ? RecordType::ListRecord
-           : std::is_same<CollectionType, HashList>::value
-               ? RecordType::HashRecord
-               : RecordType::Empty;
+               : std::is_same<CollectionType, List>::value
+                     ? RecordType::ListRecord
+                     : std::is_same<CollectionType, HashList>::value
+                           ? RecordType::HashRecord
+                           : RecordType::Empty;
   }
 
   static PointerType pointerType(RecordType rtype) {

engine/kv_engine_cleaner.cpp

@@ -737,9 +737,9 @@ double Cleaner::SearchOutdatedCollections() {
                            outdated_collections_.lists.size() +
                            outdated_collections_.skiplists.size());
 
-  if (max_thread_num_ != 0) {
+  if (max_thread_num_ > 0) {
     double diff_size_ratio =
-        (after_queue_size - before_queue_size) * 1.0 / max_thread_num_;
+        (double)(after_queue_size - before_queue_size) / max_thread_num_;
     if (diff_size_ratio > 0) {
       outdated_collections_.increase_ratio =
           outdated_collections_.increase_ratio == 0

engine/list_collection/rebuilder.hpp

@@ -73,7 +73,7 @@ class ListRebuilder {
     }
     std::vector<std::future<Status>> fs;
     size_t i = 0;
-    for (auto& list : rebuild_lists_) {
+    for (auto list : rebuild_lists_) {
       i++;
       fs.push_back(
           std::async(&ListRebuilder::rebuildIndex, this, list.second.get()));

engine/pmem_allocator/free_list.hpp

@@ -281,7 +281,7 @@ class Freelist {
   const uint64_t num_segment_blocks_;
   const uint32_t block_size_;
   const uint32_t max_small_entry_block_size_;
-  const uint32_t max_block_size_index_ = kMaxBlockSizeIndex;
+  const uint32_t max_block_size_index_;
   SpaceEntryPool active_pool_;
   SpaceEntryPool merged_pool_;
   SpaceMap space_map_;

engine/pmem_allocator/pmem_allocator.cpp

@@ -82,6 +82,7 @@ PMEMAllocator* PMEMAllocator::NewPMEMAllocator(
 
     if (!is_pmem) {
       GlobalLogger.Error("%s is not a pmem path\n", pmem_file.c_str());
+      pmem_unmap(pmem, mapped_size);
       return nullptr;
     }
   } else {
@@ -107,11 +108,19 @@ PMEMAllocator* PMEMAllocator::NewPMEMAllocator(
     }
   }
 
+  auto unmap_pmem = [&]() {
+    if (!use_devdax_mode) {
+      pmem_unmap(pmem, mapped_size);
+    } else {
+      munmap(pmem, pmem_size);
+    }
+  };
+
   if (mapped_size != pmem_size) {
     GlobalLogger.Error(
         "Pmem map file %s size %lu is not same as expected %lu\n",
         pmem_file.c_str(), mapped_size, pmem_size);
-    munmap(pmem, pmem_size);
+    unmap_pmem();
     return nullptr;
   }
 
@@ -121,7 +130,7 @@ PMEMAllocator* PMEMAllocator::NewPMEMAllocator(
     GlobalLogger.Error(
         "pmem file too small, should larger than pmem_segment_blocks * "
         "pmem_block_size * max_access_threads\n");
-    munmap(pmem, pmem_size);
+    unmap_pmem();
     return nullptr;
   }
 
@@ -134,7 +143,7 @@ PMEMAllocator* PMEMAllocator::NewPMEMAllocator(
         "Pmem file size not aligned with segment size, pmem file size is %llu, "
         "segment_size is %llu\n",
         pmem_size, block_size * num_segment_blocks);
-    munmap(pmem, pmem_size);
+    unmap_pmem();
     return nullptr;
   }
 
@@ -148,7 +157,7 @@ PMEMAllocator* PMEMAllocator::NewPMEMAllocator(
   } catch (std::bad_alloc& err) {
     GlobalLogger.Error("Error while initialize PMEMAllocator: %s\n",
                        err.what());
-    munmap(pmem, pmem_size);
+    unmap_pmem();
     return nullptr;
   }
 
@@ -233,7 +242,6 @@ bool PMEMAllocator::checkDevDaxAndGetSize(const char* path, uint64_t* size) {
 
 SpaceEntry PMEMAllocator::Allocate(uint64_t size) {
   SpaceEntry space_entry;
-  memset(&space_entry, 0, sizeof(space_entry));
   uint32_t b_size = size_2_block_size(size);
   uint32_t aligned_size = b_size * block_size_;
   // Now the requested block size should smaller than segment size

engine/sorted_collection/rebuilder.cpp

@@ -552,11 +552,10 @@ Status SortedCollectionRebuilder::rebuildSkiplistIndex(Skiplist* skiplist) {
         }
 
         if (s != Status::Ok) {
-          delete (dram_node);
+          SkiplistNode::DeleteNode(dram_node);
           return s;
         }
       }
-      delete (dram_node);
 
       valid_version_record->PersistOldVersion(kNullPMemOffset);
       splice.prev_pmem_record = valid_version_record;
@@ -569,7 +568,7 @@ Status SortedCollectionRebuilder::rebuildSkiplistIndex(Skiplist* skiplist) {
 Status SortedCollectionRebuilder::listBasedIndexRebuild() {
   std::vector<std::future<Status>> fs;
   size_t i = 0;
-  for (auto& skiplist : rebuild_skiplits_) {
+  for (auto skiplist : rebuild_skiplits_) {
     i++;
     fs.push_back(std::async(&SortedCollectionRebuilder::rebuildSkiplistIndex,
                             this, skiplist.second.get()));

engine/utils/utils.hpp

@@ -38,12 +38,7 @@ template <typename F>
 struct Defer {
   F f;
   Defer(F f) : f(f) {}
-  ~Defer() {
-    try {
-      f();
-    } catch (std::exception& err) {
-    }
-  }
+  ~Defer() { f(); }
 };
 template <typename F>
 Defer<F> defer_func(F f) {