From fb3d3f97b706e28d300617803963c896fd6b3d5a Mon Sep 17 00:00:00 2001 From: laimuxi <1721261216@qq.com> Date: Wed, 10 May 2023 23:00:02 +0800 Subject: [PATCH 01/17] [feat] change storage new/del to smartp --- src/storage/include/storage/storage.h | 12 +++++------ src/storage/src/redis.cc | 10 ++++----- src/storage/src/redis.h | 9 ++++++--- src/storage/src/redis_hyperloglog.cc | 29 +++++++++++++++++---------- src/storage/src/redis_hyperloglog.h | 4 +++- src/storage/src/redis_sets.cc | 9 ++------- src/storage/src/redis_sets.h | 2 +- src/storage/src/scope_record_lock.h | 8 ++++---- src/storage/src/storage.cc | 25 +++++++++-------------- 9 files changed, 54 insertions(+), 54 deletions(-) diff --git a/src/storage/include/storage/storage.h b/src/storage/include/storage/storage.h index 54e8d3b52..56b3fb728 100644 --- a/src/storage/include/storage/storage.h +++ b/src/storage/include/storage/storage.h @@ -1020,14 +1020,14 @@ class Storage { const std::unordered_map& options); private: - RedisStrings* strings_db_; - RedisHashes* hashes_db_; - RedisSets* sets_db_; - RedisZSets* zsets_db_; - RedisLists* lists_db_; + std::shared_ptr strings_db_; + std::shared_ptr hashes_db_; + std::shared_ptr sets_db_; + std::shared_ptr zsets_db_; + std::shared_ptr lists_db_; std::atomic is_opened_; - LRUCache* cursors_store_; + std::shared_ptr> cursors_store_; // Storage start the background thread for compaction task pthread_t bg_tasks_thread_id_; diff --git a/src/storage/src/redis.cc b/src/storage/src/redis.cc index e69df57f1..c63df09c9 100644 --- a/src/storage/src/redis.cc +++ b/src/storage/src/redis.cc @@ -13,8 +13,8 @@ Redis::Redis(Storage* const s, const DataType& type) lock_mgr_(new LockMgr(1000, 0, std::make_shared())), db_(nullptr), small_compaction_threshold_(5000) { - statistics_store_ = new LRUCache(); - scan_cursors_store_ = new LRUCache(); + statistics_store_ = std::shared_ptr> (new LRUCache()); + scan_cursors_store_ = std::shared_ptr>(new LRUCache()); scan_cursors_store_->SetCapacity(5000); default_compact_range_options_.exclusive_manual_compaction = false; default_compact_range_options_.change_level = true; @@ -28,9 +28,9 @@ Redis::~Redis() { delete handle; } delete db_; - delete lock_mgr_; - delete statistics_store_; - delete scan_cursors_store_; + // delete lock_mgr_; + // delete statistics_store_; + // delete scan_cursors_store_; } Status Redis::GetScanStartPoint(const Slice& key, const Slice& pattern, int64_t cursor, std::string* start_point) { diff --git a/src/storage/src/redis.h b/src/storage/src/redis.h index 105450388..a3a0e6ab0 100644 --- a/src/storage/src/redis.h +++ b/src/storage/src/redis.h @@ -58,22 +58,25 @@ class Redis { protected: Storage* const storage_; DataType type_; - LockMgr* lock_mgr_ = nullptr; + // LockMgr* lock_mgr_ = nullptr; + std::shared_ptr lock_mgr_; rocksdb::DB* db_ = nullptr; + // std::shared_ptr db_; + std::vector handles_; rocksdb::WriteOptions default_write_options_; rocksdb::ReadOptions default_read_options_; rocksdb::CompactRangeOptions default_compact_range_options_; // For Scan - LRUCache* scan_cursors_store_ = nullptr; + std::shared_ptr> scan_cursors_store_; Status GetScanStartPoint(const Slice& key, const Slice& pattern, int64_t cursor, std::string* start_point); Status StoreScanNextPoint(const Slice& key, const Slice& pattern, int64_t cursor, const std::string& next_point); // For Statistics std::atomic small_compaction_threshold_; - LRUCache* statistics_store_ = nullptr; + std::shared_ptr> statistics_store_; Status UpdateSpecificKeyStatistics(const std::string& key, size_t count); Status AddCompactKeyTaskIfNeeded(const std::string& key, size_t total); diff --git a/src/storage/src/redis_hyperloglog.cc b/src/storage/src/redis_hyperloglog.cc index 47fa93653..b252b94a8 100644 --- a/src/storage/src/redis_hyperloglog.cc +++ b/src/storage/src/redis_hyperloglog.cc @@ -17,28 +17,31 @@ HyperLogLog::HyperLogLog(uint8_t precision, std::string origin_register) { b_ = precision; m_ = 1 << precision; alpha_ = Alpha(); - register_ = new char[m_]; + // register_ = std::shared_ptr(new char[m_], [](char* ptr) { delete[] ptr; }); + auto register_ptr = new char[m_]; for (uint32_t i = 0; i < m_; ++i) { - register_[i] = 0; + register_ptr[i] = 0; } if (origin_register != "") { for (uint32_t i = 0; i < m_; ++i) { - register_[i] = origin_register[i]; + register_ptr[i] = origin_register[i]; } } + register_ = std::shared_ptr(register_ptr, [](char* ptr) { delete[] ptr; }); } -HyperLogLog::~HyperLogLog() { delete[] register_; } +HyperLogLog::~HyperLogLog() {} std::string HyperLogLog::Add(const char* value, uint32_t len) { uint32_t hash_value; MurmurHash3_x86_32(value, len, HLL_HASH_SEED, static_cast(&hash_value)); int32_t index = hash_value & ((1 << b_) - 1); uint8_t rank = Nctz((hash_value >> b_), 32 - b_); - if (rank > register_[index]) register_[index] = rank; + auto register_ptr = register_.get(); + if (rank > register_ptr[index]) register_ptr[index] = rank; std::string result(m_, 0); for (uint32_t i = 0; i < m_; ++i) { - result[i] = register_[i]; + result[i] = register_ptr[i]; } return result; } @@ -58,8 +61,9 @@ double HyperLogLog::Estimate() const { double HyperLogLog::FirstEstimate() const { double estimate, sum = 0.0; + auto register_ptr = register_.get(); for (uint32_t i = 0; i < m_; i++) { - sum += 1.0 / (1 << register_[i]); + sum += 1.0 / (1 << register_ptr[i]); } estimate = alpha_ * m_ * m_ / sum; @@ -81,8 +85,9 @@ double HyperLogLog::Alpha() const { uint32_t HyperLogLog::CountZero() const { uint32_t count = 0; + auto register_ptr = register_.get(); for (uint32_t i = 0; i < m_; i++) { - if (register_[i] == 0) { + if (register_ptr[i] == 0) { count++; } } @@ -93,15 +98,17 @@ std::string HyperLogLog::Merge(const HyperLogLog& hll) { if (m_ != hll.m_) { // TODO(shq) the number of registers doesn't match } + auto register_ptr = register_.get(); + auto hll_register_ptr = hll.register_.get(); for (uint32_t r = 0; r < m_; r++) { - if (register_[r] < hll.register_[r]) { - register_[r] |= hll.register_[r]; + if (register_ptr[r] < hll_register_ptr[r]) { + register_ptr[r] |= hll_register_ptr[r]; } } std::string result(m_, 0); for (uint32_t i = 0; i < m_; ++i) { - result[i] = register_[i]; + result[i] = register_ptr[i]; } return result; } diff --git a/src/storage/src/redis_hyperloglog.h b/src/storage/src/redis_hyperloglog.h index ee8ee5d83..ec5eba0db 100644 --- a/src/storage/src/redis_hyperloglog.h +++ b/src/storage/src/redis_hyperloglog.h @@ -8,6 +8,7 @@ #include #include +#include namespace storage { @@ -29,7 +30,8 @@ class HyperLogLog { uint32_t m_ = 0; // register bit width uint32_t b_ = 0; // regieter size double alpha_ = 0; - char* register_ = nullptr; // register; + // char* register_ = nullptr; // register; + std::shared_ptr register_; }; } // namespace storage diff --git a/src/storage/src/redis_sets.cc b/src/storage/src/redis_sets.cc index 238a39f73..ec9110511 100644 --- a/src/storage/src/redis_sets.cc +++ b/src/storage/src/redis_sets.cc @@ -21,16 +21,11 @@ namespace storage { RedisSets::RedisSets(Storage* const s, const DataType& type) : Redis(s, type) { - spop_counts_store_ = new LRUCache(); + spop_counts_store_ = std::shared_ptr>(new LRUCache()); spop_counts_store_->SetCapacity(1000); } -RedisSets::~RedisSets() { - if (spop_counts_store_ != nullptr) { - delete spop_counts_store_; - spop_counts_store_ = nullptr; - } -} +RedisSets::~RedisSets() {} rocksdb::Status RedisSets::Open(const StorageOptions& storage_options, const std::string& db_path) { statistics_store_->SetCapacity(storage_options.statistics_max_size); diff --git a/src/storage/src/redis_sets.h b/src/storage/src/redis_sets.h index 296a9ea90..4e9727f88 100644 --- a/src/storage/src/redis_sets.h +++ b/src/storage/src/redis_sets.h @@ -73,7 +73,7 @@ class RedisSets : public Redis { private: // For compact in time after multiple spop - LRUCache* spop_counts_store_ = nullptr; + std::shared_ptr> spop_counts_store_; Status ResetSpopCount(const std::string& key); Status AddAndGetSpopCount(const std::string& key, uint64_t* count); }; diff --git a/src/storage/src/scope_record_lock.h b/src/storage/src/scope_record_lock.h index 7ae31db5d..9fcadf760 100644 --- a/src/storage/src/scope_record_lock.h +++ b/src/storage/src/scope_record_lock.h @@ -15,13 +15,13 @@ namespace storage { class ScopeRecordLock { public: - ScopeRecordLock(LockMgr* lock_mgr, const Slice& key) : lock_mgr_(lock_mgr), key_(key) { + ScopeRecordLock(std::shared_ptr lock_mgr, const Slice& key) : lock_mgr_(lock_mgr), key_(key) { lock_mgr_->TryLock(key_.ToString()); } ~ScopeRecordLock() { lock_mgr_->UnLock(key_.ToString()); } private: - LockMgr* const lock_mgr_; + std::shared_ptr const lock_mgr_; Slice key_; ScopeRecordLock(const ScopeRecordLock&); void operator=(const ScopeRecordLock&); @@ -29,7 +29,7 @@ class ScopeRecordLock { class MultiScopeRecordLock { public: - MultiScopeRecordLock(LockMgr* lock_mgr, const std::vector& keys) : lock_mgr_(lock_mgr), keys_(keys) { + MultiScopeRecordLock(std::shared_ptr lock_mgr, const std::vector& keys) : lock_mgr_(lock_mgr), keys_(keys) { std::string pre_key; std::sort(keys_.begin(), keys_.end()); if (!keys_.empty() && keys_[0].empty()) { @@ -58,7 +58,7 @@ class MultiScopeRecordLock { } private: - LockMgr* const lock_mgr_ = nullptr; + std::shared_ptr const lock_mgr_; std::vector keys_; MultiScopeRecordLock(const MultiScopeRecordLock&); void operator=(const MultiScopeRecordLock&); diff --git a/src/storage/src/storage.cc b/src/storage/src/storage.cc index 337ad12c8..8e42aadd2 100644 --- a/src/storage/src/storage.cc +++ b/src/storage/src/storage.cc @@ -56,7 +56,7 @@ Storage::Storage() current_task_type_(kNone), bg_tasks_should_exit_(false), scan_keynum_exit_(false) { - cursors_store_ = new LRUCache(); + cursors_store_ = std::shared_ptr>(new LRUCache()); cursors_store_->SetCapacity(5000); Status s = StartBGThread(); @@ -81,13 +81,6 @@ Storage::~Storage() { if ((ret = pthread_join(bg_tasks_thread_id_, nullptr)) != 0) { LOG(ERROR) << "pthread_join failed with bgtask thread error " << ret; } - - delete strings_db_; - delete hashes_db_; - delete sets_db_; - delete lists_db_; - delete zsets_db_; - delete cursors_store_; } static std::string AppendSubDirectory(const std::string& db_path, const std::string& sub_db) { @@ -101,31 +94,31 @@ static std::string AppendSubDirectory(const std::string& db_path, const std::str Status Storage::Open(const StorageOptions& storage_options, const std::string& db_path) { mkpath(db_path.c_str(), 0755); - strings_db_ = new RedisStrings(this, kStrings); + strings_db_ = std::shared_ptr(new RedisStrings(this, kStrings)); Status s = strings_db_->Open(storage_options, AppendSubDirectory(db_path, "strings")); if (!s.ok()) { LOG(FATAL) << "open kv db failed, " << s.ToString(); } - hashes_db_ = new RedisHashes(this, kHashes); + hashes_db_ = std::shared_ptr(new RedisHashes(this, kHashes)); s = hashes_db_->Open(storage_options, AppendSubDirectory(db_path, "hashes")); if (!s.ok()) { LOG(FATAL) << "open hashes db failed, " << s.ToString(); } - sets_db_ = new RedisSets(this, kSets); + sets_db_ = std::shared_ptr(new RedisSets(this, kSets)); s = sets_db_->Open(storage_options, AppendSubDirectory(db_path, "sets")); if (!s.ok()) { LOG(FATAL) << "open set db failed, " << s.ToString(); } - lists_db_ = new RedisLists(this, kLists); + lists_db_ = std::shared_ptr(new RedisLists(this, kLists)); s = lists_db_->Open(storage_options, AppendSubDirectory(db_path, "lists")); if (!s.ok()) { LOG(FATAL) << "open list db failed, " << s.ToString(); } - zsets_db_ = new RedisZSets(this, kZSets); + zsets_db_ = std::shared_ptr(new RedisZSets(this, kZSets)); s = zsets_db_->Open(storage_options, AppendSubDirectory(db_path, "zsets")); if (!s.ok()) { LOG(FATAL) << "open zset db failed, " << s.ToString(); @@ -1571,7 +1564,7 @@ Status Storage::CompactKey(const DataType& type, const std::string& key) { } Status Storage::SetMaxCacheStatisticKeys(uint32_t max_cache_statistic_keys) { - std::vector dbs = {sets_db_, zsets_db_, hashes_db_, lists_db_}; + std::vector> dbs = {sets_db_, zsets_db_, hashes_db_, lists_db_}; for (const auto& db : dbs) { db->SetMaxCacheStatisticKeys(max_cache_statistic_keys); } @@ -1579,7 +1572,7 @@ Status Storage::SetMaxCacheStatisticKeys(uint32_t max_cache_statistic_keys) { } Status Storage::SetSmallCompactionThreshold(uint32_t small_compaction_threshold) { - std::vector dbs = {sets_db_, zsets_db_, hashes_db_, lists_db_}; + std::vector> dbs = {sets_db_, zsets_db_, hashes_db_, lists_db_}; for (const auto& db : dbs) { db->SetSmallCompactionThreshold(small_compaction_threshold); } @@ -1650,7 +1643,7 @@ uint64_t Storage::GetProperty(const std::string& db_type, const std::string& pro Status Storage::GetKeyNum(std::vector* key_infos) { KeyInfo key_info; // NOTE: keep the db order with string, hash, list, zset, set - std::vector dbs = {strings_db_, hashes_db_, lists_db_, zsets_db_, sets_db_}; + std::vector> dbs = {strings_db_, hashes_db_, lists_db_, zsets_db_, sets_db_}; for (const auto& db : dbs) { // check the scanner was stopped or not, before scanning the next db if (scan_keynum_exit_) { From c80ecb5d0cf4c8e42b934bf106439f972dc3b7d3 Mon Sep 17 00:00:00 2001 From: cjh <1271435567@qq.com> Date: Thu, 11 May 2023 18:09:14 +0800 Subject: [PATCH 02/17] Eliminated new/delete using smart ptr within src/net/. ps: Not all new/delete are eliminated, as there are some variables that are not suitable for applying smart ptr, such as handle_, response_, request_ etc..., I chose to keep them as they were. --- src/net/examples/binlog_parser_test.cc | 2 +- src/net/examples/http_server.cc | 7 ++----- src/net/examples/https_server.cc | 7 ++----- src/net/examples/mydispatch_srv.cc | 7 ++----- src/net/examples/myholy_srv.cc | 7 ++----- src/net/examples/myholy_srv_chandle.cc | 4 +--- src/net/examples/myproto_cli.cc | 4 ++-- src/net/examples/myredis_cli.cc | 12 ++++++------ src/net/examples/myredis_srv.cc | 8 +++----- src/net/examples/performance/client.cc | 3 +-- src/net/examples/performance/server.cc | 4 +--- src/net/examples/redis_cli_test.cc | 2 +- src/net/examples/redis_parser_test.cc | 2 +- src/net/examples/simple_http_server.cc | 7 ++----- src/net/include/net_cli.h | 4 ++-- src/net/include/server_thread.h | 2 +- src/net/src/dispatch_thread.cc | 21 +++++++++------------ src/net/src/dispatch_thread.h | 2 +- src/net/src/net_cli.cc | 17 +++++++---------- src/net/src/server_thread.cc | 15 +++++---------- 20 files changed, 52 insertions(+), 85 deletions(-) diff --git a/src/net/examples/binlog_parser_test.cc b/src/net/examples/binlog_parser_test.cc index 83594eb9f..9077db4c1 100644 --- a/src/net/examples/binlog_parser_test.cc +++ b/src/net/examples/binlog_parser_test.cc @@ -16,7 +16,7 @@ int main(int argc, char* argv[]) { std::string ip(argv[1]); int port = atoi(argv[2]); - NetCli* rcli = NewRedisCli(); + std::unique_ptr rcli(NewRedisCli()); rcli->set_connect_timeout(3000); Status s = rcli->Connect(ip, port, "127.0.0.1"); diff --git a/src/net/examples/http_server.cc b/src/net/examples/http_server.cc index 04d989b96..634083a43 100644 --- a/src/net/examples/http_server.cc +++ b/src/net/examples/http_server.cc @@ -96,8 +96,8 @@ int main(int argc, char* argv[]) { SignalSetup(); - ConnFactory* my_conn_factory = new MyConnFactory(); - ServerThread* st = NewDispatchThread(port, 4, my_conn_factory, 1000); + std::unique_ptr my_conn_factory = std::make_unique(); + std::unique_ptr st(NewDispatchThread(port, 4, my_conn_factory.get(), 1000)); if (st->StartThread() != 0) { printf("StartThread error happened!\n"); @@ -109,8 +109,5 @@ int main(int argc, char* argv[]) { } st->StopThread(); - delete st; - delete my_conn_factory; - return 0; } diff --git a/src/net/examples/https_server.cc b/src/net/examples/https_server.cc index 0a592cea0..7b7243a82 100644 --- a/src/net/examples/https_server.cc +++ b/src/net/examples/https_server.cc @@ -97,8 +97,8 @@ int main(int argc, char* argv[]) { SignalSetup(); - ConnFactory* my_conn_factory = new MyConnFactory(); - ServerThread* st = NewDispatchThread(port, 4, my_conn_factory, 1000); + std::unique_ptr my_conn_factory = std::make_unique(); + std::unique_ptr st(NewDispatchThread(port, 4, my_conn_factory.get(), 1000)); #if __ENABLE_SSL if (st->EnableSecurity("/complete_path_to/host.crt", "/complete_path_to/host.key") != 0) { @@ -117,8 +117,5 @@ int main(int argc, char* argv[]) { } st->StopThread(); - delete st; - delete my_conn_factory; - return 0; } diff --git a/src/net/examples/mydispatch_srv.cc b/src/net/examples/mydispatch_srv.cc index 1618c4718..23fc591b9 100644 --- a/src/net/examples/mydispatch_srv.cc +++ b/src/net/examples/mydispatch_srv.cc @@ -75,8 +75,8 @@ static void SignalSetup() { int main() { SignalSetup(); - ConnFactory* my_conn_factory = new MyConnFactory(); - ServerThread* st = NewDispatchThread(9211, 10, my_conn_factory, 1000); + std::unique_ptr my_conn_factory = std::make_unique(); + std::unique_ptr st(NewDispatchThread(9211, 10, my_conn_factory.get(), 1000)); if (st->StartThread() != 0) { printf("StartThread error happened!\n"); @@ -88,8 +88,5 @@ int main() { } st->StopThread(); - delete st; - delete my_conn_factory; - return 0; } diff --git a/src/net/examples/myholy_srv.cc b/src/net/examples/myholy_srv.cc index e82a593f5..27607e9a4 100644 --- a/src/net/examples/myholy_srv.cc +++ b/src/net/examples/myholy_srv.cc @@ -80,9 +80,9 @@ int main(int argc, char* argv[]) { SignalSetup(); - ConnFactory* conn_factory = new MyConnFactory(); + std::unique_ptr conn_factory = std::make_unique(); - ServerThread* my_thread = NewHolyThread(my_port, conn_factory); + std::unique_ptr my_thread(NewHolyThread(my_port, conn_factory.get())); if (my_thread->StartThread() != 0) { printf("StartThread error happened!\n"); exit(-1); @@ -93,8 +93,5 @@ int main(int argc, char* argv[]) { } my_thread->StopThread(); - delete my_thread; - delete conn_factory; - return 0; } diff --git a/src/net/examples/myholy_srv_chandle.cc b/src/net/examples/myholy_srv_chandle.cc index 4d64e2412..a6f6b6cd9 100644 --- a/src/net/examples/myholy_srv_chandle.cc +++ b/src/net/examples/myholy_srv_chandle.cc @@ -107,7 +107,7 @@ int main(int argc, char* argv[]) { MyConnFactory conn_factory; MyServerHandle handle; - ServerThread* my_thread = NewHolyThread(my_port, &conn_factory, 1000, &handle); + std::unique_ptr my_thread(NewHolyThread(my_port, &conn_factory, 1000, &handle)); if (my_thread->StartThread() != 0) { printf("StartThread error happened!\n"); exit(-1); @@ -118,7 +118,5 @@ int main(int argc, char* argv[]) { } my_thread->StopThread(); - delete my_thread; - return 0; } diff --git a/src/net/examples/myproto_cli.cc b/src/net/examples/myproto_cli.cc index 898a56eb9..881b2b4f7 100644 --- a/src/net/examples/myproto_cli.cc +++ b/src/net/examples/myproto_cli.cc @@ -2,6 +2,7 @@ #include #include #include +#include #include "myproto.pb.h" #include "net/include/net_cli.h" @@ -18,7 +19,7 @@ int main(int argc, char* argv[]) { std::string ip(argv[1]); int port = atoi(argv[2]); - NetCli* cli = NewPbCli(); + std::unique_ptr cli(NewPbCli()); Status s = cli->Connect(ip, port); if (!s.ok()) { @@ -48,6 +49,5 @@ int main(int argc, char* argv[]) { } cli->Close(); - delete cli; return 0; } diff --git a/src/net/examples/myredis_cli.cc b/src/net/examples/myredis_cli.cc index 80ca33fda..2fb053d07 100644 --- a/src/net/examples/myredis_cli.cc +++ b/src/net/examples/myredis_cli.cc @@ -29,7 +29,7 @@ MyConn::MyConn(int fd, const std::string& ip_port, Thread* thread, void* worker_ // Handle worker_specific_data ... } -ClientThread* client; +std::unique_ptr client; int sendto_port; int MyConn::DealMessage(const RedisCmdArgsType& argv, std::string* response) { sleep(1); @@ -95,10 +95,11 @@ int main(int argc, char* argv[]) { SignalSetup(); - ConnFactory* conn_factory = new MyConnFactory(); + std::unique_ptr conn_factory = std::make_unique(); + //"handle" will be deleted within "client->StopThread()" ClientHandle* handle = new ClientHandle(); - client = new ClientThread(conn_factory, 3000, 60, handle, nullptr); + client = std::make_unique(conn_factory.get(), 3000, 60, handle, nullptr); if (client->StartThread() != 0) { printf("StartThread error happened!\n"); @@ -107,11 +108,10 @@ int main(int argc, char* argv[]) { running.store(true); while (running.load()) { sleep(1); - DoCronWork(client, sendto_port); + DoCronWork(client.get(), sendto_port); } client->StopThread(); - delete client; - delete conn_factory; + client.reset(); return 0; } diff --git a/src/net/examples/myredis_srv.cc b/src/net/examples/myredis_srv.cc index bedc6c9e0..6672a412b 100644 --- a/src/net/examples/myredis_srv.cc +++ b/src/net/examples/myredis_srv.cc @@ -96,9 +96,10 @@ int main(int argc, char* argv[]) { SignalSetup(); - ConnFactory* conn_factory = new MyConnFactory(); + std::unique_ptr conn_factory = std::make_unique(); - ServerThread* my_thread = new HolyThread(my_port, conn_factory, 1000, nullptr, false); + std::unique_ptr my_thread = + std::make_unique(my_port, conn_factory.get(), 1000, nullptr, false); if (my_thread->StartThread() != 0) { printf("StartThread error happened!\n"); exit(-1); @@ -109,8 +110,5 @@ int main(int argc, char* argv[]) { } my_thread->StopThread(); - delete my_thread; - delete conn_factory; - return 0; } diff --git a/src/net/examples/performance/client.cc b/src/net/examples/performance/client.cc index 3f73577ef..a408b0330 100644 --- a/src/net/examples/performance/client.cc +++ b/src/net/examples/performance/client.cc @@ -18,7 +18,7 @@ int main(int argc, char* argv[]) { std::string ip(argv[1]); int port = atoi(argv[2]); - NetCli* cli = NewPbCli(); + std::unique_ptr cli(NewPbCli()); Status s = cli->Connect(ip, port); if (!s.ok()) { @@ -44,6 +44,5 @@ int main(int argc, char* argv[]) { } cli->Close(); - delete cli; return 0; } diff --git a/src/net/examples/performance/server.cc b/src/net/examples/performance/server.cc index 245457bd1..5b7b65cbc 100644 --- a/src/net/examples/performance/server.cc +++ b/src/net/examples/performance/server.cc @@ -84,7 +84,7 @@ int main(int argc, char* argv[]) { SignalSetup(); - ServerThread* st_thread = NewDispatchThread(ip, port, 24, &conn_factory, 1000); + std::unique_ptr st_thread(NewDispatchThread(ip, port, 24, &conn_factory, 1000)); st_thread->StartThread(); uint64_t st, ed; @@ -99,7 +99,5 @@ int main(int argc, char* argv[]) { } st_thread->StopThread(); - delete st_thread; - return 0; } diff --git a/src/net/examples/redis_cli_test.cc b/src/net/examples/redis_cli_test.cc index 6decf3208..6a3f4c925 100644 --- a/src/net/examples/redis_cli_test.cc +++ b/src/net/examples/redis_cli_test.cc @@ -30,7 +30,7 @@ int main(int argc, char* argv[]) { ret = net::SerializeRedisCommand(vec, &str); printf(" 2. Serialize by vec return %d, (%s)\n", ret, str.c_str()); - NetCli* rcli = NewRedisCli(); + std::shared_ptr rcli(NewRedisCli()); rcli->set_connect_timeout(3000); // redis v3.2+ protect mode will block other ip diff --git a/src/net/examples/redis_parser_test.cc b/src/net/examples/redis_parser_test.cc index 0b3cd7252..90bee2869 100644 --- a/src/net/examples/redis_parser_test.cc +++ b/src/net/examples/redis_parser_test.cc @@ -15,7 +15,7 @@ int main(int argc, char* argv[]) { std::string ip(argv[1]); int port = atoi(argv[2]); - NetCli* rcli = NewRedisCli(); + std::unique_ptr rcli(NewRedisCli()); rcli->set_connect_timeout(3000); Status s = rcli->Connect(ip, port, "127.0.0.1"); diff --git a/src/net/examples/simple_http_server.cc b/src/net/examples/simple_http_server.cc index a6cd597a6..73751c95e 100644 --- a/src/net/examples/simple_http_server.cc +++ b/src/net/examples/simple_http_server.cc @@ -76,8 +76,8 @@ int main(int argc, char* argv[]) { SignalSetup(); - ConnFactory* my_conn_factory = new MyConnFactory(); - ServerThread* st = NewDispatchThread(port, 4, my_conn_factory, 1000); + std::unique_ptr my_conn_factory = std::make_unique(); + std::unique_ptr st(NewDispatchThread(port, 4, my_conn_factory.get(), 1000)); if (st->StartThread() != 0) { printf("StartThread error happened!\n"); @@ -89,8 +89,5 @@ int main(int argc, char* argv[]) { } st->StopThread(); - delete st; - delete my_conn_factory; - return 0; } diff --git a/src/net/include/net_cli.h b/src/net/include/net_cli.h index b17881ae3..2e174749c 100644 --- a/src/net/include/net_cli.h +++ b/src/net/include/net_cli.h @@ -6,8 +6,8 @@ #ifndef NET_INCLUDE_NET_CLI_H_ #define NET_INCLUDE_NET_CLI_H_ +#include #include - #include "pstd/include/pstd_status.h" using pstd::Status; @@ -47,7 +47,7 @@ class NetCli { private: struct Rep; - Rep* rep_; + std::shared_ptr rep_; int set_tcp_nodelay(); NetCli(const NetCli&); diff --git a/src/net/include/server_thread.h b/src/net/include/server_thread.h index 387db032f..a445d2954 100644 --- a/src/net/include/server_thread.h +++ b/src/net/include/server_thread.h @@ -188,7 +188,7 @@ class ServerThread : public Thread { */ int port_ = -1; std::set ips_; - std::vector server_sockets_; + std::vector> server_sockets_; std::set server_fds_; virtual int InitHandle(); diff --git a/src/net/src/dispatch_thread.cc b/src/net/src/dispatch_thread.cc index 1035aec44..a3d581a23 100644 --- a/src/net/src/dispatch_thread.cc +++ b/src/net/src/dispatch_thread.cc @@ -20,9 +20,8 @@ DispatchThread::DispatchThread(int port, int work_num, ConnFactory* conn_factory last_thread_(0), work_num_(work_num), queue_limit_(queue_limit) { - worker_thread_ = new WorkerThread*[work_num_]; for (int i = 0; i < work_num_; i++) { - worker_thread_[i] = new WorkerThread(conn_factory, this, queue_limit, cron_interval); + worker_thread_.emplace_back(std::make_shared(conn_factory, this, queue_limit, cron_interval)); } } @@ -32,9 +31,9 @@ DispatchThread::DispatchThread(const std::string& ip, int port, int work_num, Co last_thread_(0), work_num_(work_num), queue_limit_(queue_limit) { - worker_thread_ = new WorkerThread*[work_num_]; + for (int i = 0; i < work_num_; i++) { - worker_thread_[i] = new WorkerThread(conn_factory, this, queue_limit, cron_interval); + worker_thread_.emplace_back(std::make_shared(conn_factory, this, queue_limit, cron_interval)); } } @@ -44,17 +43,15 @@ DispatchThread::DispatchThread(const std::set& ips, int port, int w last_thread_(0), work_num_(work_num), queue_limit_(queue_limit) { - worker_thread_ = new WorkerThread*[work_num_]; + for (int i = 0; i < work_num_; i++) { - worker_thread_[i] = new WorkerThread(conn_factory, this, queue_limit, cron_interval); + worker_thread_.emplace_back(std::make_shared(conn_factory, this, queue_limit, cron_interval)); + } } DispatchThread::~DispatchThread() { - for (int i = 0; i < work_num_; i++) { - delete worker_thread_[i]; - } - delete[] worker_thread_; + } int DispatchThread::StartThread() { @@ -129,7 +126,7 @@ std::shared_ptr DispatchThread::MoveConnOut(int fd) { } void DispatchThread::MoveConnIn(std::shared_ptr conn, const NotifyType& type) { - WorkerThread* worker_thread = worker_thread_[last_thread_]; + std::shared_ptr worker_thread = worker_thread_[last_thread_]; bool success = worker_thread->MoveConnIn(conn, type, true); if (success) { last_thread_ = (last_thread_ + 1) % work_num_; @@ -155,7 +152,7 @@ void DispatchThread::HandleNewConn(const int connfd, const std::string& ip_port) int next_thread = last_thread_; bool find = false; for (int cnt = 0; cnt < work_num_; cnt++) { - WorkerThread* worker_thread = worker_thread_[next_thread]; + std::shared_ptr worker_thread = worker_thread_[next_thread]; find = worker_thread->MoveConnIn(ti, false); if (find) { last_thread_ = (next_thread + 1) % work_num_; diff --git a/src/net/src/dispatch_thread.h b/src/net/src/dispatch_thread.h index a3887b3ae..bf23e16ab 100644 --- a/src/net/src/dispatch_thread.h +++ b/src/net/src/dispatch_thread.h @@ -65,7 +65,7 @@ class DispatchThread : public ServerThread { /* * This is the work threads */ - WorkerThread** worker_thread_; + std::vector> worker_thread_; int queue_limit_; std::map localdata_; diff --git a/src/net/src/net_cli.cc b/src/net/src/net_cli.cc index bb97b3c72..89b4c3a30 100644 --- a/src/net/src/net_cli.cc +++ b/src/net/src/net_cli.cc @@ -48,19 +48,16 @@ struct NetCli::Rep { available(false) {} }; -NetCli::NetCli(const std::string& ip, const int port) : rep_(new Rep(ip, port)) {} +NetCli::NetCli(const std::string& ip, const int port) : rep_(std::make_shared(ip, port)) {} -NetCli::~NetCli() { - Close(); - delete rep_; -} +NetCli::~NetCli() { Close(); } bool NetCli::Available() const { return rep_->available; } Status NetCli::Connect(const std::string& bind_ip) { return Connect(rep_->peer_ip, rep_->peer_port, bind_ip); } Status NetCli::Connect(const std::string& ip, const int port, const std::string& bind_ip) { - Rep* r = rep_; + std::shared_ptr r = rep_; Status s; int rv; char cport[6]; @@ -246,7 +243,7 @@ Status NetCli::SendRaw(void* buf, size_t count) { } Status NetCli::RecvRaw(void* buf, size_t* count) { - Rep* r = rep_; + std::shared_ptr r = rep_; char* rbuf = reinterpret_cast(buf); size_t nleft = *count; size_t pos = 0; @@ -285,7 +282,7 @@ void NetCli::Close() { void NetCli::set_connect_timeout(int connect_timeout) { rep_->connect_timeout = connect_timeout; } int NetCli::set_send_timeout(int send_timeout) { - Rep* r = rep_; + std::shared_ptr r = rep_; int ret = 0; if (send_timeout > 0) { r->send_timeout = send_timeout; @@ -296,7 +293,7 @@ int NetCli::set_send_timeout(int send_timeout) { } int NetCli::set_recv_timeout(int recv_timeout) { - Rep* r = rep_; + std::shared_ptr r = rep_; int ret = 0; if (recv_timeout > 0) { r->recv_timeout = recv_timeout; @@ -307,7 +304,7 @@ int NetCli::set_recv_timeout(int recv_timeout) { } int NetCli::set_tcp_nodelay() { - Rep* r = rep_; + std::shared_ptr r = rep_; int val = 1; int ret = 0; ret = setsockopt(r->sockfd, IPPROTO_TCP, TCP_NODELAY, &val, sizeof(val)); diff --git a/src/net/src/server_thread.cc b/src/net/src/server_thread.cc index 4afa5a20a..c64e5a508 100644 --- a/src/net/src/server_thread.cc +++ b/src/net/src/server_thread.cc @@ -106,9 +106,7 @@ ServerThread::~ServerThread() { EVP_cleanup(); } #endif - for (std::vector::iterator iter = server_sockets_.begin(); iter != server_sockets_.end(); ++iter) { - delete *iter; - } + if (own_handle_) { delete handle_; } @@ -128,14 +126,14 @@ int ServerThread::StartThread() { int ServerThread::InitHandle() { int ret = 0; - ServerSocket* socket_p; + std::shared_ptr socket_p; if (ips_.find("0.0.0.0") != ips_.end()) { ips_.clear(); ips_.insert("0.0.0.0"); } for (std::set::iterator iter = ips_.begin(); iter != ips_.end(); ++iter) { - socket_p = new ServerSocket(port_); - server_sockets_.push_back(socket_p); + socket_p = std::make_shared(port_); + server_sockets_.emplace_back(socket_p); ret = socket_p->Listen(*iter); if (ret != kSuccess) { return ret; @@ -255,9 +253,6 @@ void* ServerThread::ThreadMain() { } } - for (auto iter = server_sockets_.begin(); iter != server_sockets_.end(); iter++) { - delete *iter; - } server_sockets_.clear(); server_fds_.clear(); @@ -312,7 +307,7 @@ int ServerThread::EnableSecurity(const std::string& cert_file, const std::string } if (SSL_CTX_use_PrivateKey_file(ssl_ctx_, key_file.c_str(), SSL_FILETYPE_PEM) != 1) { - LOG(WARNING) << "SSL_CTX_use_PrivateKey_file(" << key_file << ")"; + LOG(WARNING) << "SSL_CTX_use_PrivateKey_file(" << key_file << ")"; return -1; } From 8eb737ef3cd6cc63ad8e7420f44f9696fcabc38c Mon Sep 17 00:00:00 2001 From: laimuxi Date: Fri, 12 May 2023 19:21:20 +0800 Subject: [PATCH 03/17] [feat] replace new/delete with smart pointers --- src/storage/src/backupable.cc | 15 ++++++------ src/storage/src/redis.cc | 7 ++---- src/storage/src/redis.h | 2 -- src/storage/src/redis_hyperloglog.cc | 27 ++++++++------------- src/storage/src/redis_hyperloglog.h | 3 +-- src/storage/src/redis_sets.cc | 2 +- src/storage/src/redis_strings.cc | 6 ++--- src/storage/src/storage.cc | 12 +++++----- src/storage/src/util.cc | 30 +++++++++++++----------- src/storage/src/zsets_filter.h | 2 +- src/storage/tests/lists_filter_test.cc | 18 +++++--------- src/storage/tests/lock_mgr_test.cc | 4 ++-- src/storage/tests/strings_filter_test.cc | 4 +--- 13 files changed, 55 insertions(+), 77 deletions(-) diff --git a/src/storage/src/backupable.cc b/src/storage/src/backupable.cc index 317d78e01..e45f0fb4a 100644 --- a/src/storage/src/backupable.cc +++ b/src/storage/src/backupable.cc @@ -120,13 +120,15 @@ Status BackupEngine::WaitBackupPthread() { Status BackupEngine::CreateNewBackup(const std::string& dir) { Status s = Status::OK(); - std::vector args; + // ensure cleaning up the pointers after the function has finished. + std::vector> args; + args.reserve(engines_.size()); for (const auto& engine : engines_) { pthread_t tid; - BackupSaveArgs* arg = new BackupSaveArgs(reinterpret_cast(this), dir, engine.first); - args.push_back(arg); - if (pthread_create(&tid, nullptr, &ThreadFuncSaveSpecify, arg) != 0) { - s = Status::Corruption("pthead_create failed."); + std::unique_ptr arg = std::make_unique(reinterpret_cast(this), dir, engine.first); + args.push_back(std::move(arg)); + if (pthread_create(&tid, nullptr, &ThreadFuncSaveSpecify, args.back().get()) != 0) { + s = Status::Corruption("pthread_create failed."); break; } if (!(backup_pthread_ts_.insert(std::make_pair(engine.first, tid)).second)) { @@ -140,9 +142,6 @@ Status BackupEngine::CreateNewBackup(const std::string& dir) { } s = WaitBackupPthread(); - for (auto& a : args) { - delete a; - } return s; } diff --git a/src/storage/src/redis.cc b/src/storage/src/redis.cc index c63df09c9..9b79ccfef 100644 --- a/src/storage/src/redis.cc +++ b/src/storage/src/redis.cc @@ -13,8 +13,8 @@ Redis::Redis(Storage* const s, const DataType& type) lock_mgr_(new LockMgr(1000, 0, std::make_shared())), db_(nullptr), small_compaction_threshold_(5000) { - statistics_store_ = std::shared_ptr> (new LRUCache()); - scan_cursors_store_ = std::shared_ptr>(new LRUCache()); + statistics_store_ = std::make_shared>(); + scan_cursors_store_ = std::make_shared>(); scan_cursors_store_->SetCapacity(5000); default_compact_range_options_.exclusive_manual_compaction = false; default_compact_range_options_.change_level = true; @@ -28,9 +28,6 @@ Redis::~Redis() { delete handle; } delete db_; - // delete lock_mgr_; - // delete statistics_store_; - // delete scan_cursors_store_; } Status Redis::GetScanStartPoint(const Slice& key, const Slice& pattern, int64_t cursor, std::string* start_point) { diff --git a/src/storage/src/redis.h b/src/storage/src/redis.h index a3a0e6ab0..4dba8aefb 100644 --- a/src/storage/src/redis.h +++ b/src/storage/src/redis.h @@ -58,10 +58,8 @@ class Redis { protected: Storage* const storage_; DataType type_; - // LockMgr* lock_mgr_ = nullptr; std::shared_ptr lock_mgr_; rocksdb::DB* db_ = nullptr; - // std::shared_ptr db_; std::vector handles_; rocksdb::WriteOptions default_write_options_; diff --git a/src/storage/src/redis_hyperloglog.cc b/src/storage/src/redis_hyperloglog.cc index b252b94a8..5b2e7d3c1 100644 --- a/src/storage/src/redis_hyperloglog.cc +++ b/src/storage/src/redis_hyperloglog.cc @@ -17,17 +17,15 @@ HyperLogLog::HyperLogLog(uint8_t precision, std::string origin_register) { b_ = precision; m_ = 1 << precision; alpha_ = Alpha(); - // register_ = std::shared_ptr(new char[m_], [](char* ptr) { delete[] ptr; }); - auto register_ptr = new char[m_]; + register_ = std::make_unique(m_); for (uint32_t i = 0; i < m_; ++i) { - register_ptr[i] = 0; + register_[i] = 0; } if (origin_register != "") { for (uint32_t i = 0; i < m_; ++i) { - register_ptr[i] = origin_register[i]; + register_[i] = origin_register[i]; } } - register_ = std::shared_ptr(register_ptr, [](char* ptr) { delete[] ptr; }); } HyperLogLog::~HyperLogLog() {} @@ -37,11 +35,10 @@ std::string HyperLogLog::Add(const char* value, uint32_t len) { MurmurHash3_x86_32(value, len, HLL_HASH_SEED, static_cast(&hash_value)); int32_t index = hash_value & ((1 << b_) - 1); uint8_t rank = Nctz((hash_value >> b_), 32 - b_); - auto register_ptr = register_.get(); - if (rank > register_ptr[index]) register_ptr[index] = rank; + if (rank > register_[index]) register_[index] = rank; std::string result(m_, 0); for (uint32_t i = 0; i < m_; ++i) { - result[i] = register_ptr[i]; + result[i] = register_[i]; } return result; } @@ -61,9 +58,8 @@ double HyperLogLog::Estimate() const { double HyperLogLog::FirstEstimate() const { double estimate, sum = 0.0; - auto register_ptr = register_.get(); for (uint32_t i = 0; i < m_; i++) { - sum += 1.0 / (1 << register_ptr[i]); + sum += 1.0 / (1 << register_[i]); } estimate = alpha_ * m_ * m_ / sum; @@ -85,9 +81,8 @@ double HyperLogLog::Alpha() const { uint32_t HyperLogLog::CountZero() const { uint32_t count = 0; - auto register_ptr = register_.get(); for (uint32_t i = 0; i < m_; i++) { - if (register_ptr[i] == 0) { + if (register_[i] == 0) { count++; } } @@ -98,17 +93,15 @@ std::string HyperLogLog::Merge(const HyperLogLog& hll) { if (m_ != hll.m_) { // TODO(shq) the number of registers doesn't match } - auto register_ptr = register_.get(); - auto hll_register_ptr = hll.register_.get(); for (uint32_t r = 0; r < m_; r++) { - if (register_ptr[r] < hll_register_ptr[r]) { - register_ptr[r] |= hll_register_ptr[r]; + if (register_[r] < hll.register_[r]) { + register_[r] |= hll.register_[r]; } } std::string result(m_, 0); for (uint32_t i = 0; i < m_; ++i) { - result[i] = register_ptr[i]; + result[i] = register_[i]; } return result; } diff --git a/src/storage/src/redis_hyperloglog.h b/src/storage/src/redis_hyperloglog.h index ec5eba0db..cd5529317 100644 --- a/src/storage/src/redis_hyperloglog.h +++ b/src/storage/src/redis_hyperloglog.h @@ -30,8 +30,7 @@ class HyperLogLog { uint32_t m_ = 0; // register bit width uint32_t b_ = 0; // regieter size double alpha_ = 0; - // char* register_ = nullptr; // register; - std::shared_ptr register_; + std::unique_ptr register_; }; } // namespace storage diff --git a/src/storage/src/redis_sets.cc b/src/storage/src/redis_sets.cc index ec9110511..d27495f85 100644 --- a/src/storage/src/redis_sets.cc +++ b/src/storage/src/redis_sets.cc @@ -21,7 +21,7 @@ namespace storage { RedisSets::RedisSets(Storage* const s, const DataType& type) : Redis(s, type) { - spop_counts_store_ = std::shared_ptr>(new LRUCache()); + spop_counts_store_ = std::make_shared>(); spop_counts_store_->SetCapacity(1000); } diff --git a/src/storage/src/redis_strings.cc b/src/storage/src/redis_strings.cc index 01a8a7ccb..8cff276a7 100644 --- a/src/storage/src/redis_strings.cc +++ b/src/storage/src/redis_strings.cc @@ -248,8 +248,7 @@ Status RedisStrings::BitCount(const Slice& key, int64_t start_offset, int64_t en } std::string BitOpOperate(BitOpType op, const std::vector& src_values, int64_t max_len) { - char* dest_value = new char[max_len]; - + std::unique_ptr dest_value = std::make_unique(max_len); char byte, output; for (int64_t j = 0; j < max_len; j++) { if (j < static_cast(src_values[0].size())) { @@ -284,8 +283,7 @@ std::string BitOpOperate(BitOpType op, const std::vector& src_value } dest_value[j] = output; } - std::string dest_str(dest_value, max_len); - delete[] dest_value; + std::string dest_str(dest_value.get(), max_len); return dest_str; } diff --git a/src/storage/src/storage.cc b/src/storage/src/storage.cc index 8e42aadd2..c7fd39264 100644 --- a/src/storage/src/storage.cc +++ b/src/storage/src/storage.cc @@ -56,7 +56,7 @@ Storage::Storage() current_task_type_(kNone), bg_tasks_should_exit_(false), scan_keynum_exit_(false) { - cursors_store_ = std::shared_ptr>(new LRUCache()); + cursors_store_ = std::make_shared>(); cursors_store_->SetCapacity(5000); Status s = StartBGThread(); @@ -94,31 +94,31 @@ static std::string AppendSubDirectory(const std::string& db_path, const std::str Status Storage::Open(const StorageOptions& storage_options, const std::string& db_path) { mkpath(db_path.c_str(), 0755); - strings_db_ = std::shared_ptr(new RedisStrings(this, kStrings)); + strings_db_ = std::make_shared(this, kStrings); Status s = strings_db_->Open(storage_options, AppendSubDirectory(db_path, "strings")); if (!s.ok()) { LOG(FATAL) << "open kv db failed, " << s.ToString(); } - hashes_db_ = std::shared_ptr(new RedisHashes(this, kHashes)); + hashes_db_ = std::make_shared(this, kHashes); s = hashes_db_->Open(storage_options, AppendSubDirectory(db_path, "hashes")); if (!s.ok()) { LOG(FATAL) << "open hashes db failed, " << s.ToString(); } - sets_db_ = std::shared_ptr(new RedisSets(this, kSets)); + sets_db_ = std::make_shared(this, kSets); s = sets_db_->Open(storage_options, AppendSubDirectory(db_path, "sets")); if (!s.ok()) { LOG(FATAL) << "open set db failed, " << s.ToString(); } - lists_db_ = std::shared_ptr(new RedisLists(this, kLists)); + lists_db_ = std::make_shared(this, kLists); s = lists_db_->Open(storage_options, AppendSubDirectory(db_path, "lists")); if (!s.ok()) { LOG(FATAL) << "open list db failed, " << s.ToString(); } - zsets_db_ = std::shared_ptr(new RedisZSets(this, kZSets)); + zsets_db_ = std::make_shared(this, kZSets); s = zsets_db_->Open(storage_options, AppendSubDirectory(db_path, "zsets")); if (!s.ok()) { LOG(FATAL) << "open zset db failed, " << s.ToString(); diff --git a/src/storage/src/util.cc b/src/storage/src/util.cc index 7bfaf6276..2b2465209 100644 --- a/src/storage/src/util.cc +++ b/src/storage/src/util.cc @@ -195,29 +195,31 @@ int is_dir(const char* filename) { int CalculateMetaStartAndEndKey(const std::string& key, std::string* meta_start_key, std::string* meta_end_key) { size_t needed = key.size() + 1; - char* dst = new char[needed]; - const char* start = dst; - memcpy(dst, key.data(), key.size()); - dst += key.size(); + std::unique_ptr dst(new char[needed]); + const char* start = dst.get(); + std::memcpy(dst.get(), key.data(), key.size()); + char* dst_ptr = dst.get() + key.size(); meta_start_key->assign(start, key.size()); - *dst = static_cast(0xff); + *dst_ptr = static_cast(0xff); meta_end_key->assign(start, key.size() + 1); - delete[] start; return 0; } int CalculateDataStartAndEndKey(const std::string& key, std::string* data_start_key, std::string* data_end_key) { size_t needed = sizeof(int32_t) + key.size() + 1; - char* dst = new char[needed]; - const char* start = dst; - EncodeFixed32(dst, key.size()); - dst += sizeof(int32_t); - memcpy(dst, key.data(), key.size()); - dst += key.size(); + std::unique_ptr dst(new char[needed]); + const char* start = dst.get(); + char* dst_ptr = dst.get(); + + EncodeFixed32(dst_ptr, key.size()); + dst_ptr += sizeof(int32_t); + std::memcpy(dst_ptr, key.data(), key.size()); + dst_ptr += key.size(); + *dst_ptr = static_cast(0xff); + data_start_key->assign(start, sizeof(int32_t) + key.size()); - *dst = static_cast(0xff); data_end_key->assign(start, sizeof(int32_t) + key.size() + 1); - delete[] start; + return 0; } diff --git a/src/storage/src/zsets_filter.h b/src/storage/src/zsets_filter.h index 463462b2b..ca8b8ce49 100644 --- a/src/storage/src/zsets_filter.h +++ b/src/storage/src/zsets_filter.h @@ -92,7 +92,7 @@ class ZSetsScoreFilterFactory : public rocksdb::CompactionFilterFactory { std::unique_ptr CreateCompactionFilter( const rocksdb::CompactionFilter::Context& context) override { - return std::unique_ptr(new ZSetsScoreFilter(*db_ptr_, cf_handles_ptr_)); + return std::make_unique(*db_ptr_, cf_handles_ptr_); } const char* Name() const override { return "ZSetsScoreFilterFactory"; } diff --git a/src/storage/tests/lists_filter_test.cc b/src/storage/tests/lists_filter_test.cc index 01231d1b1..951c3dbbc 100644 --- a/src/storage/tests/lists_filter_test.cc +++ b/src/storage/tests/lists_filter_test.cc @@ -67,7 +67,7 @@ TEST_F(ListsFilterTest, MetaFilterTest) { std::string new_value; // Test Meta Filter - ListsMetaFilter* lists_meta_filter = new ListsMetaFilter(); + std::unique_ptr lists_meta_filter = std::make_unique(); ASSERT_TRUE(lists_meta_filter != nullptr); // Timeout timestamp is not set, but it's an empty list. @@ -108,7 +108,6 @@ TEST_F(ListsFilterTest, MetaFilterTest) { filter_result = lists_meta_filter->Filter(0, "FILTER_TEST_KEY", lists_meta_value4.Encode(), &new_value, &value_changed); ASSERT_EQ(filter_result, true); - delete lists_meta_filter; } // Data Filter @@ -120,7 +119,7 @@ TEST_F(ListsFilterTest, DataFilterTest) { std::string new_value; // Timeout timestamp is not set, the version is valid. - ListsDataFilter* lists_data_filter1 = new ListsDataFilter(meta_db, &handles); + std::unique_ptr lists_data_filter1 = std::make_unique(meta_db, &handles); ASSERT_TRUE(lists_data_filter1 != nullptr); EncodeFixed64(str, 1); @@ -135,10 +134,9 @@ TEST_F(ListsFilterTest, DataFilterTest) { ASSERT_EQ(filter_result, false); s = meta_db->Delete(rocksdb::WriteOptions(), handles[0], "FILTER_TEST_KEY"); ASSERT_TRUE(s.ok()); - delete lists_data_filter1; // Timeout timestamp is set, but not expired. - ListsDataFilter* lists_data_filter2 = new ListsDataFilter(meta_db, &handles); + std::unique_ptr lists_data_filter2 = std::make_unique(meta_db, &handles); ASSERT_TRUE(lists_data_filter2 != nullptr); EncodeFixed64(str, 1); @@ -153,10 +151,9 @@ TEST_F(ListsFilterTest, DataFilterTest) { ASSERT_EQ(filter_result, false); s = meta_db->Delete(rocksdb::WriteOptions(), handles[0], "FILTER_TEST_KEY"); ASSERT_TRUE(s.ok()); - delete lists_data_filter2; // Timeout timestamp is set, already expired. - ListsDataFilter* lists_data_filter3 = new ListsDataFilter(meta_db, &handles); + std::unique_ptr lists_data_filter3 = std::make_unique(meta_db, &handles); ASSERT_TRUE(lists_data_filter3 != nullptr); EncodeFixed64(str, 1); @@ -172,10 +169,9 @@ TEST_F(ListsFilterTest, DataFilterTest) { ASSERT_EQ(filter_result, true); s = meta_db->Delete(rocksdb::WriteOptions(), handles[0], "FILTER_TEST_KEY"); ASSERT_TRUE(s.ok()); - delete lists_data_filter3; // Timeout timestamp is not set, the version is invalid - ListsDataFilter* lists_data_filter4 = new ListsDataFilter(meta_db, &handles); + std::unique_ptr lists_data_filter4 = std::make_unique(meta_db, &handles); ASSERT_TRUE(lists_data_filter4 != nullptr); EncodeFixed64(str, 1); @@ -192,10 +188,9 @@ TEST_F(ListsFilterTest, DataFilterTest) { ASSERT_EQ(filter_result, true); s = meta_db->Delete(rocksdb::WriteOptions(), handles[0], "FILTER_TEST_KEY"); ASSERT_TRUE(s.ok()); - delete lists_data_filter4; // Meta data has been clear - ListsDataFilter* lists_data_filter5 = new ListsDataFilter(meta_db, &handles); + std::unique_ptr lists_data_filter5 = std::make_unique(meta_db, &handles); ASSERT_TRUE(lists_data_filter5 != nullptr); EncodeFixed64(str, 1); @@ -209,7 +204,6 @@ TEST_F(ListsFilterTest, DataFilterTest) { filter_result = lists_data_filter5->Filter(0, lists_data_value5.Encode(), "FILTER_TEST_VALUE", &new_value, &value_changed); ASSERT_EQ(filter_result, true); - delete lists_data_filter5; } int main(int argc, char** argv) { diff --git a/src/storage/tests/lock_mgr_test.cc b/src/storage/tests/lock_mgr_test.cc index d6dedbb07..965ecdb98 100644 --- a/src/storage/tests/lock_mgr_test.cc +++ b/src/storage/tests/lock_mgr_test.cc @@ -19,8 +19,8 @@ void Func(LockMgr* mgr, int id, const std::string& key) { } int main() { - MutexFactory* factory = new MutexFactoryImpl; - LockMgr mgr(1, 3, std::shared_ptr(factory)); + std::shared_ptr factory = std::make_shared(); + LockMgr mgr(1, 3, factory); std::thread t1(Func, &mgr, 1, "key_1"); std::this_thread::sleep_for(std::chrono::milliseconds(100)); diff --git a/src/storage/tests/strings_filter_test.cc b/src/storage/tests/strings_filter_test.cc index c4d95dcc4..e91b649eb 100644 --- a/src/storage/tests/strings_filter_test.cc +++ b/src/storage/tests/strings_filter_test.cc @@ -16,7 +16,7 @@ using namespace storage; TEST(StringsFilterTest, FilterTest) { std::string new_value; bool is_stale, value_changed; - StringsFilter* filter = new StringsFilter; + std::unique_ptr filter = std::make_unique(); int32_t ttl = 1; StringsValue strings_value("FILTER_VALUE"); @@ -26,8 +26,6 @@ TEST(StringsFilterTest, FilterTest) { std::this_thread::sleep_for(std::chrono::milliseconds(2000)); is_stale = filter->Filter(0, "FILTER_KEY", strings_value.Encode(), &new_value, &value_changed); ASSERT_TRUE(is_stale); - - delete filter; } int main(int argc, char** argv) { From fa652de9cfe78746a703cd926179e21d65d9b682 Mon Sep 17 00:00:00 2001 From: laimuxi <1721261216@qq.com> Date: Fri, 12 May 2023 22:11:20 +0800 Subject: [PATCH 04/17] fix build --- src/storage/include/storage/storage.h | 2 +- src/storage/src/storage.cc | 2 +- src/storage/src/util.cc | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/storage/include/storage/storage.h b/src/storage/include/storage/storage.h index 56b3fb728..c213e9464 100644 --- a/src/storage/include/storage/storage.h +++ b/src/storage/include/storage/storage.h @@ -1027,7 +1027,7 @@ class Storage { std::shared_ptr lists_db_; std::atomic is_opened_; - std::shared_ptr> cursors_store_; + std::unique_ptr> cursors_store_; // Storage start the background thread for compaction task pthread_t bg_tasks_thread_id_; diff --git a/src/storage/src/storage.cc b/src/storage/src/storage.cc index c7fd39264..e43fd7bd4 100644 --- a/src/storage/src/storage.cc +++ b/src/storage/src/storage.cc @@ -56,7 +56,7 @@ Storage::Storage() current_task_type_(kNone), bg_tasks_should_exit_(false), scan_keynum_exit_(false) { - cursors_store_ = std::make_shared>(); + cursors_store_ = std::make_unique>(); cursors_store_->SetCapacity(5000); Status s = StartBGThread(); diff --git a/src/storage/src/util.cc b/src/storage/src/util.cc index 2b2465209..94740acb5 100644 --- a/src/storage/src/util.cc +++ b/src/storage/src/util.cc @@ -7,7 +7,7 @@ #include #include #include - +#include #include "pstd/include/pstd_string.h" #include "src/coding.h" From c4edc27009f520496c572f2a73e37d62af122f8e Mon Sep 17 00:00:00 2001 From: laimuxi <1721261216@qq.com> Date: Fri, 12 May 2023 23:50:51 +0800 Subject: [PATCH 05/17] change shared_ptr to unique_ptr --- src/storage/src/backupable.cc | 2 +- src/storage/src/redis.cc | 4 ++-- src/storage/src/redis.h | 4 ++-- src/storage/src/redis_sets.cc | 2 +- src/storage/src/redis_sets.h | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/storage/src/backupable.cc b/src/storage/src/backupable.cc index e45f0fb4a..8e0e48e0a 100644 --- a/src/storage/src/backupable.cc +++ b/src/storage/src/backupable.cc @@ -125,7 +125,7 @@ Status BackupEngine::CreateNewBackup(const std::string& dir) { args.reserve(engines_.size()); for (const auto& engine : engines_) { pthread_t tid; - std::unique_ptr arg = std::make_unique(reinterpret_cast(this), dir, engine.first); + auto arg = std::make_unique(reinterpret_cast(this), dir, engine.first); args.push_back(std::move(arg)); if (pthread_create(&tid, nullptr, &ThreadFuncSaveSpecify, args.back().get()) != 0) { s = Status::Corruption("pthread_create failed."); diff --git a/src/storage/src/redis.cc b/src/storage/src/redis.cc index 9b79ccfef..ddaddd3a8 100644 --- a/src/storage/src/redis.cc +++ b/src/storage/src/redis.cc @@ -13,8 +13,8 @@ Redis::Redis(Storage* const s, const DataType& type) lock_mgr_(new LockMgr(1000, 0, std::make_shared())), db_(nullptr), small_compaction_threshold_(5000) { - statistics_store_ = std::make_shared>(); - scan_cursors_store_ = std::make_shared>(); + statistics_store_ = std::make_unique>(); + scan_cursors_store_ = std::make_unique>(); scan_cursors_store_->SetCapacity(5000); default_compact_range_options_.exclusive_manual_compaction = false; default_compact_range_options_.change_level = true; diff --git a/src/storage/src/redis.h b/src/storage/src/redis.h index 4dba8aefb..a21a34110 100644 --- a/src/storage/src/redis.h +++ b/src/storage/src/redis.h @@ -67,14 +67,14 @@ class Redis { rocksdb::CompactRangeOptions default_compact_range_options_; // For Scan - std::shared_ptr> scan_cursors_store_; + std::unique_ptr> scan_cursors_store_; Status GetScanStartPoint(const Slice& key, const Slice& pattern, int64_t cursor, std::string* start_point); Status StoreScanNextPoint(const Slice& key, const Slice& pattern, int64_t cursor, const std::string& next_point); // For Statistics std::atomic small_compaction_threshold_; - std::shared_ptr> statistics_store_; + std::unique_ptr> statistics_store_; Status UpdateSpecificKeyStatistics(const std::string& key, size_t count); Status AddCompactKeyTaskIfNeeded(const std::string& key, size_t total); diff --git a/src/storage/src/redis_sets.cc b/src/storage/src/redis_sets.cc index d27495f85..88f1fbc52 100644 --- a/src/storage/src/redis_sets.cc +++ b/src/storage/src/redis_sets.cc @@ -21,7 +21,7 @@ namespace storage { RedisSets::RedisSets(Storage* const s, const DataType& type) : Redis(s, type) { - spop_counts_store_ = std::make_shared>(); + spop_counts_store_ = std::make_unique>(); spop_counts_store_->SetCapacity(1000); } diff --git a/src/storage/src/redis_sets.h b/src/storage/src/redis_sets.h index 4e9727f88..4043f4387 100644 --- a/src/storage/src/redis_sets.h +++ b/src/storage/src/redis_sets.h @@ -73,7 +73,7 @@ class RedisSets : public Redis { private: // For compact in time after multiple spop - std::shared_ptr> spop_counts_store_; + std::unique_ptr> spop_counts_store_; Status ResetSpopCount(const std::string& key); Status AddAndGetSpopCount(const std::string& key, uint64_t* count); }; From 2f3d1be1a3940efa6c804a86f61e2031346892e7 Mon Sep 17 00:00:00 2001 From: laimuxi <1721261216@qq.com> Date: Fri, 12 May 2023 23:56:47 +0800 Subject: [PATCH 06/17] change to auto --- src/storage/src/redis_strings.cc | 2 +- src/storage/tests/lists_filter_test.cc | 12 ++++++------ src/storage/tests/strings_filter_test.cc | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/storage/src/redis_strings.cc b/src/storage/src/redis_strings.cc index 8cff276a7..565fb5fbe 100644 --- a/src/storage/src/redis_strings.cc +++ b/src/storage/src/redis_strings.cc @@ -248,7 +248,7 @@ Status RedisStrings::BitCount(const Slice& key, int64_t start_offset, int64_t en } std::string BitOpOperate(BitOpType op, const std::vector& src_values, int64_t max_len) { - std::unique_ptr dest_value = std::make_unique(max_len); + auto dest_value = std::make_unique(max_len); char byte, output; for (int64_t j = 0; j < max_len; j++) { if (j < static_cast(src_values[0].size())) { diff --git a/src/storage/tests/lists_filter_test.cc b/src/storage/tests/lists_filter_test.cc index 951c3dbbc..d7699fd8d 100644 --- a/src/storage/tests/lists_filter_test.cc +++ b/src/storage/tests/lists_filter_test.cc @@ -67,7 +67,7 @@ TEST_F(ListsFilterTest, MetaFilterTest) { std::string new_value; // Test Meta Filter - std::unique_ptr lists_meta_filter = std::make_unique(); + auto lists_meta_filter = std::make_unique(); ASSERT_TRUE(lists_meta_filter != nullptr); // Timeout timestamp is not set, but it's an empty list. @@ -119,7 +119,7 @@ TEST_F(ListsFilterTest, DataFilterTest) { std::string new_value; // Timeout timestamp is not set, the version is valid. - std::unique_ptr lists_data_filter1 = std::make_unique(meta_db, &handles); + auto lists_data_filter1 = std::make_unique(meta_db, &handles); ASSERT_TRUE(lists_data_filter1 != nullptr); EncodeFixed64(str, 1); @@ -136,7 +136,7 @@ TEST_F(ListsFilterTest, DataFilterTest) { ASSERT_TRUE(s.ok()); // Timeout timestamp is set, but not expired. - std::unique_ptr lists_data_filter2 = std::make_unique(meta_db, &handles); + auto lists_data_filter2 = std::make_unique(meta_db, &handles); ASSERT_TRUE(lists_data_filter2 != nullptr); EncodeFixed64(str, 1); @@ -153,7 +153,7 @@ TEST_F(ListsFilterTest, DataFilterTest) { ASSERT_TRUE(s.ok()); // Timeout timestamp is set, already expired. - std::unique_ptr lists_data_filter3 = std::make_unique(meta_db, &handles); + auto lists_data_filter3 = std::make_unique(meta_db, &handles); ASSERT_TRUE(lists_data_filter3 != nullptr); EncodeFixed64(str, 1); @@ -171,7 +171,7 @@ TEST_F(ListsFilterTest, DataFilterTest) { ASSERT_TRUE(s.ok()); // Timeout timestamp is not set, the version is invalid - std::unique_ptr lists_data_filter4 = std::make_unique(meta_db, &handles); + auto lists_data_filter4 = std::make_unique(meta_db, &handles); ASSERT_TRUE(lists_data_filter4 != nullptr); EncodeFixed64(str, 1); @@ -190,7 +190,7 @@ TEST_F(ListsFilterTest, DataFilterTest) { ASSERT_TRUE(s.ok()); // Meta data has been clear - std::unique_ptr lists_data_filter5 = std::make_unique(meta_db, &handles); + auto lists_data_filter5 = std::make_unique(meta_db, &handles); ASSERT_TRUE(lists_data_filter5 != nullptr); EncodeFixed64(str, 1); diff --git a/src/storage/tests/strings_filter_test.cc b/src/storage/tests/strings_filter_test.cc index e91b649eb..658757626 100644 --- a/src/storage/tests/strings_filter_test.cc +++ b/src/storage/tests/strings_filter_test.cc @@ -16,7 +16,7 @@ using namespace storage; TEST(StringsFilterTest, FilterTest) { std::string new_value; bool is_stale, value_changed; - std::unique_ptr filter = std::make_unique(); + auto filter = std::make_unique(); int32_t ttl = 1; StringsValue strings_value("FILTER_VALUE"); From f2d2980d81a972df3fd42d5ce19c7cc08a64f246 Mon Sep 17 00:00:00 2001 From: laimuxi <1721261216@qq.com> Date: Sat, 13 May 2023 16:54:27 +0800 Subject: [PATCH 07/17] feat --- include/pika_partition.h | 7 ++++--- src/pika_partition.cc | 11 +++++------ src/pstd/include/scope_record_lock.h | 12 ++++++------ src/pstd/src/rsync.cc | 2 +- src/pstd/src/scope_record_lock.cc | 2 +- src/storage/include/storage/backupable.h | 5 +++-- src/storage/src/backupable.cc | 23 ++++++++++------------- 7 files changed, 30 insertions(+), 32 deletions(-) diff --git a/include/pika_partition.h b/include/pika_partition.h index 400b08e42..bade7fbeb 100644 --- a/include/pika_partition.h +++ b/include/pika_partition.h @@ -62,7 +62,7 @@ class Partition : public std::enable_shared_from_this { void DbRWLockReader(); void DbRWUnLock(); - pstd::lock::LockMgr* LockMgr(); + std::shared_ptr LockMgr(); void PrepareRsync(); bool TryUpdateMasterOffset(); @@ -97,7 +97,7 @@ class Partition : public std::enable_shared_from_this { bool opened_ = false; std::shared_mutex db_rwlock_; - pstd::lock::LockMgr* lock_mgr_ = nullptr; + std::shared_ptr lock_mgr_ = nullptr; std::shared_ptr db_; bool full_sync_ = false; @@ -116,7 +116,7 @@ class Partition : public std::enable_shared_from_this { void FinishBgsave(); BgSaveInfo bgsave_info_; pstd::Mutex bgsave_protector_; - storage::BackupEngine* bgsave_engine_; + std::unique_ptr bgsave_engine_; // key scan info use void InitKeyScan(); @@ -129,3 +129,4 @@ class Partition : public std::enable_shared_from_this { }; #endif + diff --git a/src/pika_partition.cc b/src/pika_partition.cc index 6465d6332..f1b1c307c 100644 --- a/src/pika_partition.cc +++ b/src/pika_partition.cc @@ -58,7 +58,7 @@ Partition::Partition(const std::string& table_name, uint32_t partition_id, const db_ = std::shared_ptr(new storage::Storage()); rocksdb::Status s = db_->Open(g_pika_server->storage_options(), db_path_); - lock_mgr_ = new pstd::lock::LockMgr(1000, 0, std::make_shared()); + lock_mgr_ = std::make_shared(1000, 0, std::make_shared()); opened_ = s.ok() ? true : false; assert(db_); @@ -68,8 +68,6 @@ Partition::Partition(const std::string& table_name, uint32_t partition_id, const Partition::~Partition() { Close(); - delete bgsave_engine_; - delete lock_mgr_; } void Partition::Leave() { @@ -126,7 +124,7 @@ void Partition::DbRWLockReader() { db_rwlock_.lock_shared(); } void Partition::DbRWUnLock() { db_rwlock_.unlock(); } -pstd::lock::LockMgr* Partition::LockMgr() { return lock_mgr_; } +std::shared_ptr Partition::LockMgr() { return lock_mgr_; } void Partition::PrepareRsync() { pstd::DeleteDirIfExist(dbsync_path_); @@ -371,8 +369,8 @@ bool Partition::InitBgsaveEnv() { // Prepare bgsave env, need bgsave_protector protect bool Partition::InitBgsaveEngine() { - delete bgsave_engine_; - rocksdb::Status s = storage::BackupEngine::Open(db().get(), &bgsave_engine_); + bgsave_engine_ = nullptr; + rocksdb::Status s = storage::BackupEngine::Open(db().get(), bgsave_engine_); if (!s.ok()) { LOG(WARNING) << partition_name_ << " open backup engine failed " << s.ToString(); return false; @@ -503,3 +501,4 @@ Status Partition::GetKeyNum(std::vector* key_info) { key_scan_info_.duration = time(nullptr) - key_scan_info_.start_time; return Status::OK(); } + diff --git a/src/pstd/include/scope_record_lock.h b/src/pstd/include/scope_record_lock.h index b91616b06..a63f3448c 100644 --- a/src/pstd/include/scope_record_lock.h +++ b/src/pstd/include/scope_record_lock.h @@ -18,13 +18,13 @@ namespace lock { class ScopeRecordLock { public: - ScopeRecordLock(LockMgr* lock_mgr, const Slice& key) : lock_mgr_(lock_mgr), key_(key) { + ScopeRecordLock(std::shared_ptr lock_mgr, const Slice& key) : lock_mgr_(lock_mgr), key_(key) { lock_mgr_->TryLock(key_.ToString()); } ~ScopeRecordLock() { lock_mgr_->UnLock(key_.ToString()); } private: - LockMgr* const lock_mgr_; + std::shared_ptr const lock_mgr_; Slice key_; ScopeRecordLock(const ScopeRecordLock&); void operator=(const ScopeRecordLock&); @@ -32,11 +32,11 @@ class ScopeRecordLock { class MultiScopeRecordLock { public: - MultiScopeRecordLock(LockMgr* lock_mgr, const std::vector& keys); + MultiScopeRecordLock(std::shared_ptr lock_mgr, const std::vector& keys); ~MultiScopeRecordLock(); private: - LockMgr* const lock_mgr_; + std::shared_ptr const lock_mgr_; std::vector keys_; MultiScopeRecordLock(const MultiScopeRecordLock&); void operator=(const MultiScopeRecordLock&); @@ -44,13 +44,13 @@ class MultiScopeRecordLock { class MultiRecordLock { public: - explicit MultiRecordLock(LockMgr* lock_mgr) : lock_mgr_(lock_mgr) {} + explicit MultiRecordLock(std::shared_ptr lock_mgr) : lock_mgr_(lock_mgr) {} ~MultiRecordLock() {} void Lock(const std::vector& keys); void Unlock(const std::vector& keys); private: - LockMgr* const lock_mgr_; + std::shared_ptr const lock_mgr_; MultiRecordLock(const MultiRecordLock&); void operator=(const MultiRecordLock&); }; diff --git a/src/pstd/src/rsync.cc b/src/pstd/src/rsync.cc index 083770f0a..f73647573 100644 --- a/src/pstd/src/rsync.cc +++ b/src/pstd/src/rsync.cc @@ -117,7 +117,7 @@ int StopRsync(const std::string& raw_path) { return 0; } - std::string rsync_stop_cmd = "kill -- -$(ps -o pgid= " + std::to_string(pid) + ")"; + std::string rsync_stop_cmd = "kill $(ps -o pgid=" + std::to_string(pid) + ")"; int ret = system(rsync_stop_cmd.c_str()); if (ret == 0 || (WIFEXITED(ret) && !WEXITSTATUS(ret))) { LOG(INFO) << "Stop rsync success!"; diff --git a/src/pstd/src/scope_record_lock.cc b/src/pstd/src/scope_record_lock.cc index 1c7d46f55..5f4a84f91 100644 --- a/src/pstd/src/scope_record_lock.cc +++ b/src/pstd/src/scope_record_lock.cc @@ -9,7 +9,7 @@ namespace pstd { namespace lock { -MultiScopeRecordLock::MultiScopeRecordLock(LockMgr* lock_mgr, const std::vector& keys) +MultiScopeRecordLock::MultiScopeRecordLock(std::shared_ptr lock_mgr, const std::vector& keys) : lock_mgr_(lock_mgr), keys_(keys) { std::string pre_key; std::sort(keys_.begin(), keys_.end()); diff --git a/src/storage/include/storage/backupable.h b/src/storage/include/storage/backupable.h index 3ede6b3ba..3ada28b5a 100644 --- a/src/storage/include/storage/backupable.h +++ b/src/storage/include/storage/backupable.h @@ -41,7 +41,7 @@ struct BackupContent { class BackupEngine { public: ~BackupEngine(); - static Status Open(Storage* db, BackupEngine** backup_engine_ptr); + static Status Open(Storage* db, std::unique_ptr& backup_engine_ret); Status SetBackupContent(); @@ -54,7 +54,7 @@ class BackupEngine { private: BackupEngine() {} - std::map engines_; + std::map> engines_; std::map backup_content_; std::map backup_pthread_ts_; @@ -68,3 +68,4 @@ class BackupEngine { } // namespace storage #endif // SRC_BACKUPABLE_H_ + diff --git a/src/storage/src/backupable.cc b/src/storage/src/backupable.cc index 8e0e48e0a..fdbe89af6 100644 --- a/src/storage/src/backupable.cc +++ b/src/storage/src/backupable.cc @@ -14,11 +14,6 @@ BackupEngine::~BackupEngine() { // Wait all children threads StopBackup(); WaitBackupPthread(); - // Delete engines - for (auto& engine : engines_) { - delete engine.second; - } - engines_.clear(); } Status BackupEngine::NewCheckpoint(rocksdb::DB* rocksdb_db, const std::string& type) { @@ -27,13 +22,13 @@ Status BackupEngine::NewCheckpoint(rocksdb::DB* rocksdb_db, const std::string& t if (!s.ok()) { return s; } - engines_.insert(std::make_pair(type, checkpoint)); + engines_.insert(std::make_pair(type, std::unique_ptr(checkpoint))); return s; } -Status BackupEngine::Open(storage::Storage* storage, BackupEngine** backup_engine_ptr) { - *backup_engine_ptr = new BackupEngine(); - if (!*backup_engine_ptr) { +Status BackupEngine::Open(storage::Storage* storage, std::unique_ptr& backup_engine_ret) { + auto backup_engine_ptr = std::unique_ptr(new BackupEngine); + if (!backup_engine_ptr) { return Status::Corruption("New BackupEngine failed!"); } @@ -47,14 +42,15 @@ Status BackupEngine::Open(storage::Storage* storage, BackupEngine** backup_engin } if (s.ok()) { - s = (*backup_engine_ptr)->NewCheckpoint(rocksdb_db, type); + s = backup_engine_ptr->NewCheckpoint(rocksdb_db, type); } if (!s.ok()) { - delete *backup_engine_ptr; + backup_engine_ptr = nullptr; break; } } + backup_engine_ret = std::move(backup_engine_ptr); return s; } @@ -74,8 +70,8 @@ Status BackupEngine::SetBackupContent() { } Status BackupEngine::CreateNewBackupSpecify(const std::string& backup_dir, const std::string& type) { - std::map::iterator it_engine = engines_.find(type); - std::map::iterator it_content = backup_content_.find(type); + auto it_engine = engines_.find(type); + auto it_content = backup_content_.find(type); std::string dir = GetSaveDirByType(backup_dir, type); delete_dir(dir.c_str()); @@ -150,3 +146,4 @@ void BackupEngine::StopBackup() { } } // namespace storage + From f8e637ee2ec453b80cb99237be1f91ad2f4cef6a Mon Sep 17 00:00:00 2001 From: cjh <1271435567@qq.com> Date: Sun, 14 May 2023 12:22:45 +0800 Subject: [PATCH 08/17] removed an unnecessary shared_ptr and use unique_ptr instead. --- src/net/examples/redis_cli_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/net/examples/redis_cli_test.cc b/src/net/examples/redis_cli_test.cc index 6a3f4c925..c2b40c33d 100644 --- a/src/net/examples/redis_cli_test.cc +++ b/src/net/examples/redis_cli_test.cc @@ -30,7 +30,7 @@ int main(int argc, char* argv[]) { ret = net::SerializeRedisCommand(vec, &str); printf(" 2. Serialize by vec return %d, (%s)\n", ret, str.c_str()); - std::shared_ptr rcli(NewRedisCli()); + std::unique_ptr rcli(NewRedisCli()); rcli->set_connect_timeout(3000); // redis v3.2+ protect mode will block other ip From 9be3a5e59b8e254bd50ff681facc7f810a686d6b Mon Sep 17 00:00:00 2001 From: laimuxi <1721261216@qq.com> Date: Sun, 14 May 2023 12:57:43 +0800 Subject: [PATCH 09/17] change pstd/ --- src/pstd/include/base_conf.h | 4 ++-- src/pstd/include/lock_mgr.h | 6 +++--- src/pstd/src/base_conf.cc | 17 +++++++---------- src/pstd/src/lock_mgr.cc | 20 ++++++++------------ src/pstd/src/rsync.cc | 6 ++---- src/pstd/tests/base_conf_test.cc | 4 ++-- 6 files changed, 24 insertions(+), 33 deletions(-) diff --git a/src/pstd/include/base_conf.h b/src/pstd/include/base_conf.h index ef51d36db..1a74b98c6 100644 --- a/src/pstd/include/base_conf.h +++ b/src/pstd/include/base_conf.h @@ -9,9 +9,9 @@ #include #include +#include #include #include - #include "pstd/include/pstd_define.h" namespace pstd { @@ -73,7 +73,7 @@ class BaseConf { void PushConfItem(const Rep::ConfItem& item); private: - Rep* rep_ = nullptr; + std::unique_ptr rep_; /* * No copy && no assign operator diff --git a/src/pstd/include/lock_mgr.h b/src/pstd/include/lock_mgr.h index 97e0961c5..185bee6e1 100644 --- a/src/pstd/include/lock_mgr.h +++ b/src/pstd/include/lock_mgr.h @@ -43,11 +43,11 @@ class LockMgr { // Map to locked key info std::shared_ptr lock_map_; - Status Acquire(LockMapStripe* stripe, const std::string& key); + Status Acquire(std::shared_ptr stripe, const std::string& key); - Status AcquireLocked(LockMapStripe* stripe, const std::string& key); + Status AcquireLocked(std::shared_ptr stripe, const std::string& key); - void UnLockKey(const std::string& key, LockMapStripe* stripe); + void UnLockKey(const std::string& key, std::shared_ptr stripe); // No copying allowed LockMgr(const LockMgr&); diff --git a/src/pstd/src/base_conf.cc b/src/pstd/src/base_conf.cc index ce2017a2d..472b0d053 100644 --- a/src/pstd/src/base_conf.cc +++ b/src/pstd/src/base_conf.cc @@ -21,7 +21,7 @@ static const int kConfItemLen = 1024 * 1024; BaseConf::BaseConf(const std::string& path) : rep_(new Rep(path)) {} -BaseConf::~BaseConf() { delete rep_; } +BaseConf::~BaseConf() {} int BaseConf::LoadConf() { if (!FileExists(rep_->path)) { @@ -29,7 +29,7 @@ int BaseConf::LoadConf() { } SequentialFile* sequential_file; NewSequentialFile(rep_->path, &sequential_file); - + auto sequential_file_ptr = std::unique_ptr(sequential_file); // read conf items char line[kConfItemLen]; @@ -78,19 +78,16 @@ int BaseConf::LoadConf() { } // sequential_file->Close(); - delete sequential_file; return 0; } int BaseConf::ReloadConf() { - Rep* rep = rep_; - rep_ = new Rep(rep->path); + auto rep = std::move(rep_); + rep_ = std::make_unique(rep->path); if (LoadConf() == -1) { - delete rep_; - rep_ = rep; + rep_ = std::move(rep); return -1; } - delete rep; return 0; } @@ -333,6 +330,7 @@ bool BaseConf::WriteBack() { if (!write_file) { return false; } + auto write_file_ptr = std::unique_ptr(write_file); std::string tmp; for (size_t i = 0; i < rep_->item.size(); i++) { if (rep_->item[i].type == Rep::kConf) { @@ -344,7 +342,6 @@ bool BaseConf::WriteBack() { } DeleteFile(rep_->path); RenameFile(tmp_path, rep_->path); - delete write_file; return true; } @@ -352,6 +349,7 @@ void BaseConf::WriteSampleConf() const { WritableFile* write_file; std::string sample_path = rep_->path + ".sample"; Status ret = NewWritableFile(sample_path, &write_file); + auto write_file_ptr = std::unique_ptr(write_file); std::string tmp; for (size_t i = 0; i < rep_->item.size(); i++) { if (rep_->item[i].type == Rep::kConf) { @@ -361,7 +359,6 @@ void BaseConf::WriteSampleConf() const { write_file->Append(rep_->item[i].value); } } - delete write_file; return; } diff --git a/src/pstd/src/lock_mgr.cc b/src/pstd/src/lock_mgr.cc index 342acc664..d77a6cddb 100644 --- a/src/pstd/src/lock_mgr.cc +++ b/src/pstd/src/lock_mgr.cc @@ -44,16 +44,12 @@ struct LockMap { explicit LockMap(size_t num_stripes, std::shared_ptr factory) : num_stripes_(num_stripes) { lock_map_stripes_.reserve(num_stripes); for (size_t i = 0; i < num_stripes; i++) { - LockMapStripe* stripe = new LockMapStripe(factory); + auto stripe = std::shared_ptr(new LockMapStripe(factory)); lock_map_stripes_.push_back(stripe); } } - ~LockMap() { - for (auto stripe : lock_map_stripes_) { - delete stripe; - } - } + ~LockMap() {} // Number of sepearate LockMapStripes to create, each with their own Mutex const size_t num_stripes_; @@ -62,7 +58,7 @@ struct LockMap { // (Only maintained if LockMgr::max_num_locks_ is positive.) std::atomic lock_cnt{0}; - std::vector lock_map_stripes_; + std::vector> lock_map_stripes_; size_t GetStripe(const std::string& key) const; }; @@ -87,14 +83,14 @@ Status LockMgr::TryLock(const std::string& key) { #else size_t stripe_num = lock_map_->GetStripe(key); assert(lock_map_->lock_map_stripes_.size() > stripe_num); - LockMapStripe* stripe = lock_map_->lock_map_stripes_.at(stripe_num); + auto stripe = lock_map_->lock_map_stripes_.at(stripe_num); return Acquire(stripe, key); #endif } // Helper function for TryLock(). -Status LockMgr::Acquire(LockMapStripe* stripe, const std::string& key) { +Status LockMgr::Acquire(std::shared_ptr stripe, const std::string& key) { Status result; // we wait indefinitely to acquire the lock @@ -125,7 +121,7 @@ Status LockMgr::Acquire(LockMapStripe* stripe, const std::string& key) { // Try to lock this key after we have acquired the mutex. // REQUIRED: Stripe mutex must be held. -Status LockMgr::AcquireLocked(LockMapStripe* stripe, const std::string& key) { +Status LockMgr::AcquireLocked(std::shared_ptr stripe, const std::string& key) { Status result; // Check if this key is already locked if (stripe->keys.find(key) != stripe->keys.end()) { @@ -149,7 +145,7 @@ Status LockMgr::AcquireLocked(LockMapStripe* stripe, const std::string& key) { return result; } -void LockMgr::UnLockKey(const std::string& key, LockMapStripe* stripe) { +void LockMgr::UnLockKey(const std::string& key, std::shared_ptr stripe) { #ifdef LOCKLESS #else auto stripe_iter = stripe->keys.find(key); @@ -171,7 +167,7 @@ void LockMgr::UnLock(const std::string& key) { // Lock the mutex for the stripe that this key hashes to size_t stripe_num = lock_map_->GetStripe(key); assert(lock_map_->lock_map_stripes_.size() > stripe_num); - LockMapStripe* stripe = lock_map_->lock_map_stripes_.at(stripe_num); + auto stripe = lock_map_->lock_map_stripes_.at(stripe_num); stripe->stripe_mutex->Lock(); UnLockKey(key, stripe); diff --git a/src/pstd/src/rsync.cc b/src/pstd/src/rsync.cc index f73647573..ebbd80e64 100644 --- a/src/pstd/src/rsync.cc +++ b/src/pstd/src/rsync.cc @@ -1,7 +1,7 @@ #include #include #include - +#include #include #include "pstd/include/env.h" @@ -100,16 +100,14 @@ int StopRsync(const std::string& raw_path) { LOG(WARNING) << "no rsync pid file found"; return 0; }; + auto sequential_file_ptr = std::unique_ptr(sequential_file); char line[32]; if (sequential_file->ReadLine(line, 32) == nullptr) { LOG(WARNING) << "read rsync pid file err"; - delete sequential_file; return 0; }; - delete sequential_file; - pid_t pid = atoi(line); if (pid <= 1) { diff --git a/src/pstd/tests/base_conf_test.cc b/src/pstd/tests/base_conf_test.cc index 4ee431058..dc97d14a5 100644 --- a/src/pstd/tests/base_conf_test.cc +++ b/src/pstd/tests/base_conf_test.cc @@ -34,10 +34,10 @@ class BaseConfTest : public ::testing::Test { WritableFile* write_file; Status ret = NewWritableFile(test_conf_, &write_file); if (!ret.ok()) return ret; + auto write_file_ptr = std::unique_ptr(write_file); for (std::string& item : sample_conf) { write_file->Append(item); } - delete write_file; return Status::OK(); } @@ -51,7 +51,7 @@ class BaseConfTest : public ::testing::Test { TEST_F(BaseConfTest, WriteReadConf) { ASSERT_OK(CreateSampleConf()); - BaseConf* conf = new BaseConf(test_conf_); + auto conf = std::make_unique(test_conf_); ASSERT_EQ(conf->LoadConf(), 0); // Write configuration From 4c75a1a30a5ecde53709c41be6f313a717af1a3a Mon Sep 17 00:00:00 2001 From: cjh <1271435567@qq.com> Date: Sun, 14 May 2023 13:30:50 +0800 Subject: [PATCH 10/17] revised a comment --- src/net/examples/myredis_cli.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/net/examples/myredis_cli.cc b/src/net/examples/myredis_cli.cc index 2fb053d07..64897177b 100644 --- a/src/net/examples/myredis_cli.cc +++ b/src/net/examples/myredis_cli.cc @@ -96,7 +96,7 @@ int main(int argc, char* argv[]) { SignalSetup(); std::unique_ptr conn_factory = std::make_unique(); - //"handle" will be deleted within "client->StopThread()" + //the object "client" is responsible for deleting "handle" ClientHandle* handle = new ClientHandle(); client = std::make_unique(conn_factory.get(), 3000, 60, handle, nullptr); From d3c227c5b44fce3dd322f3a5df6715e00b61e5b4 Mon Sep 17 00:00:00 2001 From: laimuxi <1721261216@qq.com> Date: Sun, 14 May 2023 15:04:32 +0800 Subject: [PATCH 11/17] change pstd implement --- include/pika_binlog.h | 6 +++--- include/pika_binlog_reader.h | 2 +- include/pika_consensus.h | 2 +- src/pika_binlog.cc | 34 ++++++++++++++------------------ src/pika_binlog_reader.cc | 13 ++++++------ src/pika_consensus.cc | 6 +++--- src/pika_meta.cc | 20 ++++++------------- src/pstd/include/env.h | 12 +++++------ src/pstd/src/base_conf.cc | 15 ++++++-------- src/pstd/src/env.cc | 25 ++++++++++------------- src/pstd/src/rsync.cc | 5 ++--- src/pstd/tests/base_conf_test.cc | 5 ++--- 12 files changed, 61 insertions(+), 84 deletions(-) diff --git a/include/pika_binlog.h b/include/pika_binlog.h index a7b1b5a99..fecd782a5 100644 --- a/include/pika_binlog.h +++ b/include/pika_binlog.h @@ -102,9 +102,9 @@ class Binlog { std::atomic opened_; - Version* version_ = nullptr; - pstd::WritableFile* queue_ = nullptr; - pstd::RWFile* versionfile_ = nullptr; + std::unique_ptr version_ = nullptr; + std::unique_ptr queue_; + std::unique_ptr versionfile_; pstd::Mutex mutex_; diff --git a/include/pika_binlog_reader.h b/include/pika_binlog_reader.h index 6640f5bc0..fc3fcff3e 100644 --- a/include/pika_binlog_reader.h +++ b/include/pika_binlog_reader.h @@ -41,7 +41,7 @@ class PikaBinlogReader { uint64_t last_record_offset_ = 0; std::shared_ptr logger_; - pstd::SequentialFile* queue_ = nullptr; + std::unique_ptr queue_ = nullptr; char* const backing_store_; Slice buffer_; diff --git a/include/pika_consensus.h b/include/pika_consensus.h index b71b14264..3eea52f8e 100644 --- a/include/pika_consensus.h +++ b/include/pika_consensus.h @@ -38,7 +38,7 @@ class Context { private: std::string path_; - pstd::RWFile* save_ = nullptr; + std::unique_ptr save_; // No copying allowed; Context(const Context&); void operator=(const Context&); diff --git a/src/pika_binlog.cc b/src/pika_binlog.cc index d8a84c5e0..ff77d26c1 100644 --- a/src/pika_binlog.cc +++ b/src/pika_binlog.cc @@ -81,24 +81,24 @@ Binlog::Binlog(const std::string& binlog_path, const int file_size) LOG(INFO) << "Binlog: Manifest file not exist, we create a new one."; profile = NewFileName(filename_, pro_num_); - s = pstd::NewWritableFile(profile, &queue_); + s = pstd::NewWritableFile(profile, queue_); if (!s.ok()) { LOG(FATAL) << "Binlog: new " << filename_ << " " << s.ToString(); } - s = pstd::NewRWFile(manifest, &versionfile_); + s = pstd::NewRWFile(manifest, versionfile_); if (!s.ok()) { LOG(FATAL) << "Binlog: new versionfile error " << s.ToString(); } - version_ = new Version(versionfile_); + version_ = std::unique_ptr(new Version(versionfile_.get())); version_->StableSave(); } else { LOG(INFO) << "Binlog: Find the exist file."; - s = pstd::NewRWFile(manifest, &versionfile_); + s = pstd::NewRWFile(manifest, versionfile_); if (s.ok()) { - version_ = new Version(versionfile_); + version_ = std::unique_ptr(new Version(versionfile_.get())); version_->Init(); pro_num_ = version_->pro_num_; @@ -110,7 +110,7 @@ Binlog::Binlog(const std::string& binlog_path, const int file_size) profile = NewFileName(filename_, pro_num_); DLOG(INFO) << "Binlog: open profile " << profile; - s = pstd::AppendWritableFile(profile, &queue_, version_->pro_offset_); + s = pstd::AppendWritableFile(profile, queue_, version_->pro_offset_); if (!s.ok()) { LOG(FATAL) << "Binlog: Open file " << profile << " error " << s.ToString(); } @@ -125,10 +125,6 @@ Binlog::Binlog(const std::string& binlog_path, const int file_size) Binlog::~Binlog() { std::lock_guard l(mutex_); Close(); - delete version_; - delete versionfile_; - - delete queue_; } void Binlog::Close() { @@ -185,15 +181,15 @@ Status Binlog::Put(const char* item, int len) { /* Check to roll log file */ uint64_t filesize = queue_->Filesize(); if (filesize > file_size_) { - pstd::WritableFile* queue = nullptr; + std::unique_ptr queue; std::string profile = NewFileName(filename_, pro_num_ + 1); - s = pstd::NewWritableFile(profile, &queue); + s = pstd::NewWritableFile(profile, queue); if (!s.ok()) { LOG(ERROR) << "Binlog: new " << filename_ << " " << s.ToString(); return s; } - delete queue_; - queue_ = queue; + queue_.reset(); + queue_ = std::move(queue); pro_num_++; { @@ -352,7 +348,7 @@ Status Binlog::SetProducerStatus(uint32_t pro_num, uint64_t pro_offset, uint32_t pro_offset = 0; } - delete queue_; + queue_.reset(); std::string init_profile = NewFileName(filename_, 0); if (pstd::FileExists(init_profile)) { @@ -364,8 +360,8 @@ Status Binlog::SetProducerStatus(uint32_t pro_num, uint64_t pro_offset, uint32_t pstd::DeleteFile(profile); } - pstd::NewWritableFile(profile, &queue_); - Binlog::AppendPadding(queue_, &pro_offset); + pstd::NewWritableFile(profile, queue_); + Binlog::AppendPadding(queue_.get(), &pro_offset); pro_num_ = pro_num; @@ -383,7 +379,7 @@ Status Binlog::SetProducerStatus(uint32_t pro_num, uint64_t pro_offset, uint32_t } Status Binlog::Truncate(uint32_t pro_num, uint64_t pro_offset, uint64_t index) { - delete queue_; + queue_.reset(); std::string profile = NewFileName(filename_, pro_num); const int fd = open(profile.c_str(), O_RDWR | O_CLOEXEC, 0644); if (fd < 0) { @@ -403,7 +399,7 @@ Status Binlog::Truncate(uint32_t pro_num, uint64_t pro_offset, uint64_t index) { version_->StableSave(); } - Status s = pstd::AppendWritableFile(profile, &queue_, version_->pro_offset_); + Status s = pstd::AppendWritableFile(profile, queue_, version_->pro_offset_); if (!s.ok()) { return s; } diff --git a/src/pika_binlog_reader.cc b/src/pika_binlog_reader.cc index f1503298e..5aea194cb 100644 --- a/src/pika_binlog_reader.cc +++ b/src/pika_binlog_reader.cc @@ -29,7 +29,6 @@ PikaBinlogReader::PikaBinlogReader() PikaBinlogReader::~PikaBinlogReader() { delete[] backing_store_; - delete queue_; } void PikaBinlogReader::GetReaderStatus(uint32_t* cur_filenum, uint64_t* cur_offset) { @@ -52,15 +51,15 @@ int PikaBinlogReader::Seek(std::shared_ptr logger, uint32_t filenum, uin LOG(WARNING) << confile << " not exits"; return -1; } - pstd::SequentialFile* readfile; - if (!pstd::NewSequentialFile(confile, &readfile).ok()) { + std::unique_ptr readfile; + if (!pstd::NewSequentialFile(confile, readfile).ok()) { LOG(WARNING) << "New swquential " << confile << " failed"; return -1; } if (queue_) { - delete queue_; + queue_.reset(); } - queue_ = readfile; + queue_ = std::move(readfile); logger_ = logger; std::lock_guard l(rwlock_); @@ -251,10 +250,10 @@ Status PikaBinlogReader::Get(std::string* scratch, uint32_t* filenum, uint64_t* // Roll to next file need retry; if (pstd::FileExists(confile)) { DLOG(INFO) << "BinlogSender roll to new binlog" << confile; - delete queue_; + queue_.reset(); queue_ = nullptr; - pstd::NewSequentialFile(confile, &(queue_)); + pstd::NewSequentialFile(confile, queue_); { std::lock_guard l(rwlock_); cur_filenum_++; diff --git a/src/pika_consensus.cc b/src/pika_consensus.cc index 0c09759fa..f2e88fc09 100644 --- a/src/pika_consensus.cc +++ b/src/pika_consensus.cc @@ -20,7 +20,7 @@ extern PikaCmdTableManager* g_pika_cmd_table_manager; Context::Context(const std::string path) : applied_index_(), path_(path), save_(nullptr) {} -Context::~Context() { delete save_; } +Context::~Context() {} Status Context::StableSave() { char* p = save_->GetData(); @@ -36,13 +36,13 @@ Status Context::StableSave() { Status Context::Init() { if (!pstd::FileExists(path_)) { - Status s = pstd::NewRWFile(path_, &save_); + Status s = pstd::NewRWFile(path_, save_); if (!s.ok()) { LOG(FATAL) << "Context new file failed " << s.ToString(); } StableSave(); } else { - Status s = pstd::NewRWFile(path_, &save_); + Status s = pstd::NewRWFile(path_, save_); if (!s.ok()) { LOG(FATAL) << "Context new file failed " << s.ToString(); } diff --git a/src/pika_meta.cc b/src/pika_meta.cc index 679215cc2..bba6e18c8 100644 --- a/src/pika_meta.cc +++ b/src/pika_meta.cc @@ -29,11 +29,10 @@ Status PikaMeta::StableSave(const std::vector& table_structs) { std::string tmp_file = local_meta_file; tmp_file.append("_tmp"); - pstd::RWFile* saver = nullptr; + std::unique_ptr saver; pstd::CreatePath(local_meta_path_); - Status s = pstd::NewRWFile(tmp_file, &saver); + Status s = pstd::NewRWFile(tmp_file, saver); if (!s.ok()) { - delete saver; LOG(WARNING) << "Open local meta file failed"; return Status::Corruption("open local meta file failed"); } @@ -50,7 +49,6 @@ Status PikaMeta::StableSave(const std::vector& table_structs) { std::string meta_str; if (!meta.SerializeToString(&meta_str)) { - delete saver; LOG(WARNING) << "Serialize meta string failed"; return Status::Corruption("serialize meta string failed"); } @@ -62,7 +60,6 @@ Status PikaMeta::StableSave(const std::vector& table_structs) { memcpy(p, &meta_str_size, sizeof(uint32_t)); p += sizeof(uint32_t); memcpy(p, meta_str.data(), meta_str.size()); - delete saver; pstd::DeleteFile(local_meta_file); if (pstd::RenameFile(tmp_file, local_meta_file)) { @@ -80,16 +77,14 @@ Status PikaMeta::ParseMeta(std::vector* const table_structs) { return Status::Corruption("meta file not found"); } - pstd::RWFile* reader = nullptr; - Status s = pstd::NewRWFile(local_meta_file, &reader); + std::unique_ptr reader; + Status s = pstd::NewRWFile(local_meta_file, reader); if (!s.ok()) { - delete reader; LOG(WARNING) << "Open local meta file failed"; return Status::Corruption("open local meta file failed"); } if (reader->GetData() == nullptr) { - delete reader; LOG(WARNING) << "Meta file init error"; return Status::Corruption("meta file init error"); } @@ -98,18 +93,15 @@ Status PikaMeta::ParseMeta(std::vector* const table_structs) { uint32_t meta_size = 0; memcpy((char*)(&version), reader->GetData(), sizeof(uint32_t)); memcpy((char*)(&meta_size), reader->GetData() + sizeof(uint32_t), sizeof(uint32_t)); - char* const buf = new char[meta_size]; + auto const buf_ptr = std::make_unique(meta_size); + char* const buf = buf_ptr.get(); memcpy(buf, reader->GetData() + 2 * sizeof(uint32_t), meta_size); InnerMessage::PikaMeta meta; if (!meta.ParseFromArray(buf, meta_size)) { - delete[] buf; - delete reader; LOG(WARNING) << "Parse meta string failed"; return Status::Corruption("parse meta string failed"); } - delete[] buf; - delete reader; table_structs->clear(); for (int idx = 0; idx < meta.table_infos_size(); ++idx) { diff --git a/src/pstd/include/env.h b/src/pstd/include/env.h index da0e4d29e..18decd15c 100644 --- a/src/pstd/include/env.h +++ b/src/pstd/include/env.h @@ -4,7 +4,7 @@ #include #include #include - +#include #include "pstd/include/pstd_status.h" namespace pstd { @@ -69,17 +69,17 @@ bool GetDescendant(const std::string& dir, std::vector& result); uint64_t NowMicros(); void SleepForMicroseconds(int micros); -Status NewSequentialFile(const std::string& fname, SequentialFile** result); +Status NewSequentialFile(const std::string& fname, std::unique_ptr& result); -Status NewWritableFile(const std::string& fname, WritableFile** result); +Status NewWritableFile(const std::string& fname, std::unique_ptr& result); -Status NewRWFile(const std::string& fname, RWFile** result); +Status NewRWFile(const std::string& fname, std::unique_ptr& result); Status AppendSequentialFile(const std::string& fname, SequentialFile** result); -Status AppendWritableFile(const std::string& fname, WritableFile** result, uint64_t write_len = 0); +Status AppendWritableFile(const std::string& fname, std::unique_ptr& result, uint64_t write_len = 0); -Status NewRandomRWFile(const std::string& fname, RandomRWFile** result); +Status NewRandomRWFile(const std::string& fname, std::unique_ptr& result); // A file abstraction for sequential writing. The implementation // must provide buffering since callers may append small fragments diff --git a/src/pstd/src/base_conf.cc b/src/pstd/src/base_conf.cc index 472b0d053..6627471b3 100644 --- a/src/pstd/src/base_conf.cc +++ b/src/pstd/src/base_conf.cc @@ -27,9 +27,8 @@ int BaseConf::LoadConf() { if (!FileExists(rep_->path)) { return -1; } - SequentialFile* sequential_file; - NewSequentialFile(rep_->path, &sequential_file); - auto sequential_file_ptr = std::unique_ptr(sequential_file); + std::unique_ptr sequential_file; + NewSequentialFile(rep_->path, sequential_file); // read conf items char line[kConfItemLen]; @@ -323,14 +322,13 @@ void BaseConf::DumpConf() const { } bool BaseConf::WriteBack() { - WritableFile* write_file; + std::unique_ptr write_file; std::string tmp_path = rep_->path + ".tmp"; - Status ret = NewWritableFile(tmp_path, &write_file); + Status ret = NewWritableFile(tmp_path, write_file); LOG(INFO) << "ret " << ret.ToString(); if (!write_file) { return false; } - auto write_file_ptr = std::unique_ptr(write_file); std::string tmp; for (size_t i = 0; i < rep_->item.size(); i++) { if (rep_->item[i].type == Rep::kConf) { @@ -346,10 +344,9 @@ bool BaseConf::WriteBack() { } void BaseConf::WriteSampleConf() const { - WritableFile* write_file; + std::unique_ptr write_file; std::string sample_path = rep_->path + ".sample"; - Status ret = NewWritableFile(sample_path, &write_file); - auto write_file_ptr = std::unique_ptr(write_file); + Status ret = NewWritableFile(sample_path, write_file); std::string tmp; for (size_t i = 0; i < rep_->item.size(); i++) { if (rep_->item[i].type == Rep::kConf) { diff --git a/src/pstd/src/env.cc b/src/pstd/src/env.cc index ae4205d05..686c9ff69 100644 --- a/src/pstd/src/env.cc +++ b/src/pstd/src/env.cc @@ -720,61 +720,56 @@ class PosixRandomRWFile : public RandomRWFile { // } }; -Status NewSequentialFile(const std::string& fname, SequentialFile** result) { +Status NewSequentialFile(const std::string& fname, std::unique_ptr& result) { FILE* f = fopen(fname.c_str(), "r"); if (f == nullptr) { - *result = nullptr; return IOError(fname, errno); } else { - *result = new PosixSequentialFile(fname, f); + result = std::unique_ptr(new PosixSequentialFile(fname, f)); return Status::OK(); } } -Status NewWritableFile(const std::string& fname, WritableFile** result) { +Status NewWritableFile(const std::string& fname, std::unique_ptr& result) { Status s; const int fd = open(fname.c_str(), O_CREAT | O_RDWR | O_TRUNC | O_CLOEXEC, 0644); if (fd < 0) { - *result = nullptr; s = IOError(fname, errno); } else { - *result = new PosixMmapFile(fname, fd, kPageSize); + result = std::unique_ptr(new PosixMmapFile(fname, fd, kPageSize)); } return s; } -Status NewRWFile(const std::string& fname, RWFile** result) { +Status NewRWFile(const std::string& fname, std::unique_ptr& result) { Status s; const int fd = open(fname.c_str(), O_CREAT | O_RDWR | O_CLOEXEC, 0644); if (fd < 0) { - *result = nullptr; s = IOError(fname, errno); } else { - *result = new MmapRWFile(fname, fd, kPageSize); + result = std::unique_ptr(new MmapRWFile(fname, fd, kPageSize)); } return s; } -Status AppendWritableFile(const std::string& fname, WritableFile** result, uint64_t write_len) { +Status AppendWritableFile(const std::string& fname, std::unique_ptr& result, uint64_t write_len) { Status s; const int fd = open(fname.c_str(), O_RDWR | O_CLOEXEC, 0644); if (fd < 0) { - *result = nullptr; s = IOError(fname, errno); } else { - *result = new PosixMmapFile(fname, fd, kPageSize, write_len); + result = std::unique_ptr(new PosixMmapFile(fname, fd, kPageSize, write_len)); } return s; } -Status NewRandomRWFile(const std::string& fname, RandomRWFile** result) { +Status NewRandomRWFile(const std::string& fname, std::unique_ptr& result) { Status s; const int fd = open(fname.c_str(), O_CREAT | O_RDWR, 0644); if (fd < 0) { - *result = nullptr; s = IOError(fname, errno); } else { - *result = new PosixRandomRWFile(fname, fd); + result = std::unique_ptr(new PosixRandomRWFile(fname, fd)); } return s; } diff --git a/src/pstd/src/rsync.cc b/src/pstd/src/rsync.cc index ebbd80e64..1a40644ae 100644 --- a/src/pstd/src/rsync.cc +++ b/src/pstd/src/rsync.cc @@ -95,12 +95,11 @@ int StopRsync(const std::string& raw_path) { } // Kill Rsync - SequentialFile* sequential_file; - if (!NewSequentialFile(pid_file, &sequential_file).ok()) { + std::unique_ptr sequential_file; + if (!NewSequentialFile(pid_file, sequential_file).ok()) { LOG(WARNING) << "no rsync pid file found"; return 0; }; - auto sequential_file_ptr = std::unique_ptr(sequential_file); char line[32]; if (sequential_file->ReadLine(line, 32) == nullptr) { diff --git a/src/pstd/tests/base_conf_test.cc b/src/pstd/tests/base_conf_test.cc index dc97d14a5..22b787748 100644 --- a/src/pstd/tests/base_conf_test.cc +++ b/src/pstd/tests/base_conf_test.cc @@ -31,10 +31,9 @@ class BaseConfTest : public ::testing::Test { "test_bool : yes\n", }; - WritableFile* write_file; - Status ret = NewWritableFile(test_conf_, &write_file); + std::unique_ptr write_file; + Status ret = NewWritableFile(test_conf_, write_file); if (!ret.ok()) return ret; - auto write_file_ptr = std::unique_ptr(write_file); for (std::string& item : sample_conf) { write_file->Append(item); } From 1b72139d233d4619c7f45bce2dafb720bdd61641 Mon Sep 17 00:00:00 2001 From: laimuxi <1721261216@qq.com> Date: Sun, 14 May 2023 19:24:53 +0800 Subject: [PATCH 12/17] remove code of jh --- include/pika_binlog.h | 2 +- include/pika_binlog_reader.h | 2 +- src/net/examples/bg_thread.cc | 8 ++++---- src/net/examples/binlog_parser_test.cc | 2 +- src/net/examples/http_server.cc | 7 +++++-- src/net/examples/https_server.cc | 7 +++++-- src/net/examples/mydispatch_srv.cc | 7 +++++-- src/net/examples/myholy_srv.cc | 7 +++++-- src/net/examples/myholy_srv_chandle.cc | 4 +++- src/net/examples/myproto_cli.cc | 4 ++-- src/net/examples/myredis_cli.cc | 12 ++++++------ src/net/examples/myredis_srv.cc | 8 +++++--- src/net/examples/performance/client.cc | 3 ++- src/net/examples/performance/server.cc | 4 +++- src/net/examples/redis_cli_test.cc | 2 +- src/net/examples/redis_parser_test.cc | 2 +- src/net/examples/simple_http_server.cc | 7 +++++-- src/net/examples/thread_pool_test.cc | 8 ++++---- src/net/include/net_cli.h | 4 ++-- src/net/include/server_thread.h | 2 +- src/net/src/dispatch_thread.cc | 21 ++++++++++++--------- src/net/src/dispatch_thread.h | 2 +- src/net/src/net_cli.cc | 17 ++++++++++------- src/net/src/server_thread.cc | 15 ++++++++++----- 24 files changed, 95 insertions(+), 62 deletions(-) diff --git a/include/pika_binlog.h b/include/pika_binlog.h index fecd782a5..c872bb159 100644 --- a/include/pika_binlog.h +++ b/include/pika_binlog.h @@ -102,7 +102,7 @@ class Binlog { std::atomic opened_; - std::unique_ptr version_ = nullptr; + std::unique_ptr version_; std::unique_ptr queue_; std::unique_ptr versionfile_; diff --git a/include/pika_binlog_reader.h b/include/pika_binlog_reader.h index fc3fcff3e..e75e5dfe8 100644 --- a/include/pika_binlog_reader.h +++ b/include/pika_binlog_reader.h @@ -41,7 +41,7 @@ class PikaBinlogReader { uint64_t last_record_offset_ = 0; std::shared_ptr logger_; - std::unique_ptr queue_ = nullptr; + std::unique_ptr queue_; char* const backing_store_; Slice buffer_; diff --git a/src/net/examples/bg_thread.cc b/src/net/examples/bg_thread.cc index b29cd95e1..14e3ce3fd 100644 --- a/src/net/examples/bg_thread.cc +++ b/src/net/examples/bg_thread.cc @@ -15,7 +15,7 @@ static pstd::Mutex print_lock; void task(void* arg) { { - pstd::MutexLock l(&print_lock); + std::lock_guard l(print_lock); std::cout << " task : " << *((int*)arg) << std::endl; } sleep(1); @@ -42,7 +42,7 @@ int main() { int* pi = new int(i); t.Schedule(task, (void*)pi); t.QueueSize(&pqsize, &qsize); - pstd::MutexLock l(&print_lock); + std::lock_guard l(print_lock); std::cout << " current queue size:" << qsize << ", " << pqsize << std::endl; } std::cout << std::endl << std::endl; @@ -58,7 +58,7 @@ int main() { int* pi = new int(i); t2.Schedule(task, (void*)pi); t2.QueueSize(&pqsize, &qsize); - pstd::MutexLock l(&print_lock); + std::lock_guard l(print_lock); std::cout << " current queue size:" << qsize << ", " << pqsize << std::endl; } std::cout << std::endl << std::endl; @@ -90,7 +90,7 @@ int main() { int* pi = new int(i); t.DelaySchedule(i * 1000, task, (void*)pi); t.QueueSize(&pqsize, &qsize); - pstd::MutexLock l(&print_lock); + std::lock_guard l(print_lock); std::cout << " current queue size:" << qsize << ", " << pqsize << std::endl; } sleep(3); diff --git a/src/net/examples/binlog_parser_test.cc b/src/net/examples/binlog_parser_test.cc index 9077db4c1..83594eb9f 100644 --- a/src/net/examples/binlog_parser_test.cc +++ b/src/net/examples/binlog_parser_test.cc @@ -16,7 +16,7 @@ int main(int argc, char* argv[]) { std::string ip(argv[1]); int port = atoi(argv[2]); - std::unique_ptr rcli(NewRedisCli()); + NetCli* rcli = NewRedisCli(); rcli->set_connect_timeout(3000); Status s = rcli->Connect(ip, port, "127.0.0.1"); diff --git a/src/net/examples/http_server.cc b/src/net/examples/http_server.cc index 634083a43..04d989b96 100644 --- a/src/net/examples/http_server.cc +++ b/src/net/examples/http_server.cc @@ -96,8 +96,8 @@ int main(int argc, char* argv[]) { SignalSetup(); - std::unique_ptr my_conn_factory = std::make_unique(); - std::unique_ptr st(NewDispatchThread(port, 4, my_conn_factory.get(), 1000)); + ConnFactory* my_conn_factory = new MyConnFactory(); + ServerThread* st = NewDispatchThread(port, 4, my_conn_factory, 1000); if (st->StartThread() != 0) { printf("StartThread error happened!\n"); @@ -109,5 +109,8 @@ int main(int argc, char* argv[]) { } st->StopThread(); + delete st; + delete my_conn_factory; + return 0; } diff --git a/src/net/examples/https_server.cc b/src/net/examples/https_server.cc index 7b7243a82..0a592cea0 100644 --- a/src/net/examples/https_server.cc +++ b/src/net/examples/https_server.cc @@ -97,8 +97,8 @@ int main(int argc, char* argv[]) { SignalSetup(); - std::unique_ptr my_conn_factory = std::make_unique(); - std::unique_ptr st(NewDispatchThread(port, 4, my_conn_factory.get(), 1000)); + ConnFactory* my_conn_factory = new MyConnFactory(); + ServerThread* st = NewDispatchThread(port, 4, my_conn_factory, 1000); #if __ENABLE_SSL if (st->EnableSecurity("/complete_path_to/host.crt", "/complete_path_to/host.key") != 0) { @@ -117,5 +117,8 @@ int main(int argc, char* argv[]) { } st->StopThread(); + delete st; + delete my_conn_factory; + return 0; } diff --git a/src/net/examples/mydispatch_srv.cc b/src/net/examples/mydispatch_srv.cc index 23fc591b9..1618c4718 100644 --- a/src/net/examples/mydispatch_srv.cc +++ b/src/net/examples/mydispatch_srv.cc @@ -75,8 +75,8 @@ static void SignalSetup() { int main() { SignalSetup(); - std::unique_ptr my_conn_factory = std::make_unique(); - std::unique_ptr st(NewDispatchThread(9211, 10, my_conn_factory.get(), 1000)); + ConnFactory* my_conn_factory = new MyConnFactory(); + ServerThread* st = NewDispatchThread(9211, 10, my_conn_factory, 1000); if (st->StartThread() != 0) { printf("StartThread error happened!\n"); @@ -88,5 +88,8 @@ int main() { } st->StopThread(); + delete st; + delete my_conn_factory; + return 0; } diff --git a/src/net/examples/myholy_srv.cc b/src/net/examples/myholy_srv.cc index 27607e9a4..e82a593f5 100644 --- a/src/net/examples/myholy_srv.cc +++ b/src/net/examples/myholy_srv.cc @@ -80,9 +80,9 @@ int main(int argc, char* argv[]) { SignalSetup(); - std::unique_ptr conn_factory = std::make_unique(); + ConnFactory* conn_factory = new MyConnFactory(); - std::unique_ptr my_thread(NewHolyThread(my_port, conn_factory.get())); + ServerThread* my_thread = NewHolyThread(my_port, conn_factory); if (my_thread->StartThread() != 0) { printf("StartThread error happened!\n"); exit(-1); @@ -93,5 +93,8 @@ int main(int argc, char* argv[]) { } my_thread->StopThread(); + delete my_thread; + delete conn_factory; + return 0; } diff --git a/src/net/examples/myholy_srv_chandle.cc b/src/net/examples/myholy_srv_chandle.cc index a6f6b6cd9..4d64e2412 100644 --- a/src/net/examples/myholy_srv_chandle.cc +++ b/src/net/examples/myholy_srv_chandle.cc @@ -107,7 +107,7 @@ int main(int argc, char* argv[]) { MyConnFactory conn_factory; MyServerHandle handle; - std::unique_ptr my_thread(NewHolyThread(my_port, &conn_factory, 1000, &handle)); + ServerThread* my_thread = NewHolyThread(my_port, &conn_factory, 1000, &handle); if (my_thread->StartThread() != 0) { printf("StartThread error happened!\n"); exit(-1); @@ -118,5 +118,7 @@ int main(int argc, char* argv[]) { } my_thread->StopThread(); + delete my_thread; + return 0; } diff --git a/src/net/examples/myproto_cli.cc b/src/net/examples/myproto_cli.cc index 881b2b4f7..898a56eb9 100644 --- a/src/net/examples/myproto_cli.cc +++ b/src/net/examples/myproto_cli.cc @@ -2,7 +2,6 @@ #include #include #include -#include #include "myproto.pb.h" #include "net/include/net_cli.h" @@ -19,7 +18,7 @@ int main(int argc, char* argv[]) { std::string ip(argv[1]); int port = atoi(argv[2]); - std::unique_ptr cli(NewPbCli()); + NetCli* cli = NewPbCli(); Status s = cli->Connect(ip, port); if (!s.ok()) { @@ -49,5 +48,6 @@ int main(int argc, char* argv[]) { } cli->Close(); + delete cli; return 0; } diff --git a/src/net/examples/myredis_cli.cc b/src/net/examples/myredis_cli.cc index 64897177b..80ca33fda 100644 --- a/src/net/examples/myredis_cli.cc +++ b/src/net/examples/myredis_cli.cc @@ -29,7 +29,7 @@ MyConn::MyConn(int fd, const std::string& ip_port, Thread* thread, void* worker_ // Handle worker_specific_data ... } -std::unique_ptr client; +ClientThread* client; int sendto_port; int MyConn::DealMessage(const RedisCmdArgsType& argv, std::string* response) { sleep(1); @@ -95,11 +95,10 @@ int main(int argc, char* argv[]) { SignalSetup(); - std::unique_ptr conn_factory = std::make_unique(); - //the object "client" is responsible for deleting "handle" + ConnFactory* conn_factory = new MyConnFactory(); ClientHandle* handle = new ClientHandle(); - client = std::make_unique(conn_factory.get(), 3000, 60, handle, nullptr); + client = new ClientThread(conn_factory, 3000, 60, handle, nullptr); if (client->StartThread() != 0) { printf("StartThread error happened!\n"); @@ -108,10 +107,11 @@ int main(int argc, char* argv[]) { running.store(true); while (running.load()) { sleep(1); - DoCronWork(client.get(), sendto_port); + DoCronWork(client, sendto_port); } client->StopThread(); - client.reset(); + delete client; + delete conn_factory; return 0; } diff --git a/src/net/examples/myredis_srv.cc b/src/net/examples/myredis_srv.cc index 6672a412b..bedc6c9e0 100644 --- a/src/net/examples/myredis_srv.cc +++ b/src/net/examples/myredis_srv.cc @@ -96,10 +96,9 @@ int main(int argc, char* argv[]) { SignalSetup(); - std::unique_ptr conn_factory = std::make_unique(); + ConnFactory* conn_factory = new MyConnFactory(); - std::unique_ptr my_thread = - std::make_unique(my_port, conn_factory.get(), 1000, nullptr, false); + ServerThread* my_thread = new HolyThread(my_port, conn_factory, 1000, nullptr, false); if (my_thread->StartThread() != 0) { printf("StartThread error happened!\n"); exit(-1); @@ -110,5 +109,8 @@ int main(int argc, char* argv[]) { } my_thread->StopThread(); + delete my_thread; + delete conn_factory; + return 0; } diff --git a/src/net/examples/performance/client.cc b/src/net/examples/performance/client.cc index a408b0330..3f73577ef 100644 --- a/src/net/examples/performance/client.cc +++ b/src/net/examples/performance/client.cc @@ -18,7 +18,7 @@ int main(int argc, char* argv[]) { std::string ip(argv[1]); int port = atoi(argv[2]); - std::unique_ptr cli(NewPbCli()); + NetCli* cli = NewPbCli(); Status s = cli->Connect(ip, port); if (!s.ok()) { @@ -44,5 +44,6 @@ int main(int argc, char* argv[]) { } cli->Close(); + delete cli; return 0; } diff --git a/src/net/examples/performance/server.cc b/src/net/examples/performance/server.cc index 5b7b65cbc..245457bd1 100644 --- a/src/net/examples/performance/server.cc +++ b/src/net/examples/performance/server.cc @@ -84,7 +84,7 @@ int main(int argc, char* argv[]) { SignalSetup(); - std::unique_ptr st_thread(NewDispatchThread(ip, port, 24, &conn_factory, 1000)); + ServerThread* st_thread = NewDispatchThread(ip, port, 24, &conn_factory, 1000); st_thread->StartThread(); uint64_t st, ed; @@ -99,5 +99,7 @@ int main(int argc, char* argv[]) { } st_thread->StopThread(); + delete st_thread; + return 0; } diff --git a/src/net/examples/redis_cli_test.cc b/src/net/examples/redis_cli_test.cc index c2b40c33d..6decf3208 100644 --- a/src/net/examples/redis_cli_test.cc +++ b/src/net/examples/redis_cli_test.cc @@ -30,7 +30,7 @@ int main(int argc, char* argv[]) { ret = net::SerializeRedisCommand(vec, &str); printf(" 2. Serialize by vec return %d, (%s)\n", ret, str.c_str()); - std::unique_ptr rcli(NewRedisCli()); + NetCli* rcli = NewRedisCli(); rcli->set_connect_timeout(3000); // redis v3.2+ protect mode will block other ip diff --git a/src/net/examples/redis_parser_test.cc b/src/net/examples/redis_parser_test.cc index 90bee2869..0b3cd7252 100644 --- a/src/net/examples/redis_parser_test.cc +++ b/src/net/examples/redis_parser_test.cc @@ -15,7 +15,7 @@ int main(int argc, char* argv[]) { std::string ip(argv[1]); int port = atoi(argv[2]); - std::unique_ptr rcli(NewRedisCli()); + NetCli* rcli = NewRedisCli(); rcli->set_connect_timeout(3000); Status s = rcli->Connect(ip, port, "127.0.0.1"); diff --git a/src/net/examples/simple_http_server.cc b/src/net/examples/simple_http_server.cc index 73751c95e..a6cd597a6 100644 --- a/src/net/examples/simple_http_server.cc +++ b/src/net/examples/simple_http_server.cc @@ -76,8 +76,8 @@ int main(int argc, char* argv[]) { SignalSetup(); - std::unique_ptr my_conn_factory = std::make_unique(); - std::unique_ptr st(NewDispatchThread(port, 4, my_conn_factory.get(), 1000)); + ConnFactory* my_conn_factory = new MyConnFactory(); + ServerThread* st = NewDispatchThread(port, 4, my_conn_factory, 1000); if (st->StartThread() != 0) { printf("StartThread error happened!\n"); @@ -89,5 +89,8 @@ int main(int argc, char* argv[]) { } st->StopThread(); + delete st; + delete my_conn_factory; + return 0; } diff --git a/src/net/examples/thread_pool_test.cc b/src/net/examples/thread_pool_test.cc index 7216c4141..d40de4a9f 100644 --- a/src/net/examples/thread_pool_test.cc +++ b/src/net/examples/thread_pool_test.cc @@ -25,7 +25,7 @@ static pstd::Mutex print_lock; void task(void* arg) { { - pstd::MutexLock l(&print_lock); + std::lock_guard l(print_lock); std::cout << " task : " << *((int*)arg) << " time(micros) " << NowMicros() << " thread id: " << pthread_self() << std::endl; } @@ -46,7 +46,7 @@ int main() { t.Schedule(task, (void*)pi); t.cur_queue_size(&qsize); t.cur_time_queue_size(&pqsize); - pstd::MutexLock l(&print_lock); + std::lock_guard l(print_lock); std::cout << " current queue size:" << qsize << ", " << pqsize << std::endl; } @@ -66,7 +66,7 @@ int main() { t.DelaySchedule(i * 1000, task, (void*)pi); t.cur_queue_size(&qsize); t.cur_time_queue_size(&pqsize); - pstd::MutexLock l(&print_lock); + std::lock_guard l(print_lock); std::cout << "Schedule task " << i << " time(micros) " << NowMicros() << " for " << i * 1000 * 1000 << " micros " << std::endl; } @@ -85,7 +85,7 @@ int main() { t.DelaySchedule(i * 1000, task, (void*)pi); t.cur_queue_size(&qsize); t.cur_time_queue_size(&pqsize); - pstd::MutexLock l(&print_lock); + std::lock_guard l(print_lock); std::cout << " current queue size:" << qsize << ", " << pqsize << std::endl; } sleep(3); diff --git a/src/net/include/net_cli.h b/src/net/include/net_cli.h index 2e174749c..b17881ae3 100644 --- a/src/net/include/net_cli.h +++ b/src/net/include/net_cli.h @@ -6,8 +6,8 @@ #ifndef NET_INCLUDE_NET_CLI_H_ #define NET_INCLUDE_NET_CLI_H_ -#include #include + #include "pstd/include/pstd_status.h" using pstd::Status; @@ -47,7 +47,7 @@ class NetCli { private: struct Rep; - std::shared_ptr rep_; + Rep* rep_; int set_tcp_nodelay(); NetCli(const NetCli&); diff --git a/src/net/include/server_thread.h b/src/net/include/server_thread.h index a445d2954..387db032f 100644 --- a/src/net/include/server_thread.h +++ b/src/net/include/server_thread.h @@ -188,7 +188,7 @@ class ServerThread : public Thread { */ int port_ = -1; std::set ips_; - std::vector> server_sockets_; + std::vector server_sockets_; std::set server_fds_; virtual int InitHandle(); diff --git a/src/net/src/dispatch_thread.cc b/src/net/src/dispatch_thread.cc index a3d581a23..1035aec44 100644 --- a/src/net/src/dispatch_thread.cc +++ b/src/net/src/dispatch_thread.cc @@ -20,8 +20,9 @@ DispatchThread::DispatchThread(int port, int work_num, ConnFactory* conn_factory last_thread_(0), work_num_(work_num), queue_limit_(queue_limit) { + worker_thread_ = new WorkerThread*[work_num_]; for (int i = 0; i < work_num_; i++) { - worker_thread_.emplace_back(std::make_shared(conn_factory, this, queue_limit, cron_interval)); + worker_thread_[i] = new WorkerThread(conn_factory, this, queue_limit, cron_interval); } } @@ -31,9 +32,9 @@ DispatchThread::DispatchThread(const std::string& ip, int port, int work_num, Co last_thread_(0), work_num_(work_num), queue_limit_(queue_limit) { - + worker_thread_ = new WorkerThread*[work_num_]; for (int i = 0; i < work_num_; i++) { - worker_thread_.emplace_back(std::make_shared(conn_factory, this, queue_limit, cron_interval)); + worker_thread_[i] = new WorkerThread(conn_factory, this, queue_limit, cron_interval); } } @@ -43,15 +44,17 @@ DispatchThread::DispatchThread(const std::set& ips, int port, int w last_thread_(0), work_num_(work_num), queue_limit_(queue_limit) { - + worker_thread_ = new WorkerThread*[work_num_]; for (int i = 0; i < work_num_; i++) { - worker_thread_.emplace_back(std::make_shared(conn_factory, this, queue_limit, cron_interval)); - + worker_thread_[i] = new WorkerThread(conn_factory, this, queue_limit, cron_interval); } } DispatchThread::~DispatchThread() { - + for (int i = 0; i < work_num_; i++) { + delete worker_thread_[i]; + } + delete[] worker_thread_; } int DispatchThread::StartThread() { @@ -126,7 +129,7 @@ std::shared_ptr DispatchThread::MoveConnOut(int fd) { } void DispatchThread::MoveConnIn(std::shared_ptr conn, const NotifyType& type) { - std::shared_ptr worker_thread = worker_thread_[last_thread_]; + WorkerThread* worker_thread = worker_thread_[last_thread_]; bool success = worker_thread->MoveConnIn(conn, type, true); if (success) { last_thread_ = (last_thread_ + 1) % work_num_; @@ -152,7 +155,7 @@ void DispatchThread::HandleNewConn(const int connfd, const std::string& ip_port) int next_thread = last_thread_; bool find = false; for (int cnt = 0; cnt < work_num_; cnt++) { - std::shared_ptr worker_thread = worker_thread_[next_thread]; + WorkerThread* worker_thread = worker_thread_[next_thread]; find = worker_thread->MoveConnIn(ti, false); if (find) { last_thread_ = (next_thread + 1) % work_num_; diff --git a/src/net/src/dispatch_thread.h b/src/net/src/dispatch_thread.h index bf23e16ab..a3887b3ae 100644 --- a/src/net/src/dispatch_thread.h +++ b/src/net/src/dispatch_thread.h @@ -65,7 +65,7 @@ class DispatchThread : public ServerThread { /* * This is the work threads */ - std::vector> worker_thread_; + WorkerThread** worker_thread_; int queue_limit_; std::map localdata_; diff --git a/src/net/src/net_cli.cc b/src/net/src/net_cli.cc index 89b4c3a30..bb97b3c72 100644 --- a/src/net/src/net_cli.cc +++ b/src/net/src/net_cli.cc @@ -48,16 +48,19 @@ struct NetCli::Rep { available(false) {} }; -NetCli::NetCli(const std::string& ip, const int port) : rep_(std::make_shared(ip, port)) {} +NetCli::NetCli(const std::string& ip, const int port) : rep_(new Rep(ip, port)) {} -NetCli::~NetCli() { Close(); } +NetCli::~NetCli() { + Close(); + delete rep_; +} bool NetCli::Available() const { return rep_->available; } Status NetCli::Connect(const std::string& bind_ip) { return Connect(rep_->peer_ip, rep_->peer_port, bind_ip); } Status NetCli::Connect(const std::string& ip, const int port, const std::string& bind_ip) { - std::shared_ptr r = rep_; + Rep* r = rep_; Status s; int rv; char cport[6]; @@ -243,7 +246,7 @@ Status NetCli::SendRaw(void* buf, size_t count) { } Status NetCli::RecvRaw(void* buf, size_t* count) { - std::shared_ptr r = rep_; + Rep* r = rep_; char* rbuf = reinterpret_cast(buf); size_t nleft = *count; size_t pos = 0; @@ -282,7 +285,7 @@ void NetCli::Close() { void NetCli::set_connect_timeout(int connect_timeout) { rep_->connect_timeout = connect_timeout; } int NetCli::set_send_timeout(int send_timeout) { - std::shared_ptr r = rep_; + Rep* r = rep_; int ret = 0; if (send_timeout > 0) { r->send_timeout = send_timeout; @@ -293,7 +296,7 @@ int NetCli::set_send_timeout(int send_timeout) { } int NetCli::set_recv_timeout(int recv_timeout) { - std::shared_ptr r = rep_; + Rep* r = rep_; int ret = 0; if (recv_timeout > 0) { r->recv_timeout = recv_timeout; @@ -304,7 +307,7 @@ int NetCli::set_recv_timeout(int recv_timeout) { } int NetCli::set_tcp_nodelay() { - std::shared_ptr r = rep_; + Rep* r = rep_; int val = 1; int ret = 0; ret = setsockopt(r->sockfd, IPPROTO_TCP, TCP_NODELAY, &val, sizeof(val)); diff --git a/src/net/src/server_thread.cc b/src/net/src/server_thread.cc index c64e5a508..4afa5a20a 100644 --- a/src/net/src/server_thread.cc +++ b/src/net/src/server_thread.cc @@ -106,7 +106,9 @@ ServerThread::~ServerThread() { EVP_cleanup(); } #endif - + for (std::vector::iterator iter = server_sockets_.begin(); iter != server_sockets_.end(); ++iter) { + delete *iter; + } if (own_handle_) { delete handle_; } @@ -126,14 +128,14 @@ int ServerThread::StartThread() { int ServerThread::InitHandle() { int ret = 0; - std::shared_ptr socket_p; + ServerSocket* socket_p; if (ips_.find("0.0.0.0") != ips_.end()) { ips_.clear(); ips_.insert("0.0.0.0"); } for (std::set::iterator iter = ips_.begin(); iter != ips_.end(); ++iter) { - socket_p = std::make_shared(port_); - server_sockets_.emplace_back(socket_p); + socket_p = new ServerSocket(port_); + server_sockets_.push_back(socket_p); ret = socket_p->Listen(*iter); if (ret != kSuccess) { return ret; @@ -253,6 +255,9 @@ void* ServerThread::ThreadMain() { } } + for (auto iter = server_sockets_.begin(); iter != server_sockets_.end(); iter++) { + delete *iter; + } server_sockets_.clear(); server_fds_.clear(); @@ -307,7 +312,7 @@ int ServerThread::EnableSecurity(const std::string& cert_file, const std::string } if (SSL_CTX_use_PrivateKey_file(ssl_ctx_, key_file.c_str(), SSL_FILETYPE_PEM) != 1) { - LOG(WARNING) << "SSL_CTX_use_PrivateKey_file(" << key_file << ")"; + LOG(WARNING) << "SSL_CTX_use_PrivateKey_file(" << key_file << ")"; return -1; } From 6a5add3d5a846b1deb389aa07b7ff5d0127fce1d Mon Sep 17 00:00:00 2001 From: laimuxi <1721261216@qq.com> Date: Sun, 14 May 2023 21:21:57 +0800 Subject: [PATCH 13/17] fix tools build --- include/pika_partition.h | 2 +- tools/binlog_sender/binlog_consumer.cc | 7 ++--- tools/binlog_sender/binlog_consumer.h | 2 +- .../manifest_generator/include/pika_binlog.h | 8 ++--- tools/manifest_generator/pika_binlog.cc | 29 ++++++++----------- tools/pika-port/pika_port_3/pika_binlog.cc | 29 ++++++++----------- tools/pika-port/pika_port_3/pika_binlog.h | 8 ++--- tools/rdb_to_pika/protocoltopika.cc | 2 +- 8 files changed, 38 insertions(+), 49 deletions(-) diff --git a/include/pika_partition.h b/include/pika_partition.h index bade7fbeb..998b67555 100644 --- a/include/pika_partition.h +++ b/include/pika_partition.h @@ -97,7 +97,7 @@ class Partition : public std::enable_shared_from_this { bool opened_ = false; std::shared_mutex db_rwlock_; - std::shared_ptr lock_mgr_ = nullptr; + std::shared_ptr lock_mgr_; std::shared_ptr db_; bool full_sync_ = false; diff --git a/tools/binlog_sender/binlog_consumer.cc b/tools/binlog_sender/binlog_consumer.cc index ed1124192..71e195584 100644 --- a/tools/binlog_sender/binlog_consumer.cc +++ b/tools/binlog_sender/binlog_consumer.cc @@ -16,7 +16,6 @@ BinlogConsumer::BinlogConsumer(const std::string& binlog_path, uint32_t first_fi BinlogConsumer::~BinlogConsumer() { delete[] backing_store_; - delete queue_; } std::string BinlogConsumer::NewFileName(const std::string& name, const uint32_t current) { @@ -37,7 +36,7 @@ bool BinlogConsumer::Init() { current_filenum_ = first_filenum_; profile = NewFileName(filename_, current_filenum_); - pstd::Status s = pstd::NewSequentialFile(profile, &queue_); + pstd::Status s = pstd::NewSequentialFile(profile, queue_); if (!s.ok()) { return false; } else { @@ -212,10 +211,10 @@ pstd::Status BinlogConsumer::Parse(std::string* scratch) { // Roll to next File if (pstd::FileExists(confile)) { // DLOG(INFO) << "BinlogSender roll to new binlog" << confile; - delete queue_; + queue_.reset(); queue_ = nullptr; - pstd::NewSequentialFile(confile, &queue_); + pstd::NewSequentialFile(confile, queue_); current_filenum_++; current_offset_ = 0; diff --git a/tools/binlog_sender/binlog_consumer.h b/tools/binlog_sender/binlog_consumer.h index 992cd1b9e..1e0766019 100644 --- a/tools/binlog_sender/binlog_consumer.h +++ b/tools/binlog_sender/binlog_consumer.h @@ -58,7 +58,7 @@ class BinlogConsumer { pstd::Slice buffer_; char* const backing_store_; - pstd::SequentialFile* queue_; + std::unique_ptr queue_; }; #endif // INCLUDE_BINLOG_Consumber_H_ diff --git a/tools/manifest_generator/include/pika_binlog.h b/tools/manifest_generator/include/pika_binlog.h index a99d7bd74..4c4a79480 100644 --- a/tools/manifest_generator/include/pika_binlog.h +++ b/tools/manifest_generator/include/pika_binlog.h @@ -65,7 +65,7 @@ class Binlog { static Status AppendBlank(pstd::WritableFile* file, uint64_t len); - pstd::WritableFile* queue() { return queue_; } + // pstd::WritableFile* queue() { return queue_; } uint64_t file_size() { return file_size_; } @@ -83,9 +83,9 @@ class Binlog { uint32_t consumer_num_; uint64_t item_num_; - Version* version_; - pstd::WritableFile* queue_; - pstd::RWFile* versionfile_; + std::unique_ptr version_; + std::unique_ptr queue_; + std::unique_ptr versionfile_; pstd::Mutex mutex_; diff --git a/tools/manifest_generator/pika_binlog.cc b/tools/manifest_generator/pika_binlog.cc index 5f19efe06..df0baff97 100644 --- a/tools/manifest_generator/pika_binlog.cc +++ b/tools/manifest_generator/pika_binlog.cc @@ -74,26 +74,26 @@ Binlog::Binlog(const std::string& binlog_path, const int file_size) std::cout << "Binlog: Manifest file not exist, we create a new one."; profile = NewFileName(filename, pro_num_); - s = pstd::NewWritableFile(profile, &queue_); + s = pstd::NewWritableFile(profile, queue_); if (!s.ok()) { std::cout << "Binlog: new " << filename << " " << s.ToString(); exit(-1); } - s = pstd::NewRWFile(manifest, &versionfile_); + s = pstd::NewRWFile(manifest, versionfile_); if (!s.ok()) { std::cout << "Binlog: new versionfile error " << s.ToString(); exit(-1); } - version_ = new Version(versionfile_); + version_ = std::unique_ptr(new Version(versionfile_.get())); version_->StableSave(); } else { std::cout << "Binlog: Find the exist file."; - s = pstd::NewRWFile(manifest, &versionfile_); + s = pstd::NewRWFile(manifest, versionfile_); if (s.ok()) { - version_ = new Version(versionfile_); + version_ = std::unique_ptr(new Version(versionfile_.get())); version_->Init(); pro_num_ = version_->pro_num_; @@ -105,7 +105,7 @@ Binlog::Binlog(const std::string& binlog_path, const int file_size) } profile = NewFileName(filename, pro_num_); - s = pstd::AppendWritableFile(profile, &queue_, version_->pro_offset_); + s = pstd::AppendWritableFile(profile, queue_, version_->pro_offset_); if (!s.ok()) { std::cout << "Binlog: Open file " << profile << " error " << s.ToString(); exit(-1); @@ -115,12 +115,7 @@ Binlog::Binlog(const std::string& binlog_path, const int file_size) InitLogFile(); } -Binlog::~Binlog() { - delete version_; - delete versionfile_; - - delete queue_; -} +Binlog::~Binlog() {} void Binlog::InitLogFile() { assert(queue_ != nullptr); @@ -151,12 +146,12 @@ Status Binlog::Put(const char* item, int len) { /* Check to roll log file */ uint64_t filesize = queue_->Filesize(); if (filesize > file_size_) { - delete queue_; + queue_.reset(); queue_ = nullptr; pro_num_++; std::string profile = NewFileName(filename, pro_num_); - pstd::NewWritableFile(profile, &queue_); + pstd::NewWritableFile(profile, queue_); { std::lock_guard l(version_->rwlock_); @@ -309,7 +304,7 @@ Status Binlog::SetProducerStatus(uint32_t pro_num, uint64_t pro_offset) { pro_offset = 0; } - delete queue_; + queue_.reset(); std::string init_profile = NewFileName(filename, 0); if (pstd::FileExists(init_profile)) { @@ -321,8 +316,8 @@ Status Binlog::SetProducerStatus(uint32_t pro_num, uint64_t pro_offset) { pstd::DeleteFile(profile); } - pstd::NewWritableFile(profile, &queue_); - Binlog::AppendBlank(queue_, pro_offset); + pstd::NewWritableFile(profile, queue_); + Binlog::AppendBlank(queue_.get(), pro_offset); pro_num_ = pro_num; diff --git a/tools/pika-port/pika_port_3/pika_binlog.cc b/tools/pika-port/pika_port_3/pika_binlog.cc index ca18a8b81..943910328 100644 --- a/tools/pika-port/pika_port_3/pika_binlog.cc +++ b/tools/pika-port/pika_port_3/pika_binlog.cc @@ -87,24 +87,24 @@ Binlog::Binlog(const std::string& binlog_path, const int file_size) LOG(INFO) << "Binlog: Manifest file not exist, we create a new one."; profile = NewFileName(filename, pro_num_); - s = pstd::NewWritableFile(profile, &queue_); + s = pstd::NewWritableFile(profile, queue_); if (!s.ok()) { LOG(FATAL) << "Binlog: NewWritableFile(" << filename << ") = " << s.ToString(); } - s = pstd::NewRWFile(manifest, &versionfile_); + s = pstd::NewRWFile(manifest, versionfile_); if (!s.ok()) { LOG(FATAL) << "Binlog: new versionfile error " << s.ToString(); } - version_ = new Version(versionfile_); + version_ = std::unique_ptr(new Version(versionfile_.get())); version_->StableSave(); } else { LOG(INFO) << "Binlog: Find the exist file."; - s = pstd::NewRWFile(manifest, &versionfile_); + s = pstd::NewRWFile(manifest, versionfile_); if (s.ok()) { - version_ = new Version(versionfile_); + version_ = std::unique_ptr(new Version(versionfile_.get())); version_->Init(); pro_num_ = version_->pro_num_; @@ -116,7 +116,7 @@ Binlog::Binlog(const std::string& binlog_path, const int file_size) profile = NewFileName(filename, pro_num_); LOG(INFO) << "Binlog: open profile " << profile; - s = pstd::AppendWritableFile(profile, &queue_, version_->pro_offset_); + s = pstd::AppendWritableFile(profile, queue_, version_->pro_offset_); if (!s.ok()) { LOG(FATAL) << "Binlog: Open file " << profile << " error " << s.ToString(); } @@ -128,12 +128,7 @@ Binlog::Binlog(const std::string& binlog_path, const int file_size) InitLogFile(); } -Binlog::~Binlog() { - delete version_; - delete versionfile_; - - delete queue_; -} +Binlog::~Binlog() {} void Binlog::InitLogFile() { assert(queue_); @@ -164,12 +159,12 @@ Status Binlog::Put(const char* item, int len) { /* Check to roll log file */ uint64_t filesize = queue_->Filesize(); if (filesize > file_size_) { - delete queue_; + queue_.reset(); queue_ = nullptr; pro_num_++; std::string profile = NewFileName(filename, pro_num_); - pstd::NewWritableFile(profile, &queue_); + pstd::NewWritableFile(profile, queue_); { std::lock_guard l(version_->rwlock_); @@ -322,7 +317,7 @@ Status Binlog::SetProducerStatus(uint32_t pro_num, uint64_t pro_offset) { pro_offset = 0; } - delete queue_; + queue_.reset(); std::string init_profile = NewFileName(filename, 0); if (pstd::FileExists(init_profile)) { @@ -334,8 +329,8 @@ Status Binlog::SetProducerStatus(uint32_t pro_num, uint64_t pro_offset) { pstd::DeleteFile(profile); } - pstd::NewWritableFile(profile, &queue_); - Binlog::AppendBlank(queue_, pro_offset); + pstd::NewWritableFile(profile, queue_); + Binlog::AppendBlank(queue_.get(), pro_offset); pro_num_ = pro_num; diff --git a/tools/pika-port/pika_port_3/pika_binlog.h b/tools/pika-port/pika_port_3/pika_binlog.h index dd6085d29..5cdd6ac7d 100644 --- a/tools/pika-port/pika_port_3/pika_binlog.h +++ b/tools/pika-port/pika_port_3/pika_binlog.h @@ -45,7 +45,7 @@ class Binlog { static Status AppendBlank(pstd::WritableFile* file, uint64_t len); - pstd::WritableFile* queue() { return queue_; } + // pstd::WritableFile* queue() { return queue_; } uint64_t file_size() { return file_size_; } @@ -63,9 +63,9 @@ class Binlog { uint32_t consumer_num_; uint64_t item_num_; - Version* version_; - pstd::WritableFile* queue_; - pstd::RWFile* versionfile_; + std::unique_ptr version_; + std::unique_ptr queue_; + std::unique_ptr versionfile_; pstd::Mutex mutex_; diff --git a/tools/rdb_to_pika/protocoltopika.cc b/tools/rdb_to_pika/protocoltopika.cc index f1c9ae998..b87ae3ba5 100644 --- a/tools/rdb_to_pika/protocoltopika.cc +++ b/tools/rdb_to_pika/protocoltopika.cc @@ -3,7 +3,7 @@ #include #include #include -#include "hiredis-vip/hiredis.h" +#include void Usage() { std::cout << "Usage:" << std::endl; From 0abe10a0cd664701018c5cf48bb8c43ab4d98a22 Mon Sep 17 00:00:00 2001 From: laimuxi <1721261216@qq.com> Date: Tue, 16 May 2023 19:07:31 +0800 Subject: [PATCH 14/17] fix --- include/pika_consensus.h | 2 +- src/pika_admin.cc | 2 +- src/pika_auxiliary_thread.cc | 2 +- src/pika_client_conn.cc | 2 +- src/pika_command.cc | 2 +- src/pika_consensus.cc | 6 ++---- src/pika_dispatch_thread.cc | 2 +- src/pika_partition.cc | 2 +- src/pika_pubsub.cc | 2 +- src/pika_repl_bgworker.cc | 2 +- src/pika_repl_client.cc | 2 +- src/pika_repl_client_conn.cc | 2 +- src/pika_repl_client_thread.cc | 2 +- src/pika_repl_server.cc | 2 +- src/pika_repl_server_conn.cc | 2 +- src/pika_repl_server_thread.cc | 2 +- src/pika_rm.cc | 2 +- src/pika_server.cc | 2 +- src/pika_stable_log.cc | 2 +- src/pika_table.cc | 2 +- 20 files changed, 21 insertions(+), 23 deletions(-) diff --git a/include/pika_consensus.h b/include/pika_consensus.h index 24d199882..31b83341c 100644 --- a/include/pika_consensus.h +++ b/include/pika_consensus.h @@ -37,7 +37,7 @@ class Context { private: std::string path_; - std::shared_ptr save_; + std::unique_ptr save_; // No copying allowed; Context(const Context&); void operator=(const Context&); diff --git a/src/pika_admin.cc b/src/pika_admin.cc index a9380c405..107d45b21 100644 --- a/src/pika_admin.cc +++ b/src/pika_admin.cc @@ -19,7 +19,7 @@ #include "include/pika_version.h" #include "pstd/include/rsync.h" -extern PikaServer* g_pika_server;; +extern PikaServer* g_pika_server; extern std::unique_ptr g_pika_conf; extern PikaReplicaManager* g_pika_rm; diff --git a/src/pika_auxiliary_thread.cc b/src/pika_auxiliary_thread.cc index d35153eb2..f8fc7effb 100644 --- a/src/pika_auxiliary_thread.cc +++ b/src/pika_auxiliary_thread.cc @@ -9,7 +9,7 @@ #include "include/pika_rm.h" #include "include/pika_server.h" -extern PikaServer* g_pika_server;; +extern PikaServer* g_pika_server; extern PikaReplicaManager* g_pika_rm; using namespace std::chrono_literals; diff --git a/src/pika_client_conn.cc b/src/pika_client_conn.cc index 62358df9e..e36390a12 100644 --- a/src/pika_client_conn.cc +++ b/src/pika_client_conn.cc @@ -17,7 +17,7 @@ #include "include/pika_server.h" extern std::unique_ptr g_pika_conf; -extern PikaServer* g_pika_server;; +extern PikaServer* g_pika_server; extern PikaReplicaManager* g_pika_rm; extern PikaCmdTableManager* g_pika_cmd_table_manager; diff --git a/src/pika_command.cc b/src/pika_command.cc index eea71b10a..0ae0c2692 100644 --- a/src/pika_command.cc +++ b/src/pika_command.cc @@ -19,7 +19,7 @@ #include "include/pika_set.h" #include "include/pika_zset.h" -extern PikaServer* g_pika_server;; +extern PikaServer* g_pika_server; extern PikaReplicaManager* g_pika_rm; extern PikaCmdTableManager* g_pika_cmd_table_manager; diff --git a/src/pika_consensus.cc b/src/pika_consensus.cc index e78728834..b5a06ea3b 100644 --- a/src/pika_consensus.cc +++ b/src/pika_consensus.cc @@ -11,7 +11,7 @@ #include "include/pika_rm.h" #include "include/pika_server.h" -extern PikaServer* g_pika_server;; +extern PikaServer* g_pika_server; extern std::unique_ptr g_pika_conf; extern PikaReplicaManager* g_pika_rm; extern PikaCmdTableManager* g_pika_cmd_table_manager; @@ -34,9 +34,7 @@ Status Context::StableSave() { Status Context::Init() { if (!pstd::FileExists(path_)) { - std::unique_ptr tmp_file; - Status s = pstd::NewRWFile(path_, tmp_file); - save_.reset(tmp_file.release()); + Status s = pstd::NewRWFile(path_, save_); if (!s.ok()) { LOG(FATAL) << "Context new file failed " << s.ToString(); } diff --git a/src/pika_dispatch_thread.cc b/src/pika_dispatch_thread.cc index 80a016c95..2904d18ba 100644 --- a/src/pika_dispatch_thread.cc +++ b/src/pika_dispatch_thread.cc @@ -12,7 +12,7 @@ #include "pstd/include/testutil.h" extern std::unique_ptr g_pika_conf; -extern PikaServer* g_pika_server;; +extern PikaServer* g_pika_server; PikaDispatchThread::PikaDispatchThread(std::set& ips, int port, int work_num, int cron_interval, int queue_limit, int max_conn_rbuf_size) diff --git a/src/pika_partition.cc b/src/pika_partition.cc index 4d7931c59..28515fab9 100644 --- a/src/pika_partition.cc +++ b/src/pika_partition.cc @@ -14,7 +14,7 @@ #include "pstd/include/mutex_impl.h" extern std::unique_ptr g_pika_conf; -extern PikaServer* g_pika_server;; +extern PikaServer* g_pika_server; extern PikaReplicaManager* g_pika_rm; std::string PartitionPath(const std::string& table_path, uint32_t partition_id) { diff --git a/src/pika_pubsub.cc b/src/pika_pubsub.cc index b073a5ff5..9812131de 100644 --- a/src/pika_pubsub.cc +++ b/src/pika_pubsub.cc @@ -7,7 +7,7 @@ #include "include/pika_server.h" -extern PikaServer* g_pika_server;; +extern PikaServer* g_pika_server; static std::string ConstructPubSubResp(const std::string& cmd, const std::vector>& result) { std::stringstream resp; diff --git a/src/pika_repl_bgworker.cc b/src/pika_repl_bgworker.cc index a35168ebe..ea2018b3a 100644 --- a/src/pika_repl_bgworker.cc +++ b/src/pika_repl_bgworker.cc @@ -13,7 +13,7 @@ #include "include/pika_server.h" extern std::unique_ptr g_pika_conf; -extern PikaServer* g_pika_server;; +extern PikaServer* g_pika_server; extern PikaReplicaManager* g_pika_rm; extern PikaCmdTableManager* g_pika_cmd_table_manager; diff --git a/src/pika_repl_client.cc b/src/pika_repl_client.cc index b34628213..06954d394 100644 --- a/src/pika_repl_client.cc +++ b/src/pika_repl_client.cc @@ -18,7 +18,7 @@ #include "include/pika_rm.h" #include "include/pika_server.h" -extern PikaServer* g_pika_server;; +extern PikaServer* g_pika_server; extern PikaReplicaManager* g_pika_rm; PikaReplClient::PikaReplClient(int cron_interval, int keepalive_timeout) : next_avail_(0) { diff --git a/src/pika_repl_client_conn.cc b/src/pika_repl_client_conn.cc index b7dcbb77f..5735cbf83 100644 --- a/src/pika_repl_client_conn.cc +++ b/src/pika_repl_client_conn.cc @@ -19,7 +19,7 @@ #include "pika_inner_message.pb.h" extern std::unique_ptr g_pika_conf; -extern PikaServer* g_pika_server;; +extern PikaServer* g_pika_server; extern PikaReplicaManager* g_pika_rm; PikaReplClientConn::PikaReplClientConn(int fd, const std::string& ip_port, net::Thread* thread, diff --git a/src/pika_repl_client_thread.cc b/src/pika_repl_client_thread.cc index 85830fa53..165e3254a 100644 --- a/src/pika_repl_client_thread.cc +++ b/src/pika_repl_client_thread.cc @@ -10,7 +10,7 @@ #include "pstd/include/pstd_string.h" -extern PikaServer* g_pika_server;; +extern PikaServer* g_pika_server; extern PikaReplicaManager* g_pika_rm; PikaReplClientThread::PikaReplClientThread(int cron_interval, int keepalive_timeout) diff --git a/src/pika_repl_server.cc b/src/pika_repl_server.cc index 9e335d434..db3b12e36 100644 --- a/src/pika_repl_server.cc +++ b/src/pika_repl_server.cc @@ -12,7 +12,7 @@ #include "include/pika_server.h" extern std::unique_ptr g_pika_conf; -extern PikaServer* g_pika_server;; +extern PikaServer* g_pika_server; extern PikaReplicaManager* g_pika_rm; PikaReplServer::PikaReplServer(const std::set& ips, int port, int cron_interval) { diff --git a/src/pika_repl_server_conn.cc b/src/pika_repl_server_conn.cc index 0141cc0be..829fad70c 100644 --- a/src/pika_repl_server_conn.cc +++ b/src/pika_repl_server_conn.cc @@ -10,7 +10,7 @@ #include "include/pika_rm.h" #include "include/pika_server.h" -extern PikaServer* g_pika_server;; +extern PikaServer* g_pika_server; extern PikaReplicaManager* g_pika_rm; PikaReplServerConn::PikaReplServerConn(int fd, std::string ip_port, net::Thread* thread, void* worker_specific_data, diff --git a/src/pika_repl_server_thread.cc b/src/pika_repl_server_thread.cc index 5a7f3373d..bc636d8e7 100644 --- a/src/pika_repl_server_thread.cc +++ b/src/pika_repl_server_thread.cc @@ -8,7 +8,7 @@ #include "include/pika_rm.h" #include "include/pika_server.h" -extern PikaServer* g_pika_server;; +extern PikaServer* g_pika_server; extern PikaReplicaManager* g_pika_rm; PikaReplServerThread::PikaReplServerThread(const std::set& ips, int port, int cron_interval) diff --git a/src/pika_rm.cc b/src/pika_rm.cc index 359b4b457..45bc6ea39 100644 --- a/src/pika_rm.cc +++ b/src/pika_rm.cc @@ -20,7 +20,7 @@ extern std::unique_ptr g_pika_conf; extern PikaReplicaManager* g_pika_rm; -extern PikaServer* g_pika_server;; +extern PikaServer* g_pika_server; /* SyncPartition */ diff --git a/src/pika_server.cc b/src/pika_server.cc index 9182ca16c..fba8311c0 100644 --- a/src/pika_server.cc +++ b/src/pika_server.cc @@ -25,7 +25,7 @@ #include "include/pika_rm.h" #include "include/pika_server.h" -extern PikaServer* g_pika_server;; +extern PikaServer* g_pika_server; extern PikaReplicaManager* g_pika_rm; extern PikaCmdTableManager* g_pika_cmd_table_manager; diff --git a/src/pika_stable_log.cc b/src/pika_stable_log.cc index 2f2aaaa64..75e0756a1 100644 --- a/src/pika_stable_log.cc +++ b/src/pika_stable_log.cc @@ -14,7 +14,7 @@ #include "pstd/include/env.h" extern std::unique_ptr g_pika_conf; -extern PikaServer* g_pika_server;; +extern PikaServer* g_pika_server; extern PikaReplicaManager* g_pika_rm; StableLog::StableLog(const std::string table_name, uint32_t partition_id, const std::string& log_path) diff --git a/src/pika_table.cc b/src/pika_table.cc index 62115cbe8..b660350ae 100644 --- a/src/pika_table.cc +++ b/src/pika_table.cc @@ -9,7 +9,7 @@ #include "include/pika_rm.h" #include "include/pika_server.h" -extern PikaServer* g_pika_server;; +extern PikaServer* g_pika_server; extern PikaReplicaManager* g_pika_rm; extern PikaCmdTableManager* g_pika_cmd_table_manager; From 6afda63edbc00bae4e31672f73539e51024af325 Mon Sep 17 00:00:00 2001 From: laimuxi <1721261216@qq.com> Date: Fri, 19 May 2023 22:04:47 +0800 Subject: [PATCH 15/17] feature --- include/pika_binlog_reader.h | 4 ++-- include/pika_client_processor.h | 6 +++--- include/pika_cmd_table_manager.h | 4 ++-- include/pika_conf.h | 6 +++--- include/pika_consensus.h | 2 +- include/pika_repl_client.h | 4 ++-- include/pika_repl_server.h | 4 ++-- include/pika_rm.h | 6 +++--- include/pika_server.h | 12 ++++++------ src/pika.cc | 10 ++++------ src/pika_admin.cc | 2 +- src/pika_auxiliary_thread.cc | 2 +- src/pika_binlog_reader.cc | 24 ++++++++++-------------- src/pika_client_conn.cc | 4 ++-- src/pika_client_processor.cc | 11 +++-------- src/pika_cmd_table_manager.cc | 20 ++++++-------------- src/pika_command.cc | 4 ++-- src/pika_conf.cc | 8 ++++---- src/pika_consensus.cc | 4 ++-- src/pika_partition.cc | 5 +++-- src/pika_repl_bgworker.cc | 4 ++-- src/pika_repl_client.cc | 16 +++++----------- src/pika_repl_client_conn.cc | 2 +- src/pika_repl_client_thread.cc | 2 +- src/pika_repl_server.cc | 8 +++----- src/pika_repl_server_conn.cc | 2 +- src/pika_repl_server_thread.cc | 2 +- src/pika_rm.cc | 15 ++++----------- src/pika_server.cc | 28 +++++++++------------------- src/pika_stable_log.cc | 2 +- src/pika_table.cc | 4 ++-- src/storage/src/db_checkpoint.cc | 2 ++ 32 files changed, 94 insertions(+), 135 deletions(-) diff --git a/include/pika_binlog_reader.h b/include/pika_binlog_reader.h index e75e5dfe8..a12e2dc0f 100644 --- a/include/pika_binlog_reader.h +++ b/include/pika_binlog_reader.h @@ -23,7 +23,7 @@ class PikaBinlogReader { public: PikaBinlogReader(uint32_t cur_filenum, uint64_t cur_offset); PikaBinlogReader(); - ~PikaBinlogReader(); + ~PikaBinlogReader() {}; Status Get(std::string* scratch, uint32_t* filenum, uint64_t* offset); int Seek(std::shared_ptr logger, uint32_t filenum, uint64_t offset); bool ReadToTheEnd(); @@ -43,7 +43,7 @@ class PikaBinlogReader { std::shared_ptr logger_; std::unique_ptr queue_; - char* const backing_store_; + std::unique_ptr const backing_store_; Slice buffer_; }; diff --git a/include/pika_client_processor.h b/include/pika_client_processor.h index d18ac6b68..59059c3a4 100644 --- a/include/pika_client_processor.h +++ b/include/pika_client_processor.h @@ -8,7 +8,7 @@ #include #include - +#include #include "net/include/bg_thread.h" #include "net/include/thread_pool.h" @@ -23,7 +23,7 @@ class PikaClientProcessor { size_t ThreadPoolCurQueueSize(); private: - net::ThreadPool* pool_ = nullptr; - std::vector bg_threads_; + std::unique_ptr pool_; + std::vector> bg_threads_; }; #endif // PIKA_CLIENT_PROCESSOR_H_ diff --git a/include/pika_cmd_table_manager.h b/include/pika_cmd_table_manager.h index 94c519cdd..e99695309 100644 --- a/include/pika_cmd_table_manager.h +++ b/include/pika_cmd_table_manager.h @@ -15,7 +15,7 @@ class PikaCmdTableManager { public: PikaCmdTableManager(); - virtual ~PikaCmdTableManager(); + virtual ~PikaCmdTableManager(){}; std::shared_ptr GetCmd(const std::string& opt); uint32_t DistributeKey(const std::string& key, uint32_t partition_num); @@ -28,6 +28,6 @@ class PikaCmdTableManager { std::unique_ptr cmds_; std::shared_mutex map_protector_; - std::unordered_map thread_distribution_map_; + std::unordered_map> thread_distribution_map_; }; #endif diff --git a/include/pika_conf.h b/include/pika_conf.h index 800de4d01..242072aba 100644 --- a/include/pika_conf.h +++ b/include/pika_conf.h @@ -26,7 +26,7 @@ class PikaConf : public pstd::BaseConf { public: PikaConf(const std::string& path); - ~PikaConf(); + ~PikaConf(){}; // Getter int port() { @@ -291,7 +291,7 @@ class PikaConf : public pstd::BaseConf { bool daemonize() { return daemonize_; } std::string pidfile() { return pidfile_; } int binlog_file_size() { return binlog_file_size_; } - PikaMeta* local_meta() { return local_meta_; } + PikaMeta* local_meta() { return local_meta_.get(); } std::vector compression_per_level(); static rocksdb::CompressionType GetCompression(const std::string& value); @@ -576,7 +576,7 @@ class PikaConf : public pstd::BaseConf { int64_t blob_cache_ = 0; int64_t blob_num_shard_bits_ = 0; - PikaMeta* local_meta_ = nullptr; + std::unique_ptr local_meta_; std::shared_mutex rwlock_; }; diff --git a/include/pika_consensus.h b/include/pika_consensus.h index 31b83341c..bea3534ce 100644 --- a/include/pika_consensus.h +++ b/include/pika_consensus.h @@ -12,7 +12,7 @@ #include "include/pika_stable_log.h" #include "pstd/include/env.h" -class Context { +class Context final { public: Context(const std::string path); diff --git a/include/pika_repl_client.h b/include/pika_repl_client.h index 48fae9332..d47d3205c 100644 --- a/include/pika_repl_client.h +++ b/include/pika_repl_client.h @@ -85,10 +85,10 @@ class PikaReplClient { size_t GetHashIndex(std::string key, bool upper_half); void UpdateNextAvail() { next_avail_ = (next_avail_ + 1) % bg_workers_.size(); } - PikaReplClientThread* client_thread_; + std::unique_ptr client_thread_; int next_avail_ = 0; std::hash str_hash; - std::vector bg_workers_; + std::vector> bg_workers_; }; #endif diff --git a/include/pika_repl_server.h b/include/pika_repl_server.h index 852fe0e21..5e743dea3 100644 --- a/include/pika_repl_server.h +++ b/include/pika_repl_server.h @@ -41,8 +41,8 @@ class PikaReplServer { void KillAllConns(); private: - net::ThreadPool* server_tp_ = nullptr; - PikaReplServerThread* pika_repl_server_thread_ = nullptr; + std::unique_ptr server_tp_ = nullptr; + std::unique_ptr pika_repl_server_thread_ = nullptr; std::shared_mutex client_conn_rwlock_; std::map client_conn_map_; diff --git a/include/pika_rm.h b/include/pika_rm.h index 0ba202604..42b431393 100644 --- a/include/pika_rm.h +++ b/include/pika_rm.h @@ -167,7 +167,7 @@ class SyncSlavePartition : public SyncPartition { class PikaReplicaManager { public: PikaReplicaManager(); - ~PikaReplicaManager(); + ~PikaReplicaManager(){}; friend Cmd; @@ -255,8 +255,8 @@ class PikaReplicaManager { // every host owns a queue, the key is "ip+port" std::unordered_map>> write_queues_; - PikaReplClient* pika_repl_client_ = nullptr; - PikaReplServer* pika_repl_server_ = nullptr; + std::unique_ptr pika_repl_client_; + std::unique_ptr pika_repl_server_; }; #endif // PIKA_RM_H diff --git a/include/pika_server.h b/include/pika_server.h index 18e2ae213..2fa3c69f8 100644 --- a/include/pika_server.h +++ b/include/pika_server.h @@ -370,8 +370,8 @@ class PikaServer { * Communicate with the client used */ int worker_num_ = 0; - PikaClientProcessor* pika_client_processor_ = nullptr; - PikaDispatchThread* pika_dispatch_thread_ = nullptr; + std::unique_ptr pika_client_processor_; + std::unique_ptr pika_dispatch_thread_ = nullptr; /* * Slave used @@ -411,22 +411,22 @@ class PikaServer { /* * Monitor used */ - PikaMonitorThread* pika_monitor_thread_ = nullptr; + std::unique_ptr pika_monitor_thread_; /* * Rsync used */ - PikaRsyncService* pika_rsync_service_ = nullptr; + std::unique_ptr pika_rsync_service_; /* * Pubsub used */ - net::PubSubThread* pika_pubsub_thread_ = nullptr; + std::unique_ptr pika_pubsub_thread_; /* * Communication used */ - PikaAuxiliaryThread* pika_auxiliary_thread_ = nullptr; + std::unique_ptr pika_auxiliary_thread_; /* * Slowlog used diff --git a/src/pika.cc b/src/pika.cc index 8288f181e..e8204657e 100644 --- a/src/pika.cc +++ b/src/pika.cc @@ -21,9 +21,9 @@ std::unique_ptr g_pika_conf; // todo : change to unique_ptr will coredump PikaServer* g_pika_server; -PikaReplicaManager* g_pika_rm; +std::unique_ptr g_pika_rm; -PikaCmdTableManager* g_pika_cmd_table_manager; +std::unique_ptr g_pika_cmd_table_manager; static void version() { char version[32]; @@ -184,9 +184,9 @@ int main(int argc, char* argv[]) { PikaSignalSetup(); LOG(INFO) << "Server at: " << path; - g_pika_cmd_table_manager = new PikaCmdTableManager(); + g_pika_cmd_table_manager = std::make_unique(); g_pika_server = new PikaServer(); - g_pika_rm = new PikaReplicaManager(); + g_pika_rm = std::make_unique(); if (g_pika_conf->daemonize()) { close_std(); @@ -204,8 +204,6 @@ int main(int argc, char* argv[]) { g_pika_rm->Stop(); delete g_pika_server; - delete g_pika_rm; - delete g_pika_cmd_table_manager; ::google::ShutdownGoogleLogging(); return 0; diff --git a/src/pika_admin.cc b/src/pika_admin.cc index 9985fa3cf..0c5343364 100644 --- a/src/pika_admin.cc +++ b/src/pika_admin.cc @@ -21,7 +21,7 @@ extern PikaServer* g_pika_server; extern std::unique_ptr g_pika_conf; -extern PikaReplicaManager* g_pika_rm; +extern std::unique_ptr g_pika_rm; static std::string ConstructPinginPubSubResp(const PikaCmdArgsType& argv) { if (argv.size() > 2) { diff --git a/src/pika_auxiliary_thread.cc b/src/pika_auxiliary_thread.cc index d65de9590..cfc654ae2 100644 --- a/src/pika_auxiliary_thread.cc +++ b/src/pika_auxiliary_thread.cc @@ -10,7 +10,7 @@ #include "include/pika_server.h" extern PikaServer* g_pika_server; -extern PikaReplicaManager* g_pika_rm; +extern std::unique_ptr g_pika_rm; using namespace std::chrono_literals; diff --git a/src/pika_binlog_reader.cc b/src/pika_binlog_reader.cc index 5aea194cb..0108c0e3a 100644 --- a/src/pika_binlog_reader.cc +++ b/src/pika_binlog_reader.cc @@ -12,7 +12,7 @@ PikaBinlogReader::PikaBinlogReader(uint32_t cur_filenum, uint64_t cur_offset) cur_offset_(cur_offset), logger_(nullptr), queue_(nullptr), - backing_store_(new char[kBlockSize]), + backing_store_(std::make_unique(kBlockSize)), buffer_() { last_record_offset_ = cur_offset % kBlockSize; } @@ -22,15 +22,11 @@ PikaBinlogReader::PikaBinlogReader() cur_offset_(0), logger_(nullptr), queue_(nullptr), - backing_store_(new char[kBlockSize]), + backing_store_(std::make_unique(kBlockSize)), buffer_() { last_record_offset_ = 0 % kBlockSize; } -PikaBinlogReader::~PikaBinlogReader() { - delete[] backing_store_; -} - void PikaBinlogReader::GetReaderStatus(uint32_t* cur_filenum, uint64_t* cur_offset) { std::shared_lock l(rwlock_); *cur_filenum = cur_filenum_; @@ -98,7 +94,7 @@ bool PikaBinlogReader::GetNext(uint64_t* size) { while (true) { buffer_.clear(); - s = queue_->Read(kHeaderSize, &buffer_, backing_store_); + s = queue_->Read(kHeaderSize, &buffer_, backing_store_.get()); if (!s.ok()) { is_error = true; return is_error; @@ -114,21 +110,21 @@ bool PikaBinlogReader::GetNext(uint64_t* size) { if (length > (kBlockSize - kHeaderSize)) return true; if (type == kFullType) { - s = queue_->Read(length, &buffer_, backing_store_); + s = queue_->Read(length, &buffer_, backing_store_.get()); offset += kHeaderSize + length; break; } else if (type == kFirstType) { - s = queue_->Read(length, &buffer_, backing_store_); + s = queue_->Read(length, &buffer_, backing_store_.get()); offset += kHeaderSize + length; } else if (type == kMiddleType) { - s = queue_->Read(length, &buffer_, backing_store_); + s = queue_->Read(length, &buffer_, backing_store_.get()); offset += kHeaderSize + length; } else if (type == kLastType) { - s = queue_->Read(length, &buffer_, backing_store_); + s = queue_->Read(length, &buffer_, backing_store_.get()); offset += kHeaderSize + length; break; } else if (type == kBadRecord) { - s = queue_->Read(length, &buffer_, backing_store_); + s = queue_->Read(length, &buffer_, backing_store_.get()); offset += kHeaderSize + length; break; } else { @@ -149,7 +145,7 @@ unsigned int PikaBinlogReader::ReadPhysicalRecord(pstd::Slice* result, uint32_t* last_record_offset_ = 0; } buffer_.clear(); - s = queue_->Read(kHeaderSize, &buffer_, backing_store_); + s = queue_->Read(kHeaderSize, &buffer_, backing_store_.get()); if (s.IsEndFile()) { return kEof; } else if (!s.ok()) { @@ -171,7 +167,7 @@ unsigned int PikaBinlogReader::ReadPhysicalRecord(pstd::Slice* result, uint32_t* } buffer_.clear(); - s = queue_->Read(length, &buffer_, backing_store_); + s = queue_->Read(length, &buffer_, backing_store_.get()); *result = pstd::Slice(buffer_.data(), buffer_.size()); last_record_offset_ += kHeaderSize + length; if (s.ok()) { diff --git a/src/pika_client_conn.cc b/src/pika_client_conn.cc index a51b5fc55..13e8fd77a 100644 --- a/src/pika_client_conn.cc +++ b/src/pika_client_conn.cc @@ -18,8 +18,8 @@ extern std::unique_ptr g_pika_conf; extern PikaServer* g_pika_server; -extern PikaReplicaManager* g_pika_rm; -extern PikaCmdTableManager* g_pika_cmd_table_manager; +extern std::unique_ptr g_pika_rm; +extern std::unique_ptr g_pika_cmd_table_manager; PikaClientConn::PikaClientConn(int fd, std::string ip_port, net::Thread* thread, net::NetMultiplexer* mpx, const net::HandleType& handle_type, int max_conn_rbuf_size) diff --git a/src/pika_client_processor.cc b/src/pika_client_processor.cc index 14b7979f8..9b2463e01 100644 --- a/src/pika_client_processor.cc +++ b/src/pika_client_processor.cc @@ -8,19 +8,14 @@ #include PikaClientProcessor::PikaClientProcessor(size_t worker_num, size_t max_queue_size, const std::string& name_prefix) { - pool_ = new net::ThreadPool(worker_num, max_queue_size, name_prefix + "Pool"); + pool_ = std::make_unique(worker_num, max_queue_size, name_prefix + "Pool"); for (size_t i = 0; i < worker_num; ++i) { - net::BGThread* bg_thread = new net::BGThread(max_queue_size); - bg_threads_.push_back(bg_thread); - bg_thread->set_thread_name(name_prefix + "BgThread"); + bg_threads_.push_back(std::make_unique(max_queue_size)); + bg_threads_.back()->set_thread_name(name_prefix + "BgThread"); } } PikaClientProcessor::~PikaClientProcessor() { - delete pool_; - for (size_t i = 0; i < bg_threads_.size(); ++i) { - delete bg_threads_[i]; - } LOG(INFO) << "PikaClientProcessor exit!!!"; } diff --git a/src/pika_cmd_table_manager.cc b/src/pika_cmd_table_manager.cc index 5f4d7be9c..967132dcf 100644 --- a/src/pika_cmd_table_manager.cc +++ b/src/pika_cmd_table_manager.cc @@ -14,17 +14,11 @@ extern std::unique_ptr g_pika_conf; PikaCmdTableManager::PikaCmdTableManager() { - cmds_ = std::unique_ptr(new CmdTable()); + cmds_ = std::make_unique(); cmds_->reserve(300); InitCmdTable(cmds_.get()); } -PikaCmdTableManager::~PikaCmdTableManager() { - for (const auto& item : thread_distribution_map_) { - delete item.second; - } -} - std::shared_ptr PikaCmdTableManager::GetCmd(const std::string& opt) { std::string internal_opt = opt; return NewCommand(internal_opt); @@ -48,25 +42,23 @@ bool PikaCmdTableManager::CheckCurrentThreadDistributionMapExist(const std::thre void PikaCmdTableManager::InsertCurrentThreadDistributionMap() { auto tid = std::this_thread::get_id(); - PikaDataDistribution* distribution = nullptr; + std::unique_ptr distribution; if (g_pika_conf->classic_mode()) { - distribution = new HashModulo(); + distribution = std::make_unique(); } else { - distribution = new Crc32(); + distribution = std::make_unique(); } distribution->Init(); std::lock_guard l(map_protector_); - thread_distribution_map_.emplace(tid, distribution); + thread_distribution_map_.emplace(tid, std::move(distribution)); } uint32_t PikaCmdTableManager::DistributeKey(const std::string& key, uint32_t partition_num) { auto tid = std::this_thread::get_id(); - PikaDataDistribution* data_dist = nullptr; if (!CheckCurrentThreadDistributionMapExist(tid)) { InsertCurrentThreadDistributionMap(); } std::shared_lock l(map_protector_); - data_dist = thread_distribution_map_[tid]; - return data_dist->Distribute(key, partition_num); + return thread_distribution_map_[tid]->Distribute(key, partition_num); } diff --git a/src/pika_command.cc b/src/pika_command.cc index 6d27f7349..cc07fad61 100644 --- a/src/pika_command.cc +++ b/src/pika_command.cc @@ -20,8 +20,8 @@ #include "include/pika_zset.h" extern PikaServer* g_pika_server; -extern PikaReplicaManager* g_pika_rm; -extern PikaCmdTableManager* g_pika_cmd_table_manager; +extern std::unique_ptr g_pika_rm; +extern std::unique_ptr g_pika_cmd_table_manager; void InitCmdTable(CmdTable* cmd_table) { // Admin diff --git a/src/pika_conf.cc b/src/pika_conf.cc index cc1d25a9e..cbda0face 100644 --- a/src/pika_conf.cc +++ b/src/pika_conf.cc @@ -14,9 +14,9 @@ #include "include/pika_define.h" -PikaConf::PikaConf(const std::string& path) : pstd::BaseConf(path), conf_path_(path) { local_meta_ = new PikaMeta(); } +PikaConf::PikaConf(const std::string& path) + : pstd::BaseConf(path), conf_path_(path), local_meta_(std::make_unique()) {} -PikaConf::~PikaConf() { delete local_meta_; } Status PikaConf::InternalGetTargetTable(const std::string& table_name, uint32_t* const target) { int32_t table_index = -1; @@ -365,13 +365,13 @@ int PikaConf::Load() { // rate-limiter-refill-period-us GetConfInt64("rate-limiter-refill-period-us", &rate_limiter_refill_period_us_); - if (rate_limiter_refill_period_us_ <= 0 ) { + if (rate_limiter_refill_period_us_ <= 0) { rate_limiter_refill_period_us_ = 100 * 1000; } // rate-limiter-fairness GetConfInt64("rate-limiter-fairness", &rate_limiter_fairness_); - if (rate_limiter_fairness_ <= 0 ) { + if (rate_limiter_fairness_ <= 0) { rate_limiter_fairness_ = 10; } diff --git a/src/pika_consensus.cc b/src/pika_consensus.cc index 211095bd3..f4c57150b 100644 --- a/src/pika_consensus.cc +++ b/src/pika_consensus.cc @@ -13,8 +13,8 @@ extern PikaServer* g_pika_server; extern std::unique_ptr g_pika_conf; -extern PikaReplicaManager* g_pika_rm; -extern PikaCmdTableManager* g_pika_cmd_table_manager; +extern std::unique_ptr g_pika_rm; +extern std::unique_ptr g_pika_cmd_table_manager; /* Context */ diff --git a/src/pika_partition.cc b/src/pika_partition.cc index 3fb589be6..e4c1dc903 100644 --- a/src/pika_partition.cc +++ b/src/pika_partition.cc @@ -15,7 +15,7 @@ extern std::unique_ptr g_pika_conf; extern PikaServer* g_pika_server; -extern PikaReplicaManager* g_pika_rm; +extern std::unique_ptr g_pika_rm; std::string PartitionPath(const std::string& table_path, uint32_t partition_id) { char buf[100]; @@ -55,7 +55,7 @@ Partition::Partition(const std::string& table_name, uint32_t partition_id, const dbsync_path_ = DbSyncPath(g_pika_conf->db_sync_path(), table_name_, partition_id_, g_pika_conf->classic_mode()); partition_name_ = g_pika_conf->classic_mode() ? table_name : PartitionName(table_name_, partition_id_); - db_ = std::make_shared(); + db_ = std::make_unique(); rocksdb::Status s = db_->Open(g_pika_server->storage_options(), db_path_); lock_mgr_ = std::make_shared(1000, 0, std::make_shared()); @@ -81,6 +81,7 @@ void Partition::Close() { } std::lock_guard lock(db_rwlock_); db_.reset(); + lock_mgr_.reset(); opened_ = false; } diff --git a/src/pika_repl_bgworker.cc b/src/pika_repl_bgworker.cc index bfcd8a6e5..cea2a62a1 100644 --- a/src/pika_repl_bgworker.cc +++ b/src/pika_repl_bgworker.cc @@ -15,8 +15,8 @@ extern std::unique_ptr g_pika_conf; extern PikaServer* g_pika_server; -extern PikaReplicaManager* g_pika_rm; -extern PikaCmdTableManager* g_pika_cmd_table_manager; +extern std::unique_ptr g_pika_rm; +extern std::unique_ptr g_pika_cmd_table_manager; PikaReplBgWorker::PikaReplBgWorker(int queue_size) : bg_thread_(queue_size) { bg_thread_.set_thread_name("ReplBgWorker"); diff --git a/src/pika_repl_client.cc b/src/pika_repl_client.cc index 06954d394..1e7068628 100644 --- a/src/pika_repl_client.cc +++ b/src/pika_repl_client.cc @@ -19,22 +19,18 @@ #include "include/pika_server.h" extern PikaServer* g_pika_server; -extern PikaReplicaManager* g_pika_rm; +extern std::unique_ptr g_pika_rm; PikaReplClient::PikaReplClient(int cron_interval, int keepalive_timeout) : next_avail_(0) { - client_thread_ = new PikaReplClientThread(cron_interval, keepalive_timeout); + client_thread_ = std::make_unique(cron_interval, keepalive_timeout); client_thread_->set_thread_name("PikaReplClient"); for (int i = 0; i < 2 * g_pika_conf->sync_thread_num(); ++i) { - bg_workers_.push_back(new PikaReplBgWorker(PIKA_SYNC_BUFFER_SIZE)); + bg_workers_.push_back(std::make_unique(PIKA_SYNC_BUFFER_SIZE)); } } PikaReplClient::~PikaReplClient() { client_thread_->StopThread(); - delete client_thread_; - for (size_t i = 0; i < bg_workers_.size(); ++i) { - delete bg_workers_[i]; - } LOG(INFO) << "PikaReplClient exit!!!"; } @@ -72,7 +68,7 @@ void PikaReplClient::ScheduleWriteBinlogTask(std::string table_partition, std::shared_ptr conn, void* res_private_data) { size_t index = GetHashIndex(table_partition, true); ReplClientWriteBinlogTaskArg* task_arg = - new ReplClientWriteBinlogTaskArg(res, conn, res_private_data, bg_workers_[index]); + new ReplClientWriteBinlogTaskArg(res, conn, res_private_data, bg_workers_[index].get()); bg_workers_[index]->Schedule(&PikaReplBgWorker::HandleBGWorkerWriteBinlog, static_cast(task_arg)); } @@ -98,7 +94,7 @@ Status PikaReplClient::Close(const std::string& ip, const int port) { return cli Status PikaReplClient::SendMetaSync() { std::string local_ip; - net::NetCli* cli = net::NewRedisCli(); + std::unique_ptr cli (net::NewRedisCli()); cli->set_connect_timeout(1500); if ((cli->Connect(g_pika_server->master_ip(), g_pika_server->master_port(), "")).ok()) { struct sockaddr_in laddr; @@ -107,7 +103,6 @@ Status PikaReplClient::SendMetaSync() { std::string tmp_local_ip(inet_ntoa(laddr.sin_addr)); local_ip = tmp_local_ip; cli->Close(); - delete cli; } else { LOG(WARNING) << "Failed to connect master, Master (" << g_pika_server->master_ip() << ":" << g_pika_server->master_port() << "), try reconnect"; @@ -115,7 +110,6 @@ Status PikaReplClient::SendMetaSync() { // when the connection fails sleep(3); g_pika_server->ResetMetaSyncStatus(); - delete cli; return Status::Corruption("Connect master error"); } diff --git a/src/pika_repl_client_conn.cc b/src/pika_repl_client_conn.cc index 96debd352..007772a3a 100644 --- a/src/pika_repl_client_conn.cc +++ b/src/pika_repl_client_conn.cc @@ -20,7 +20,7 @@ extern std::unique_ptr g_pika_conf; extern PikaServer* g_pika_server; -extern PikaReplicaManager* g_pika_rm; +extern std::unique_ptr g_pika_rm; PikaReplClientConn::PikaReplClientConn(int fd, const std::string& ip_port, net::Thread* thread, void* worker_specific_data, net::NetMultiplexer* mpx) diff --git a/src/pika_repl_client_thread.cc b/src/pika_repl_client_thread.cc index 165e3254a..9c29d50fb 100644 --- a/src/pika_repl_client_thread.cc +++ b/src/pika_repl_client_thread.cc @@ -11,7 +11,7 @@ #include "pstd/include/pstd_string.h" extern PikaServer* g_pika_server; -extern PikaReplicaManager* g_pika_rm; +extern std::unique_ptr g_pika_rm; PikaReplClientThread::PikaReplClientThread(int cron_interval, int keepalive_timeout) : ClientThread(&conn_factory_, cron_interval, keepalive_timeout, &handle_, nullptr) {} diff --git a/src/pika_repl_server.cc b/src/pika_repl_server.cc index db3b12e36..205863492 100644 --- a/src/pika_repl_server.cc +++ b/src/pika_repl_server.cc @@ -13,17 +13,15 @@ extern std::unique_ptr g_pika_conf; extern PikaServer* g_pika_server; -extern PikaReplicaManager* g_pika_rm; +extern std::unique_ptr g_pika_rm; PikaReplServer::PikaReplServer(const std::set& ips, int port, int cron_interval) { - server_tp_ = new net::ThreadPool(PIKA_REPL_SERVER_TP_SIZE, 100000); - pika_repl_server_thread_ = new PikaReplServerThread(ips, port, cron_interval); + server_tp_ = std::make_unique(PIKA_REPL_SERVER_TP_SIZE, 100000); + pika_repl_server_thread_ = std::make_unique(ips, port, cron_interval); pika_repl_server_thread_->set_thread_name("PikaReplServer"); } PikaReplServer::~PikaReplServer() { - delete pika_repl_server_thread_; - delete server_tp_; LOG(INFO) << "PikaReplServer exit!!!"; } diff --git a/src/pika_repl_server_conn.cc b/src/pika_repl_server_conn.cc index f7b57b841..85765de00 100644 --- a/src/pika_repl_server_conn.cc +++ b/src/pika_repl_server_conn.cc @@ -11,7 +11,7 @@ #include "include/pika_server.h" extern PikaServer* g_pika_server; -extern PikaReplicaManager* g_pika_rm; +extern std::unique_ptr g_pika_rm; PikaReplServerConn::PikaReplServerConn(int fd, std::string ip_port, net::Thread* thread, void* worker_specific_data, net::NetMultiplexer* mpx) diff --git a/src/pika_repl_server_thread.cc b/src/pika_repl_server_thread.cc index bc636d8e7..bffe6d9bf 100644 --- a/src/pika_repl_server_thread.cc +++ b/src/pika_repl_server_thread.cc @@ -9,7 +9,7 @@ #include "include/pika_server.h" extern PikaServer* g_pika_server; -extern PikaReplicaManager* g_pika_rm; +extern std::unique_ptr g_pika_rm; PikaReplServerThread::PikaReplServerThread(const std::set& ips, int port, int cron_interval) : HolyThread(ips, port, &conn_factory_, cron_interval, &handle_, true), diff --git a/src/pika_rm.cc b/src/pika_rm.cc index 53804a19a..318ecec3f 100644 --- a/src/pika_rm.cc +++ b/src/pika_rm.cc @@ -19,7 +19,7 @@ #include "include/pika_command.h" extern std::unique_ptr g_pika_conf; -extern PikaReplicaManager* g_pika_rm; +extern std::unique_ptr g_pika_rm; extern PikaServer* g_pika_server; /* SyncPartition */ @@ -643,16 +643,11 @@ PikaReplicaManager::PikaReplicaManager() { std::set ips; ips.insert("0.0.0.0"); int port = g_pika_conf->port() + kPortShiftReplServer; - pika_repl_client_ = new PikaReplClient(3000, 60); - pika_repl_server_ = new PikaReplServer(ips, port, 3000); + pika_repl_client_ = std::make_unique(3000, 60); + pika_repl_server_ = std::make_unique(ips, port, 3000); InitPartition(); } -PikaReplicaManager::~PikaReplicaManager() { - delete pika_repl_client_; - delete pika_repl_server_; -} - void PikaReplicaManager::Start() { int ret = 0; ret = pika_repl_client_->Start(); @@ -950,7 +945,7 @@ Status PikaReplicaManager::GetPartitionInfo(const std::string& table, uint32_t p Status PikaReplicaManager::SelectLocalIp(const std::string& remote_ip, const int remote_port, std::string* const local_ip) { - net::NetCli* cli = net::NewRedisCli(); + std::unique_ptr cli(net::NewRedisCli()); cli->set_connect_timeout(1500); if ((cli->Connect(remote_ip, remote_port, "")).ok()) { struct sockaddr_in laddr; @@ -959,10 +954,8 @@ Status PikaReplicaManager::SelectLocalIp(const std::string& remote_ip, const int std::string tmp_ip(inet_ntoa(laddr.sin_addr)); *local_ip = tmp_ip; cli->Close(); - delete cli; } else { LOG(WARNING) << "Failed to connect remote node(" << remote_ip << ":" << remote_port << ")"; - delete cli; return Status::Corruption("connect remote node error"); } return Status::OK(); diff --git a/src/pika_server.cc b/src/pika_server.cc index 31055dcee..4c7aa0a0e 100644 --- a/src/pika_server.cc +++ b/src/pika_server.cc @@ -26,8 +26,8 @@ #include "include/pika_server.h" extern PikaServer* g_pika_server; -extern PikaReplicaManager* g_pika_rm; -extern PikaCmdTableManager* g_pika_cmd_table_manager; +extern std::unique_ptr g_pika_rm; +extern std::unique_ptr g_pika_cmd_table_manager; void DoPurgeDir(void* arg) { std::unique_ptr path(static_cast(arg)); @@ -78,20 +78,19 @@ PikaServer::PikaServer() int worker_queue_limit = g_pika_conf->maxclients() / worker_num_ + 100; LOG(INFO) << "Worker queue limit is " << worker_queue_limit; pika_dispatch_thread_ = - new PikaDispatchThread(ips, port_, worker_num_, 3000, worker_queue_limit, g_pika_conf->max_conn_rbuf_size()); - pika_monitor_thread_ = new PikaMonitorThread(); - pika_rsync_service_ = new PikaRsyncService(g_pika_conf->db_sync_path(), g_pika_conf->port() + kPortShiftRSync); - pika_pubsub_thread_ = new net::PubSubThread(); - pika_auxiliary_thread_ = new PikaAuxiliaryThread(); + std::make_unique(ips, port_, worker_num_, 3000, worker_queue_limit, g_pika_conf->max_conn_rbuf_size()); + pika_monitor_thread_ = std::make_unique(); + pika_rsync_service_ = std::make_unique(g_pika_conf->db_sync_path(), g_pika_conf->port() + kPortShiftRSync); + pika_pubsub_thread_ = std::make_unique(); + pika_auxiliary_thread_ = std::make_unique(); - pika_client_processor_ = new PikaClientProcessor(g_pika_conf->thread_pool_size(), 100000); + pika_client_processor_ = std::make_unique(g_pika_conf->thread_pool_size(), 100000); } PikaServer::~PikaServer() { // DispatchThread will use queue of worker thread, // so we need to delete dispatch before worker. pika_client_processor_->Stop(); - delete pika_dispatch_thread_; { std::lock_guard l(slave_mutex_); @@ -101,13 +100,6 @@ PikaServer::~PikaServer() { LOG(INFO) << "Delete slave success"; } } - - delete pika_pubsub_thread_; - delete pika_auxiliary_thread_; - delete pika_rsync_service_; - delete pika_client_processor_; - delete pika_monitor_thread_; - bgsave_thread_.StopThread(); key_scan_thread_.StopThread(); @@ -1028,7 +1020,7 @@ void PikaServer::DbSyncSendFile(const std::string& ip, int port, const std::stri pstd::RsyncSendClearTarget(bg_path + "/sets", remote_path + "/sets", secret_file_path, remote); pstd::RsyncSendClearTarget(bg_path + "/zsets", remote_path + "/zsets", secret_file_path, remote); - net::NetCli* cli = net::NewRedisCli(); + std::unique_ptr cli(net::NewRedisCli()); std::string lip(host_); if (cli->Connect(ip, port, "").ok()) { struct sockaddr_in laddr; @@ -1036,11 +1028,9 @@ void PikaServer::DbSyncSendFile(const std::string& ip, int port, const std::stri getsockname(cli->fd(), (struct sockaddr*)&laddr, &llen); lip = inet_ntoa(laddr.sin_addr); cli->Close(); - delete cli; } else { LOG(WARNING) << "Rsync try connect slave rsync service error" << ", slave rsync service(" << ip << ":" << port << ")"; - delete cli; } // Send info file at last diff --git a/src/pika_stable_log.cc b/src/pika_stable_log.cc index bedcc4052..9476813d4 100644 --- a/src/pika_stable_log.cc +++ b/src/pika_stable_log.cc @@ -15,7 +15,7 @@ extern std::unique_ptr g_pika_conf; extern PikaServer* g_pika_server; -extern PikaReplicaManager* g_pika_rm; +extern std::unique_ptr g_pika_rm; StableLog::StableLog(const std::string table_name, uint32_t partition_id, const std::string& log_path) : purging_(false), table_name_(table_name), partition_id_(partition_id), log_path_(log_path) { diff --git a/src/pika_table.cc b/src/pika_table.cc index 90d66e307..519b33152 100644 --- a/src/pika_table.cc +++ b/src/pika_table.cc @@ -10,8 +10,8 @@ #include "include/pika_server.h" extern PikaServer* g_pika_server; -extern PikaReplicaManager* g_pika_rm; -extern PikaCmdTableManager* g_pika_cmd_table_manager; +extern std::unique_ptr g_pika_rm; +extern std::unique_ptr g_pika_cmd_table_manager; std::string TablePath(const std::string& path, const std::string& table_name) { char buf[100]; diff --git a/src/storage/src/db_checkpoint.cc b/src/storage/src/db_checkpoint.cc index f572343bf..f8990469b 100644 --- a/src/storage/src/db_checkpoint.cc +++ b/src/storage/src/db_checkpoint.cc @@ -17,6 +17,7 @@ # include +#include # include "file/file_util.h" # include "rocksdb/db.h" // #include "file/filename.h" @@ -183,6 +184,7 @@ Status DBCheckpointImpl::CreateCheckpointWithFiles(const std::string& checkpoint s = CopyFile(db_->GetEnv(), db_->GetOptions().wal_dir + live_wal_files[i]->PathName(), full_private_path + live_wal_files[i]->PathName(), live_wal_files[i]->SizeFileBytes()); # else + LOG(INFO) << "come here : " << db_->GetOptions().wal_dir <<" "<< live_wal_files[i]->PathName() << full_private_path; s = CopyFile(db_->GetFileSystem(), db_->GetOptions().wal_dir + live_wal_files[i]->PathName(), full_private_path + live_wal_files[i]->PathName(), live_wal_files[i]->SizeFileBytes(), false, nullptr, Temperature::kUnknown); From 8fc489262c1154ef79d3bd7082aa7eff857798ed Mon Sep 17 00:00:00 2001 From: laimuxi <1721261216@qq.com> Date: Sat, 20 May 2023 18:44:05 +0800 Subject: [PATCH 16/17] fix --- include/pika_binlog.h | 1 + include/pika_partition.h | 1 + src/storage/include/storage/storage.h | 10 +++++----- src/storage/src/storage.cc | 16 ++++++++-------- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/include/pika_binlog.h b/include/pika_binlog.h index e3d3a2211..ecbc3d4ce 100644 --- a/include/pika_binlog.h +++ b/include/pika_binlog.h @@ -42,6 +42,7 @@ class Version { } private: + // shared with versionfile_ std::shared_ptr save_; // No copying allowed; diff --git a/include/pika_partition.h b/include/pika_partition.h index 998b67555..a01ba00c7 100644 --- a/include/pika_partition.h +++ b/include/pika_partition.h @@ -97,6 +97,7 @@ class Partition : public std::enable_shared_from_this { bool opened_ = false; std::shared_mutex db_rwlock_; + // class may be shared, using shared_ptr would be a better choice std::shared_ptr lock_mgr_; std::shared_ptr db_; diff --git a/src/storage/include/storage/storage.h b/src/storage/include/storage/storage.h index c213e9464..fceebb854 100644 --- a/src/storage/include/storage/storage.h +++ b/src/storage/include/storage/storage.h @@ -1020,11 +1020,11 @@ class Storage { const std::unordered_map& options); private: - std::shared_ptr strings_db_; - std::shared_ptr hashes_db_; - std::shared_ptr sets_db_; - std::shared_ptr zsets_db_; - std::shared_ptr lists_db_; + std::unique_ptr strings_db_; + std::unique_ptr hashes_db_; + std::unique_ptr sets_db_; + std::unique_ptr zsets_db_; + std::unique_ptr lists_db_; std::atomic is_opened_; std::unique_ptr> cursors_store_; diff --git a/src/storage/src/storage.cc b/src/storage/src/storage.cc index e43fd7bd4..93695da39 100644 --- a/src/storage/src/storage.cc +++ b/src/storage/src/storage.cc @@ -94,31 +94,31 @@ static std::string AppendSubDirectory(const std::string& db_path, const std::str Status Storage::Open(const StorageOptions& storage_options, const std::string& db_path) { mkpath(db_path.c_str(), 0755); - strings_db_ = std::make_shared(this, kStrings); + strings_db_ = std::make_unique(this, kStrings); Status s = strings_db_->Open(storage_options, AppendSubDirectory(db_path, "strings")); if (!s.ok()) { LOG(FATAL) << "open kv db failed, " << s.ToString(); } - hashes_db_ = std::make_shared(this, kHashes); + hashes_db_ = std::make_unique(this, kHashes); s = hashes_db_->Open(storage_options, AppendSubDirectory(db_path, "hashes")); if (!s.ok()) { LOG(FATAL) << "open hashes db failed, " << s.ToString(); } - sets_db_ = std::make_shared(this, kSets); + sets_db_ = std::make_unique(this, kSets); s = sets_db_->Open(storage_options, AppendSubDirectory(db_path, "sets")); if (!s.ok()) { LOG(FATAL) << "open set db failed, " << s.ToString(); } - lists_db_ = std::make_shared(this, kLists); + lists_db_ = std::make_unique(this, kLists); s = lists_db_->Open(storage_options, AppendSubDirectory(db_path, "lists")); if (!s.ok()) { LOG(FATAL) << "open list db failed, " << s.ToString(); } - zsets_db_ = std::make_shared(this, kZSets); + zsets_db_ = std::make_unique(this, kZSets); s = zsets_db_->Open(storage_options, AppendSubDirectory(db_path, "zsets")); if (!s.ok()) { LOG(FATAL) << "open zset db failed, " << s.ToString(); @@ -1564,7 +1564,7 @@ Status Storage::CompactKey(const DataType& type, const std::string& key) { } Status Storage::SetMaxCacheStatisticKeys(uint32_t max_cache_statistic_keys) { - std::vector> dbs = {sets_db_, zsets_db_, hashes_db_, lists_db_}; + std::vector dbs = {sets_db_.get(), zsets_db_.get(), hashes_db_.get(), lists_db_.get()}; for (const auto& db : dbs) { db->SetMaxCacheStatisticKeys(max_cache_statistic_keys); } @@ -1572,7 +1572,7 @@ Status Storage::SetMaxCacheStatisticKeys(uint32_t max_cache_statistic_keys) { } Status Storage::SetSmallCompactionThreshold(uint32_t small_compaction_threshold) { - std::vector> dbs = {sets_db_, zsets_db_, hashes_db_, lists_db_}; + std::vector dbs = {sets_db_.get(), zsets_db_.get(), hashes_db_.get(), lists_db_.get()}; for (const auto& db : dbs) { db->SetSmallCompactionThreshold(small_compaction_threshold); } @@ -1643,7 +1643,7 @@ uint64_t Storage::GetProperty(const std::string& db_type, const std::string& pro Status Storage::GetKeyNum(std::vector* key_infos) { KeyInfo key_info; // NOTE: keep the db order with string, hash, list, zset, set - std::vector> dbs = {strings_db_, hashes_db_, lists_db_, zsets_db_, sets_db_}; + std::vector dbs = {strings_db_.get(), hashes_db_.get(), lists_db_.get(), zsets_db_.get(), sets_db_.get()}; for (const auto& db : dbs) { // check the scanner was stopped or not, before scanning the next db if (scan_keynum_exit_) { From 74dea31dda9013451c1e2774ee4d88f37764463f Mon Sep 17 00:00:00 2001 From: laimuxi <1721261216@qq.com> Date: Sat, 20 May 2023 18:52:40 +0800 Subject: [PATCH 17/17] fix --- include/pika_partition.h | 2 +- src/pika_partition.cc | 2 +- src/storage/include/storage/backupable.h | 2 +- src/storage/src/backupable.cc | 13 ++++++------- 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/include/pika_partition.h b/include/pika_partition.h index a01ba00c7..f3896f0f8 100644 --- a/include/pika_partition.h +++ b/include/pika_partition.h @@ -117,7 +117,7 @@ class Partition : public std::enable_shared_from_this { void FinishBgsave(); BgSaveInfo bgsave_info_; pstd::Mutex bgsave_protector_; - std::unique_ptr bgsave_engine_; + std::shared_ptr bgsave_engine_; // key scan info use void InitKeyScan(); diff --git a/src/pika_partition.cc b/src/pika_partition.cc index a679f7edb..bfe8ef78f 100644 --- a/src/pika_partition.cc +++ b/src/pika_partition.cc @@ -364,7 +364,7 @@ bool Partition::InitBgsaveEnv() { // Prepare bgsave env, need bgsave_protector protect bool Partition::InitBgsaveEngine() { - bgsave_engine_ = nullptr; + bgsave_engine_.reset(); rocksdb::Status s = storage::BackupEngine::Open(db().get(), bgsave_engine_); if (!s.ok()) { LOG(WARNING) << partition_name_ << " open backup engine failed " << s.ToString(); diff --git a/src/storage/include/storage/backupable.h b/src/storage/include/storage/backupable.h index 3ada28b5a..1de1dce89 100644 --- a/src/storage/include/storage/backupable.h +++ b/src/storage/include/storage/backupable.h @@ -41,7 +41,7 @@ struct BackupContent { class BackupEngine { public: ~BackupEngine(); - static Status Open(Storage* db, std::unique_ptr& backup_engine_ret); + static Status Open(Storage* db, std::shared_ptr& backup_engine_ret); Status SetBackupContent(); diff --git a/src/storage/src/backupable.cc b/src/storage/src/backupable.cc index 5f4ab5fdb..9a1c92ba4 100644 --- a/src/storage/src/backupable.cc +++ b/src/storage/src/backupable.cc @@ -26,10 +26,10 @@ Status BackupEngine::NewCheckpoint(rocksdb::DB* rocksdb_db, const std::string& t return s; } -Status BackupEngine::Open(storage::Storage* storage, std::unique_ptr& backup_engine_ret) { - // BackupEngine() is private, can't use make_unique - auto backup_engine_ptr = std::unique_ptr(new BackupEngine()); - if (!backup_engine_ptr) { +Status BackupEngine::Open(storage::Storage* storage, std::shared_ptr& backup_engine_ret) { + // BackupEngine() is private, can't use make_shared + backup_engine_ret = std::shared_ptr(new BackupEngine()); + if (!backup_engine_ret) { return Status::Corruption("New BackupEngine failed!"); } @@ -43,15 +43,14 @@ Status BackupEngine::Open(storage::Storage* storage, std::unique_ptrNewCheckpoint(rocksdb_db, type); + s = backup_engine_ret->NewCheckpoint(rocksdb_db, type); } if (!s.ok()) { - backup_engine_ptr = nullptr; + backup_engine_ret = nullptr; break; } } - backup_engine_ret = std::move(backup_engine_ptr); return s; }