diff --git a/CMakeLists.txt b/CMakeLists.txt index b8f194c8..ad956eb4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -112,17 +112,17 @@ add_executable(bench benchmark/bench.cpp) target_link_libraries(bench PUBLIC engine) target_include_directories(bench PUBLIC ./include ./extern ./) -option(BUILD_TESTING "Build the tests" ON) +option(BUILD_TESTING "Build the tests" OFF) if (BUILD_TESTING) add_subdirectory(${CMAKE_SOURCE_DIR}/extern/gtest) add_subdirectory(tests) endif () -option(BUILD_TUTORIAL "Build the tutorial" ON) +option(BUILD_TUTORIAL "Build the tutorial" OFF) add_subdirectory(examples/tutorial) -option(BUILD_GRAPH_SIMULATOR "Build the graph simulator" ON) +option(BUILD_GRAPH_SIMULATOR "Build the graph simulator" OFF) add_subdirectory(examples/graph_sim) add_custom_target(checkers ALL) diff --git a/README.md b/README.md index decdead5..8226343a 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ * C/C++/Java APIs # Limitations -* The maximum supported key-value size is 64KB-4GB. +* The maximum supported key-value size is 64KB-4GB according to configs. * No approach to key-value compression. * Users can't expand the persistent memory space on the fly. diff --git a/benchmark/bench.cpp b/benchmark/bench.cpp index 6593bdb6..1d2c5bc4 100644 --- a/benchmark/bench.cpp +++ b/benchmark/bench.cpp @@ -169,6 +169,9 @@ void DBWrite(int tid) { std::unique_ptr batch; if (engine != nullptr) { batch = engine->WriteBatchCreate(); + if (batch == nullptr) return; + } else { + return; } for (size_t operations = 0; operations < operations_per_thread; @@ -501,7 +504,7 @@ int main(int argc, char** argv) { if (s != Status::Ok) { throw std::runtime_error{ std::string{"Fail to open KVDK instance. Status: "} + - KVDKStatusStrings[static_cast(s)]}; + ((s <= Status::Unknown) ? KVDKStatusStrings[s] : "")}; } } @@ -528,7 +531,7 @@ int main(int argc, char** argv) { switch (bench_data_type) { case DataType::Sorted: { printf("Create %ld Sorted Collections\n", FLAGS_num_collection); - for (auto col : collections) { + for (auto& col : collections) { SortedCollectionConfigs s_configs; Status s = engine->SortedCreate(col, s_configs); if (s != Status::Ok && s != Status::Existed) { @@ -538,7 +541,7 @@ int main(int argc, char** argv) { break; } case DataType::Hashes: { - for (auto col : collections) { + for (auto& col : collections) { Status s = engine->HashCreate(col); if (s != Status::Ok && s != Status::Existed) { throw std::runtime_error{"Fail to create Hashset"}; @@ -547,7 +550,7 @@ int main(int argc, char** argv) { break; } case DataType::List: { - for (auto col : collections) { + for (auto& col : collections) { Status s = engine->ListCreate(col); if (s != Status::Ok && s != Status::Existed) { throw std::runtime_error{"Fail to create List"}; diff --git a/engine/backup_log.hpp b/engine/backup_log.hpp index bea59fd3..bc107283 100644 --- a/engine/backup_log.hpp +++ b/engine/backup_log.hpp @@ -78,6 +78,7 @@ class BackupLog { BackupLog() = default; BackupLog(const BackupLog&) = delete; + BackupLog& operator=(const BackupLog& rhs) = delete; // Init a new backup log Status Init(const std::string& backup_log) { @@ -141,7 +142,7 @@ class BackupLog { file_size_ = lseek(fd_, 0, SEEK_END); if (file_size_ < sizeof(BackupStage)) { GlobalLogger.Error( - "Open backup log file %s error: file size %lu smaller than " + "Open backup log file %lu error: file size %lu smaller than " "persisted " "stage flag", backup_log.size(), file_size_); @@ -207,7 +208,7 @@ class BackupLog { // Close backup log file void Close() { - if (log_file_ != nullptr) { + if (log_file_ != nullptr && file_size_ > 0) { munmap(log_file_, file_size_); } if (fd_ >= 0) { @@ -224,7 +225,9 @@ class BackupLog { // Destroy backup log file void Destroy() { Close(); - remove(file_name_.c_str()); + if (remove(file_name_.c_str()) == -1) { + GlobalLogger.Error("Fail to remove file %s\n", file_name_.c_str()); + } } private: @@ -272,8 +275,8 @@ class BackupLog { std::string file_name_{}; std::string delta_{}; void* log_file_{nullptr}; - size_t file_size_{0}; + uint64_t file_size_{0}; int fd_{-1}; BackupStage stage_{BackupStage::NotFinished}; }; -} // namespace KVDK_NAMESPACE \ No newline at end of file +} // namespace KVDK_NAMESPACE diff --git a/engine/c/kvdk_sorted.cpp b/engine/c/kvdk_sorted.cpp index 66bb4c27..b08c8cd9 100644 --- a/engine/c/kvdk_sorted.cpp +++ b/engine/c/kvdk_sorted.cpp @@ -26,9 +26,6 @@ KVDKStatus KVDKSortedCreate(KVDKEngine* engine, const char* collection_name, KVDKSortedCollectionConfigs* configs) { KVDKStatus s = engine->rep->SortedCreate( StringView(collection_name, collection_len), configs->rep); - if (s != KVDKStatus::Ok) { - return s; - } return s; } diff --git a/engine/dl_list.hpp b/engine/dl_list.hpp index f450fe35..908ef218 100644 --- a/engine/dl_list.hpp +++ b/engine/dl_list.hpp @@ -39,6 +39,8 @@ class DLList { WriteArgs(WriteArgs&& args) = delete; + WriteArgs& operator=(const WriteArgs& rhs) = delete; + StringView key; StringView val; RecordType type; @@ -197,7 +199,8 @@ class DLListDataIterator { DLRecord* findValidVersion(DLRecord* pmem_record) { DLRecord* curr = pmem_record; TimestampType ts = snapshot_->GetTimestamp(); - while (curr != nullptr && curr->GetTimestamp() > ts) { + while (curr != nullptr) { + if (curr->GetTimestamp() <= ts) break; curr = pmem_allocator_->offset2addr(curr->old_version); kvdk_assert(curr == nullptr || curr->Validate(), "Broken checkpoint: invalid older version sorted record"); @@ -346,4 +349,4 @@ class DLListRecoveryUtils { private: const PMEMAllocator* pmem_allocator_; }; -} // namespace KVDK_NAMESPACE \ No newline at end of file +} // namespace KVDK_NAMESPACE diff --git a/engine/dram_allocator.cpp b/engine/dram_allocator.cpp index 87fdcfdf..0d264b00 100644 --- a/engine/dram_allocator.cpp +++ b/engine/dram_allocator.cpp @@ -15,6 +15,7 @@ void ChunkBasedAllocator::Free(const SpaceEntry&) { SpaceEntry ChunkBasedAllocator::Allocate(uint64_t size) { kvdk_assert(ThreadManager::ThreadID() >= 0, ""); SpaceEntry entry; + memset(&entry, 0, sizeof(entry)); auto& tc = dalloc_thread_cache_[ThreadManager::ThreadID() % dalloc_thread_cache_.size()]; if (size > chunk_size_) { @@ -43,4 +44,4 @@ SpaceEntry ChunkBasedAllocator::Allocate(uint64_t size) { tc.usable_bytes -= size; return entry; } -} // namespace KVDK_NAMESPACE \ No newline at end of file +} // namespace KVDK_NAMESPACE diff --git a/engine/dram_allocator.hpp b/engine/dram_allocator.hpp index 46458f7b..1df8450d 100644 --- a/engine/dram_allocator.hpp +++ b/engine/dram_allocator.hpp @@ -34,12 +34,16 @@ class ChunkBasedAllocator : Allocator { : dalloc_thread_cache_(max_access_threads) {} ChunkBasedAllocator(ChunkBasedAllocator const&) = delete; ChunkBasedAllocator(ChunkBasedAllocator&&) = delete; + ChunkBasedAllocator& operator=(const ChunkBasedAllocator& rhs) = delete; ~ChunkBasedAllocator() { - for (uint64_t i = 0; i < dalloc_thread_cache_.size(); i++) { - auto& tc = dalloc_thread_cache_[i]; - for (void* chunk : tc.allocated_chunks) { - free(chunk); + try { + for (uint64_t i = 0; i < dalloc_thread_cache_.size(); i++) { + auto& tc = dalloc_thread_cache_[i]; + for (void* chunk : tc.allocated_chunks) { + free(chunk); + } } + } catch (std::exception& err) { } } @@ -52,9 +56,10 @@ class ChunkBasedAllocator : Allocator { DAllocThreadCache() = default; DAllocThreadCache(const DAllocThreadCache&) = delete; DAllocThreadCache(DAllocThreadCache&&) = delete; + DAllocThreadCache& operator=(const DAllocThreadCache& rhs) = delete; }; const uint32_t chunk_size_ = (1 << 20); Array dalloc_thread_cache_; }; -} // namespace KVDK_NAMESPACE \ No newline at end of file +} // namespace KVDK_NAMESPACE diff --git a/engine/hash_collection/hash_list.cpp b/engine/hash_collection/hash_list.cpp index fa65ab41..f4f03dbb 100644 --- a/engine/hash_collection/hash_list.cpp +++ b/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; @@ -423,4 +423,4 @@ HashList::WriteResult HashList::deletePrepared( return ret; } -} // namespace KVDK_NAMESPACE \ No newline at end of file +} // namespace KVDK_NAMESPACE diff --git a/engine/hash_collection/rebuilder.hpp b/engine/hash_collection/rebuilder.hpp index 1211f895..f59a0dba 100644 --- a/engine/hash_collection/rebuilder.hpp +++ b/engine/hash_collection/rebuilder.hpp @@ -234,8 +234,8 @@ class HashListRebuilder { CollectionIDType id = HashList::FetchID(pmem_record); DLRecord* curr = pmem_record; - while (curr != nullptr && - curr->GetTimestamp() > checkpoint_.CheckpointTS()) { + while (curr != nullptr) { + if (curr->GetTimestamp() <= checkpoint_.CheckpointTS()) break; curr = pmem_allocator_->offset2addr(curr->old_version); kvdk_assert(curr == nullptr || curr->Validate(), "Broken checkpoint: invalid older version sorted record"); @@ -353,11 +353,11 @@ class HashListRebuilder { invalid_hlists_; std::unordered_map> rebuild_hlists_; - CollectionIDType max_recovered_id_; + CollectionIDType max_recovered_id_{0}; // We manually allocate recovery thread id for no conflict in multi-thread // recovering // Todo: do not hard code std::atomic next_tid_{0}; }; -} // namespace KVDK_NAMESPACE \ No newline at end of file +} // namespace KVDK_NAMESPACE diff --git a/engine/hash_table.cpp b/engine/hash_table.cpp index 5cc5986f..f59c8fd3 100644 --- a/engine/hash_table.cpp +++ b/engine/hash_table.cpp @@ -21,7 +21,7 @@ HashTable* HashTable::NewHashTable(uint64_t hash_bucket_num, table = new HashTable(hash_bucket_num, num_buckets_per_slot, pmem_allocator, max_access_threads); } catch (std::bad_alloc& b) { - GlobalLogger.Error("No enough dram to create global hash table: b\n", + GlobalLogger.Error("No enough dram to create global hash table: %s\n", b.what()); table = nullptr; } @@ -80,7 +80,7 @@ bool HashEntry::Match(const StringView& key, uint32_t hash_k_prefix, } } - if (data_entry_metadata != nullptr) { + if (data_entry_metadata != nullptr && pmem_record != nullptr) { memcpy(data_entry_metadata, pmem_record, sizeof(DataEntry)); } diff --git a/engine/hash_table.hpp b/engine/hash_table.hpp index a0d6d6fd..a4c74c8c 100644 --- a/engine/hash_table.hpp +++ b/engine/hash_table.hpp @@ -129,16 +129,18 @@ class HashTable { HashEntry* entry_ptr{nullptr}; LookupResult& operator=(LookupResult const& other) { - s = other.s; - memcpy_16(&entry, &other.entry); - entry_ptr = other.entry_ptr; - key_hash_prefix = other.key_hash_prefix; + if (&other != this) { + s = other.s; + memcpy_16(&entry, &other.entry); + entry_ptr = other.entry_ptr; + key_hash_prefix = other.key_hash_prefix; + } return *this; } private: friend class HashTable; - uint32_t key_hash_prefix; + uint32_t key_hash_prefix{0}; }; static HashTable* NewHashTable(uint64_t hash_bucket_num, @@ -254,7 +256,6 @@ class HashTable { Array slots_; std::vector hash_bucket_entries_; Array hash_buckets_; - void* main_buckets_; }; // Iterator all hash entries in a hash table bucket diff --git a/engine/kv_engine.cpp b/engine/kv_engine.cpp index cb7cc794..95493b0b 100644 --- a/engine/kv_engine.cpp +++ b/engine/kv_engine.cpp @@ -219,7 +219,7 @@ Status KVEngine::restoreData() { SpaceEntry segment_recovering; DataEntry data_entry_cached; uint64_t cnt = 0; - Status s; + Status s = Status::Ok; while (true) { if (segment_recovering.size == 0) { if (!pmem_allocator_->FetchSegment(&segment_recovering)) { @@ -415,7 +415,8 @@ Status KVEngine::Backup(const pmem::obj::string_view backup_log, switch (slot_iter->GetRecordType()) { case RecordType::String: { StringRecord* record = slot_iter->GetIndex().string_record; - while (record != nullptr && record->GetTimestamp() > backup_ts) { + while (record != nullptr) { + if (record->GetTimestamp() <= backup_ts) break; record = pmem_allocator_->offset2addr(record->old_version); } @@ -428,7 +429,8 @@ Status KVEngine::Backup(const pmem::obj::string_view backup_log, } case RecordType::SortedRecord: { DLRecord* header = slot_iter->GetIndex().skiplist->HeaderRecord(); - while (header != nullptr && header->GetTimestamp() > backup_ts) { + while (header != nullptr) { + if (header->GetTimestamp() <= backup_ts) break; header = pmem_allocator_->offset2addr(header->old_version); } @@ -458,7 +460,8 @@ Status KVEngine::Backup(const pmem::obj::string_view backup_log, } case RecordType::HashRecord: { DLRecord* header = slot_iter->GetIndex().hlist->HeaderRecord(); - while (header != nullptr && header->GetTimestamp() > backup_ts) { + while (header != nullptr) { + if (header->GetTimestamp() <= backup_ts) break; header = pmem_allocator_->offset2addr(header->old_version); } @@ -487,7 +490,8 @@ Status KVEngine::Backup(const pmem::obj::string_view backup_log, } case RecordType::ListRecord: { DLRecord* header = slot_iter->GetIndex().list->HeaderRecord(); - while (header != nullptr && header->GetTimestamp() > backup_ts) { + while (header != nullptr) { + if (header->GetTimestamp() <= backup_ts) break; header = pmem_allocator_->offset2addr(header->old_version); } @@ -838,7 +842,7 @@ Status KVEngine::checkConfigs(const Configs& configs) { if (configs.pmem_file_size % segment_size != 0) { GlobalLogger.Error( "pmem file size should align to segment " - "size(pmem_segment_blocks*pmem_block_size) (%d bytes)\n", + "size(pmem_segment_blocks*pmem_block_size) (%lu bytes)\n", segment_size); return Status::InvalidConfiguration; } @@ -1162,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}; @@ -1219,7 +1224,6 @@ Status KVEngine::batchWriteRollbackLogs() { return Status::PMemMapFileError; } } - closedir(dir); std::string cmd{"rm -rf " + batch_log_dir_ + "*"}; [[gnu::unused]] int ret = system(cmd.c_str()); @@ -1432,7 +1436,8 @@ T* KVEngine::removeOutDatedVersion(T* record, TimestampType min_snapshot_ts) { "Invalid record type, should be StringRecord or DLRecord."); T* ret = nullptr; auto old_record = record; - while (old_record && old_record->GetTimestamp() > min_snapshot_ts) { + while (old_record) { + if (old_record->GetTimestamp() <= min_snapshot_ts) break; old_record = static_cast(pmem_allocator_->offset2addr(old_record->old_version)); } diff --git a/engine/kv_engine.hpp b/engine/kv_engine.hpp index f577e96b..0d9c1e3d 100644 --- a/engine/kv_engine.hpp +++ b/engine/kv_engine.hpp @@ -659,6 +659,7 @@ class KVEngine : public Engine { struct BackgroundWorkSignals { BackgroundWorkSignals() = default; BackgroundWorkSignals(const BackgroundWorkSignals&) = delete; + BackgroundWorkSignals& operator=(const BackgroundWorkSignals& rhs) = delete; std::condition_variable_any pmem_usage_reporter_cv; std::condition_variable_any pmem_allocator_organizer_cv; diff --git a/engine/kv_engine_cleaner.cpp b/engine/kv_engine_cleaner.cpp index b5f17b5f..4ee270b0 100644 --- a/engine/kv_engine_cleaner.cpp +++ b/engine/kv_engine_cleaner.cpp @@ -737,13 +737,15 @@ double Cleaner::SearchOutdatedCollections() { outdated_collections_.lists.size() + outdated_collections_.skiplists.size()); - double diff_size_ratio = - (after_queue_size - before_queue_size) / max_thread_num_; - if (diff_size_ratio > 0) { - outdated_collections_.increase_ratio = - outdated_collections_.increase_ratio == 0 - ? diff_size_ratio - : diff_size_ratio / outdated_collections_.increase_ratio; + if (max_thread_num_ > 0) { + double diff_size_ratio = + (double)(after_queue_size - before_queue_size) / max_thread_num_; + if (diff_size_ratio > 0) { + outdated_collections_.increase_ratio = + outdated_collections_.increase_ratio == 0 + ? diff_size_ratio + : diff_size_ratio / outdated_collections_.increase_ratio; + } } return outdated_collections_.increase_ratio; } diff --git a/engine/kv_engine_cleaner.hpp b/engine/kv_engine_cleaner.hpp index 6968170f..d21eed8b 100644 --- a/engine/kv_engine_cleaner.hpp +++ b/engine/kv_engine_cleaner.hpp @@ -171,8 +171,8 @@ class Cleaner { } private: - bool keep_work_; - bool join_; + bool keep_work_{false}; + bool join_{false}; std::condition_variable_any cv; SpinMutex spin; std::thread worker_thread; @@ -181,8 +181,8 @@ class Cleaner { KVEngine* kv_engine_; - size_t max_thread_num_; - size_t min_thread_num_; + size_t max_thread_num_{1}; + size_t min_thread_num_{1}; std::atomic close_; std::atomic start_slot_; std::atomic active_clean_workers_; @@ -221,4 +221,4 @@ class Cleaner { void mainWork(); }; -} // namespace KVDK_NAMESPACE \ No newline at end of file +} // namespace KVDK_NAMESPACE diff --git a/engine/kv_engine_sorted.cpp b/engine/kv_engine_sorted.cpp index 9dc801ed..d76f1f4f 100644 --- a/engine/kv_engine_sorted.cpp +++ b/engine/kv_engine_sorted.cpp @@ -36,7 +36,7 @@ Status KVEngine::buildSkiplist(const StringView& collection_name, auto comparator = comparators_.GetComparator(s_configs.comparator_name); if (comparator == nullptr) { GlobalLogger.Error("Compare function %s is not registered\n", - s_configs.comparator_name); + s_configs.comparator_name.c_str()); return Status::Abort; } CollectionIDType id = collection_id_.fetch_add(1); diff --git a/engine/list_collection/list.cpp b/engine/list_collection/list.cpp index 1098a839..c6d509db 100644 --- a/engine/list_collection/list.cpp +++ b/engine/list_collection/list.cpp @@ -310,7 +310,7 @@ List::PushNArgs List::PreparePushN(ListPos pos, SpaceEntry space = pmem_allocator_->Allocate(DLRecord::RecordSize(internal_key, elem)); if (space.size == 0) { - GlobalLogger.Error("Try allocate %lu error\n", + GlobalLogger.Error("Try allocate %u error\n", DLRecord::RecordSize(internal_key, elem)); for (auto& sp : args.spaces) { pmem_allocator_->Free(sp); @@ -466,4 +466,4 @@ void List::DestroyAll() { } pmem_allocator_->BatchFree(to_free); } -} // namespace KVDK_NAMESPACE \ No newline at end of file +} // namespace KVDK_NAMESPACE diff --git a/engine/list_collection/list.hpp b/engine/list_collection/list.hpp index 999c660b..c010c84d 100644 --- a/engine/list_collection/list.hpp +++ b/engine/list_collection/list.hpp @@ -34,7 +34,7 @@ class List : public Collection { private: friend List; std::vector::iterator> to_pop_{}; - TimestampType timestamp_; + TimestampType timestamp_{0}; }; struct PushNArgs { @@ -167,4 +167,4 @@ class List : public Collection { // in a deque to support fast write operations std::deque live_records_; }; -} // namespace KVDK_NAMESPACE \ No newline at end of file +} // namespace KVDK_NAMESPACE diff --git a/engine/list_collection/rebuilder.hpp b/engine/list_collection/rebuilder.hpp index 5f93dfa6..13b22b38 100644 --- a/engine/list_collection/rebuilder.hpp +++ b/engine/list_collection/rebuilder.hpp @@ -203,8 +203,8 @@ class ListRebuilder { CollectionIDType id = List::FetchID(pmem_record); DLRecord* curr = pmem_record; - while (curr != nullptr && - curr->GetTimestamp() > checkpoint_.CheckpointTS()) { + while (curr != nullptr) { + if (curr->GetTimestamp() <= checkpoint_.CheckpointTS()) break; curr = pmem_allocator_->offset2addr(curr->old_version); kvdk_assert(curr == nullptr || curr->Validate(), "Broken checkpoint: invalid older version sorted record"); @@ -299,11 +299,11 @@ class ListRebuilder { SpinMutex lock_; std::unordered_map> invalid_lists_; std::unordered_map> rebuild_lists_; - CollectionIDType max_recovered_id_; + CollectionIDType max_recovered_id_{0}; // We manually allocate recovery thread id for no conflict in multi-thread // recovering // Todo: do not hard code std::atomic next_tid_{0}; }; -} // namespace KVDK_NAMESPACE \ No newline at end of file +} // namespace KVDK_NAMESPACE diff --git a/engine/logger.hpp b/engine/logger.hpp index 6bf978a1..1fa2a9cd 100644 --- a/engine/logger.hpp +++ b/engine/logger.hpp @@ -28,7 +28,7 @@ class Logger { void Log(const char* log_type, const char* format, va_list& args); FILE* log_file_ = NULL; - LogLevel level_; + LogLevel level_{LogLevel::None}; std::mutex mut_; std::chrono::time_point start_ts_; @@ -36,4 +36,4 @@ class Logger { extern Logger GlobalLogger; -} // namespace KVDK_NAMESPACE \ No newline at end of file +} // namespace KVDK_NAMESPACE diff --git a/engine/pmem_allocator/free_list.hpp b/engine/pmem_allocator/free_list.hpp index bb3f3292..b36fda45 100644 --- a/engine/pmem_allocator/free_list.hpp +++ b/engine/pmem_allocator/free_list.hpp @@ -179,7 +179,8 @@ class Freelist { block_size_(block_size), max_small_entry_block_size_(max_small_entry_b_size), max_block_size_index_(std::min( - kMaxBlockSizeIndex, blockSizeIndex(num_segment_blocks_) + 1)), + kMaxBlockSizeIndex, + blockSizeIndex(num_segment_blocks_, kMaxBlockSizeIndex) + 1)), active_pool_(max_small_entry_b_size, max_block_size_index_), merged_pool_(max_small_entry_b_size, max_block_size_index_), space_map_(num_blocks), @@ -240,6 +241,7 @@ class Freelist { FlistThreadCache() = delete; FlistThreadCache(FlistThreadCache&&) = delete; FlistThreadCache(const FlistThreadCache&) = delete; + FlistThreadCache& operator=(const FlistThreadCache& rhs) = delete; // Offsets of small free space entries whose block size smaller than // max_small_entry_b_size. Array index indicates block size of entries @@ -260,12 +262,16 @@ class Freelist { }; uint32_t blockSizeIndex(uint32_t block_size) { + return blockSizeIndex(block_size, max_block_size_index_); + } + + uint32_t blockSizeIndex(uint32_t block_size, uint32_t max_block_size_index) { kvdk_assert(block_size <= num_segment_blocks_, ""); uint32_t ret = block_size < max_small_entry_block_size_ ? 0 : (block_size - max_small_entry_block_size_) / kBlockSizeIndexInterval; - return std::min(ret, max_block_size_index_ - 1); + return std::min(ret, max_block_size_index - 1); } bool getSmallEntry(uint32_t size, SpaceEntry* space_entry); diff --git a/engine/pmem_allocator/pmem_allocator.cpp b/engine/pmem_allocator/pmem_allocator.cpp index f7307c97..44872b88 100644 --- a/engine/pmem_allocator/pmem_allocator.cpp +++ b/engine/pmem_allocator/pmem_allocator.cpp @@ -82,13 +82,13 @@ 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 { if (!checkDevDaxAndGetSize(pmem_file.c_str(), &mapped_size)) { - GlobalLogger.Error( - "checkDevDaxAndGetSize %s failed device %s faild: %s\n", - pmem_file.c_str(), strerror(errno)); + GlobalLogger.Error("checkDevDaxAndGetSize device %s faild: %s\n", + pmem_file.c_str(), strerror(errno)); return nullptr; } @@ -108,10 +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); + unmap_pmem(); return nullptr; } @@ -121,6 +130,7 @@ PMEMAllocator* PMEMAllocator::NewPMEMAllocator( GlobalLogger.Error( "pmem file too small, should larger than pmem_segment_blocks * " "pmem_block_size * max_access_threads\n"); + unmap_pmem(); return nullptr; } @@ -133,6 +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); + unmap_pmem(); return nullptr; } @@ -146,6 +157,7 @@ PMEMAllocator* PMEMAllocator::NewPMEMAllocator( } catch (std::bad_alloc& err) { GlobalLogger.Error("Error while initialize PMEMAllocator: %s\n", err.what()); + unmap_pmem(); return nullptr; } diff --git a/engine/sorted_collection/rebuilder.cpp b/engine/sorted_collection/rebuilder.cpp index 4b27b691..37d923fe 100644 --- a/engine/sorted_collection/rebuilder.cpp +++ b/engine/sorted_collection/rebuilder.cpp @@ -552,6 +552,7 @@ Status SortedCollectionRebuilder::rebuildSkiplistIndex(Skiplist* skiplist) { } if (s != Status::Ok) { + SkiplistNode::DeleteNode(dram_node); return s; } } @@ -623,7 +624,7 @@ Status SortedCollectionRebuilder::insertHashIndex(const StringView& key, PointerType index_type) { // TODO: ttl RecordType record_type = RecordType::Empty; - RecordStatus record_status; + RecordStatus record_status = {}; if (index_type == PointerType::DLRecord) { record_type = RecordType::SortedElem; record_status = static_cast(index_ptr)->GetRecordStatus(); @@ -682,7 +683,8 @@ DLRecord* SortedCollectionRebuilder::findCheckpointVersion( } CollectionIDType id = Skiplist::FetchID(pmem_record); DLRecord* curr = pmem_record; - while (curr != nullptr && curr->GetTimestamp() > checkpoint_.CheckpointTS()) { + while (curr != nullptr) { + if (curr->GetTimestamp() <= checkpoint_.CheckpointTS()) break; curr = kv_engine_->pmem_allocator_->offset2addr(curr->old_version); @@ -700,4 +702,4 @@ DLRecord* SortedCollectionRebuilder::findCheckpointVersion( } return curr; } -} // namespace KVDK_NAMESPACE \ No newline at end of file +} // namespace KVDK_NAMESPACE diff --git a/engine/sorted_collection/skiplist.cpp b/engine/sorted_collection/skiplist.cpp index 67c0f961..fb38dde5 100644 --- a/engine/sorted_collection/skiplist.cpp +++ b/engine/sorted_collection/skiplist.cpp @@ -379,7 +379,7 @@ Skiplist::WriteResult Skiplist::Write(SortedWriteArgs& args) { SortedWriteArgs Skiplist::InitWriteArgs(const StringView& key, const StringView& value, WriteOp op) { - SortedWriteArgs args; + SortedWriteArgs args = {}; args.collection = Name(); args.skiplist = this; args.key = key; @@ -952,4 +952,4 @@ void Skiplist::UpdateSize(int64_t delta) { "Update skiplist size to negative"); size_.fetch_add(delta, std::memory_order_relaxed); } -} // namespace KVDK_NAMESPACE \ No newline at end of file +} // namespace KVDK_NAMESPACE diff --git a/engine/sorted_collection/skiplist.hpp b/engine/sorted_collection/skiplist.hpp index a069ca48..1d5ef9c4 100644 --- a/engine/sorted_collection/skiplist.hpp +++ b/engine/sorted_collection/skiplist.hpp @@ -48,6 +48,10 @@ struct SortedWriteArgs { * */ struct SkiplistNode { public: + SkiplistNode(const SkiplistNode&) = delete; + SkiplistNode& operator=(const SkiplistNode&) = delete; + SkiplistNode(SkiplistNode&&) = delete; + enum class NodeStatus : uint8_t { Normal = 0, Deleted = 1, @@ -187,6 +191,10 @@ class Skiplist : public Collection { HashTable* hash_table, LockTable* lock_table, bool index_with_hashtable); + Skiplist(const Skiplist& s) = delete; + Skiplist& operator=(const Skiplist& s) = delete; + Skiplist(Skiplist&& s) = delete; + ~Skiplist() final; SkiplistNode* HeaderNode() { return header_; } @@ -524,8 +532,8 @@ class Skiplist : public Collection { struct Splice { // Seeking skiplist Skiplist* seeking_list; - std::array nexts; - std::array prevs; + std::array nexts{nullptr}; + std::array prevs{nullptr}; DLRecord* prev_pmem_record{nullptr}; DLRecord* next_pmem_record{nullptr}; diff --git a/engine/transaction_impl.cpp b/engine/transaction_impl.cpp index 51bcde49..da88ba52 100644 --- a/engine/transaction_impl.cpp +++ b/engine/transaction_impl.cpp @@ -14,7 +14,7 @@ constexpr int64_t kLockTimeoutMicrosecondsMin = 5000; constexpr int64_t kLockTimeoutMicrosecondsMax = 15000; TransactionImpl::TransactionImpl(KVEngine* engine) - : engine_(engine), timeout_(randomTimeout()) { + : engine_(engine), status_(Status::Ok), timeout_(randomTimeout()) { kvdk_assert(engine_ != nullptr, ""); batch_.reset( dynamic_cast(engine_->WriteBatchCreate().release())); @@ -266,4 +266,4 @@ int64_t TransactionImpl::randomTimeout() { (kLockTimeoutMicrosecondsMax - kLockTimeoutMicrosecondsMin) + kLockTimeoutMicrosecondsMin; } -} // namespace KVDK_NAMESPACE \ No newline at end of file +} // namespace KVDK_NAMESPACE diff --git a/engine/transaction_impl.hpp b/engine/transaction_impl.hpp index 00ef3bf6..6db5bd90 100644 --- a/engine/transaction_impl.hpp +++ b/engine/transaction_impl.hpp @@ -29,6 +29,7 @@ class CollectionTransactionCV { TransactionToken(const TransactionToken&) = delete; TransactionToken(TransactionToken&&) = delete; + TransactionToken& operator=(const TransactionToken& rhs) = delete; ~TransactionToken() { cv_->FinishTransaction(); } @@ -46,6 +47,7 @@ class CollectionTransactionCV { CollectionToken(const CollectionToken&) = delete; CollectionToken(CollectionToken&&) = delete; + CollectionToken& operator=(const CollectionToken& rhs) = delete; private: CollectionTransactionCV* cv_; @@ -55,6 +57,9 @@ class CollectionTransactionCV { CollectionTransactionCV(const CollectionTransactionCV&) = delete; + CollectionTransactionCV& operator=(const CollectionTransactionCV& rhs) = + delete; + void AcquireTransaction() { std::unique_lock ul(spin_); while (processing_collection_ > 0) { @@ -148,4 +153,4 @@ class TransactionImpl final : public Transaction { std::unique_ptr ct_token_; int64_t timeout_; }; -} // namespace KVDK_NAMESPACE \ No newline at end of file +} // namespace KVDK_NAMESPACE diff --git a/engine/version/version_controller.hpp b/engine/version/version_controller.hpp index 97f1b50f..accd2871 100644 --- a/engine/version/version_controller.hpp +++ b/engine/version/version_controller.hpp @@ -101,7 +101,10 @@ class VersionController { } ~LocalSnapshotHolder() { if (owner_ != nullptr) { - owner_->ReleaseLocalSnapshot(); + try { + owner_->ReleaseLocalSnapshot(); + } catch (std::exception& err) { + } } } TimestampType Timestamp() { return ts_; } @@ -162,10 +165,14 @@ class VersionController { } ~BatchWriteToken() { if (owner_ != nullptr) { - auto& tc = - owner_->version_thread_cache_[ThreadManager::ThreadID() % + try { + auto& tc = + owner_ + ->version_thread_cache_[ThreadManager::ThreadID() % owner_->version_thread_cache_.size()]; - tc.batch_write_ts = kMaxTimestamp; + tc.batch_write_ts = kMaxTimestamp; + } catch (std::exception& err) { + } } } TimestampType Timestamp() { return ts_; } @@ -311,8 +318,8 @@ class VersionController { // These two used to get current timestamp of the instance // version_base_: The newest timestamp on instance closing last time // tsc_on_startup_: The CPU tsc on instance start up - uint64_t base_timestamp_; - uint64_t tsc_on_startup_; + uint64_t base_timestamp_{0}; + uint64_t tsc_on_startup_{0}; }; class CheckPoint { @@ -338,4 +345,4 @@ class CheckPoint { TimestampType checkpoint_ts_; }; -} // namespace KVDK_NAMESPACE \ No newline at end of file +} // namespace KVDK_NAMESPACE diff --git a/engine/write_batch_impl.hpp b/engine/write_batch_impl.hpp index 7ce49045..19ba911b 100644 --- a/engine/write_batch_impl.hpp +++ b/engine/write_batch_impl.hpp @@ -328,7 +328,7 @@ class BatchWriteLog { private: Stage stage{Stage::Initializing}; - TimestampType timestamp_; + TimestampType timestamp_{0}; StringLog string_logs_; SortedLog sorted_logs_; HashLog hash_logs_; diff --git a/examples/tutorial/c_api_tutorial.c b/examples/tutorial/c_api_tutorial.c index df5d7fc5..be33e46d 100644 --- a/examples/tutorial/c_api_tutorial.c +++ b/examples/tutorial/c_api_tutorial.c @@ -154,8 +154,8 @@ void SortedIteratorExample(KVDKEngine* kvdk_engine) { assert(s == Ok); for (int i = 0; i < 10; ++i) { char key[10] = "key", value[10] = "value"; - strcat(key, nums[i]); - strcat(value, nums[i]); + strncat(key, nums[i], 1); + strncat(value, nums[i], 1); s = KVDKSortedPut(kvdk_engine, sorted_collection, strlen(sorted_collection), key, strlen(key), value, strlen(value)); assert(s == Ok); diff --git a/include/kvdk/types.h b/include/kvdk/types.h index f7a1b637..ab164d6b 100644 --- a/include/kvdk/types.h +++ b/include/kvdk/types.h @@ -71,7 +71,8 @@ __attribute__((unused)) static char const* KVDKValueTypeString[] = { GEN(InvalidConfiguration) \ GEN(Fail) \ GEN(Timeout) \ - GEN(Abort) + GEN(Abort) \ + GEN(Unknown) typedef enum { KVDK_STATUS(GENERATE_ENUM) } KVDKStatus;