fix: change irrewritable attributes in config from atomic to primitive #352
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,16 +33,16 @@ extern PConfig g_config; | |
|
|
||
| class BaseValue { | ||
| public: | ||
| BaseValue(std::string key, CheckFunc check_func_ptr, bool rewritable = false) | ||
| : key_(std::move(key)), custom_check_func_ptr_(std::move(check_func_ptr)), rewritable_(rewritable) {} | ||
|
|
||
| virtual ~BaseValue() = default; | ||
|
|
||
| const std::string& Key() const { return key_; } | ||
|
|
||
| virtual std::string Value() const = 0; | ||
|
|
||
| Status Set(const std::string& value, bool init_stage); | ||
|
|
||
| protected: | ||
| virtual Status SetValue(const std::string&) = 0; | ||
|
|
@@ -62,8 +62,10 @@ class BaseValue { | |
| class StringValue : public BaseValue { | ||
| public: | ||
| StringValue(const std::string& key, CheckFunc check_func_ptr, bool rewritable, | ||
| std::vector<std::string*> value_ptr_vec, char delimiter = ' ') | ||
| : BaseValue(key, std::move(check_func_ptr), rewritable), | ||
| values_(std::move(value_ptr_vec)), | ||
| delimiter_(delimiter) { | ||
| assert(!values_.empty()); | ||
| } | ||
| ~StringValue() override = default; | ||
|
|
@@ -73,15 +75,69 @@ class StringValue : public BaseValue { | |
| private: | ||
| Status SetValue(const std::string& value) override; | ||
|
|
||
| std::vector<std::string*> values_; | ||
| char delimiter_ = 0; | ||
| }; | ||
|
|
||
| template <typename T> | ||
| class NumberValue : public BaseValue { | ||
| public: | ||
| NumberValue(const std::string& key, CheckFunc check_func_ptr, bool rewritable, T* value, | ||
| T min = std::numeric_limits<T>::min(), T max = std::numeric_limits<T>::max()) | ||
| : BaseValue(key, check_func_ptr, rewritable), value_(value), value_min_(min), value_max_(max) { | ||
| assert(value_ != nullptr); | ||
| assert(value_min_ <= value_max_); | ||
| }; | ||
|
|
||
| std::string Value() const override { return std::to_string(*value_); } | ||
|
|
||
| private: | ||
| Status SetValue(const std::string& value) override; | ||
|
|
||
| T* value_; | ||
| T value_min_; | ||
| T value_max_; | ||
| }; | ||
|
|
||
| class BoolValue : public BaseValue { | ||
| public: | ||
| BoolValue(const std::string& key, CheckFunc check_func_ptr, bool rewritable, bool* value) | ||
| : BaseValue(key, std::move(check_func_ptr), rewritable), value_(value) { | ||
| assert(value_ != nullptr); | ||
| }; | ||
|
|
||
| std::string Value() const override { return *value_ ? "yes" : "no"; }; | ||
|
|
||
| private: | ||
| Status SetValue(const std::string& value) override; | ||
| bool* value_ = nullptr; | ||
| }; | ||
|
|
||
| class AtomicStringValue : public BaseValue { | ||
| public: | ||
| AtomicStringValue(const std::string& key, CheckFunc check_func_ptr, bool rewritable, | ||
| std::vector<AtomicString*> value_ptr_vec, char delimiter = ' ') | ||
| : BaseValue(key, std::move(check_func_ptr), rewritable), | ||
| values_(std::move(value_ptr_vec)), | ||
| delimiter_(delimiter) { | ||
| assert(!values_.empty()); | ||
| } | ||
| ~AtomicStringValue() override = default; | ||
|
|
||
| std::string Value() const override { return MergeString(values_, delimiter_); }; | ||
|
|
||
| private: | ||
| Status SetValue(const std::string& value) override; | ||
|
|
||
| std::vector<AtomicString*> values_; | ||
| char delimiter_ = 0; | ||
| }; | ||
|
|
||
| template <typename T> | ||
| class AtomicNumberValue : public BaseValue { | ||
| public: | ||
| AtomicNumberValue(const std::string& key, CheckFunc check_func_ptr, bool rewritable, std::atomic<T>* value_ptr, | ||
| T min = std::numeric_limits<T>::min(), T max = std::numeric_limits<T>::max()) | ||
| : BaseValue(key, check_func_ptr, rewritable), value_(value_ptr), value_min_(min), value_max_(max) { | ||
| assert(value_ != nullptr); | ||
| assert(value_min_ <= value_max_); | ||
|
|
@@ -97,10 +153,10 @@ class NumberValue : public BaseValue { | |
| T value_max_; | ||
| }; | ||
|
|
||
| class AtomicBoolValue : public BaseValue { | ||
| public: | ||
| AtomicBoolValue(const std::string& key, CheckFunc check_func_ptr, bool rewritable, std::atomic<bool>* value_ptr) | ||
| : BaseValue(key, std::move(check_func_ptr), rewritable), value_(value_ptr) { | ||
| assert(value_ != nullptr); | ||
| }; | ||
|
|
||
|
|
@@ -121,7 +177,7 @@ class PConfig { | |
| bool LoadFromFile(const std::string& file_name); | ||
| const std::string& ConfigFileName() const { return config_file_name_; } | ||
| void Get(const std::string&, std::vector<std::string>*) const; | ||
| Status Set(std::string, const std::string&, bool init_stage = false); | ||
|
|
||
| public: | ||
| std::atomic_uint32_t timeout = 0; | ||
|
|
@@ -136,41 +192,41 @@ class PConfig { | |
| std::atomic_uint32_t master_port; // replication | ||
| AtomicString include_file; // the template config | ||
| std::vector<PString> modules; // modules | ||
| int32_t fast_cmd_threads_num = 4; | ||
| int32_t slow_cmd_threads_num = 4; | ||
| std::atomic_uint64_t max_client_response_size = 1073741824; | ||
| std::atomic_uint64_t small_compaction_threshold = 604800; | ||
| std::atomic_uint64_t small_compaction_duration_threshold = 259200; | ||
|
|
||
| bool daemonize = false; | ||
| AtomicString pid_file = "./pikiwidb.pid"; | ||
| std::string ip = "127.0.0.1"; | ||
| uint16_t port = 9221; | ||
| std::atomic_uint16_t raft_port_offset = 10; | ||
| std::string db_path = "./db/"; | ||
| std::string log_dir = "stdout"; // the log directory, differ from redis | ||
| std::string log_level = "warning"; | ||
| AtomicString run_id; | ||
| size_t databases = 16; | ||
| uint32_t worker_threads_num = 2; | ||
| std::atomic_uint32_t slave_threads_num = 2; | ||
| std::atomic<size_t> db_instance_num = 3; | ||
|
There was a problem hiding this comment. 这两个是动态可调整嘛？ 我在issue里看到说db_instance_num也是只读，这个需要考虑改成普通变量吗 There was a problem hiding this comment. 是的，加载之后不会修改，slave_threads_num 也是 There was a problem hiding this comment. 像上面的 master_ip 逻辑上应该也不会在加载之后修改，可以对上面的 config 类中的变量逐个考虑一下是否是只读属性 |
||
| bool use_raft = true; | ||
|
|
||
| uint32_t rocksdb_max_subcompactions = 0; | ||
| // default 2 | ||
| int rocksdb_max_background_jobs = 4; | ||
| // default 2 | ||
| size_t rocksdb_max_write_buffer_number = 2; | ||
| // default 2 | ||
| int rocksdb_min_write_buffer_number_to_merge = 2; | ||
| // default 64M | ||
| size_t rocksdb_write_buffer_size = 64 << 20; | ||
| int rocksdb_level0_file_num_compaction_trigger = 4; | ||
| std::atomic_int rocksdb_num_levels = 7; | ||
| bool rocksdb_enable_pipelined_write = false; | ||
| int rocksdb_level0_slowdown_writes_trigger = 20; | ||
| int rocksdb_level0_stop_writes_trigger = 36; | ||
| std::atomic_uint64_t rocksdb_ttl_second = 604800; // default 86400 * 7 | ||
| std::atomic_uint64_t rocksdb_periodic_second = 259200; // default 86400 * 3 | ||
|
|
||
|
|
@@ -179,26 +235,48 @@ class PConfig { | |
| rocksdb::BlockBasedTableOptions GetRocksDBBlockBasedTableOptions(); | ||
|
|
||
| private: | ||
| inline void AddString(const std::string& key, bool rewritable, const std::vector<std::string*>& values_ptr_vector) { | ||
| config_map_.emplace(key, std::make_unique<StringValue>(key, nullptr, rewritable, values_ptr_vector)); | ||
| } | ||
| inline void AddStrinWithFunc(const std::string& key, const CheckFunc& checkfunc, bool rewritable, | ||
| const std::vector<std::string*>& values_ptr_vector) { | ||
|
Comment on lines
+238
to
+242
There was a problem hiding this comment. Tip Codebase Verification
请在这些方法中添加键存在性检查，以避免重复添加相同的键。 Analysis chain添加了 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# 检查AddString和AddStrinWithFunc方法的调用情况，确保没有重复添加相同的键。
rg --type cpp "AddString" -A 3
Length of output: 1121 Script: #!/bin/bash
# Check if config_map_ is checked for existing keys before adding new entries in AddString and AddStrinWithFunc methods.
rg --type cpp "config_map_" -A 5
Length of output: 4153 There was a problem hiding this comment. 这些方法只会在 There was a problem hiding this comment. @mirthfulLee, thank you for clarifying the usage context of the Analysis chainScripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Search for the usage of `AddString` and `AddStrinWithFunc` methods within the `PConfig::PConfig()` constructor to verify the user's claim.
ast-grep --lang cpp --pattern $'class PConfig {
PConfig() {
$$$
}
}'
Length of output: 77 Learnings added
|
||
| config_map_.emplace(key, std::make_unique<StringValue>(key, checkfunc, rewritable, values_ptr_vector)); | ||
| } | ||
| inline void AddBool(const std::string& key, const CheckFunc& checkfunc, bool rewritable, bool* value_ptr) { | ||
| config_map_.emplace(key, std::make_unique<BoolValue>(key, checkfunc, rewritable, value_ptr)); | ||
| } | ||
| template <typename T> | ||
| inline void AddNumber(const std::string& key, bool rewritable, T* value_ptr) { | ||
| config_map_.emplace(key, std::make_unique<NumberValue<T>>(key, nullptr, rewritable, value_ptr)); | ||
| } | ||
| template <typename T> | ||
| inline void AddNumberWihLimit(const std::string& key, bool rewritable, T* value_ptr, T min, T max) { | ||
| config_map_.emplace(key, std::make_unique<NumberValue<T>>(key, nullptr, rewritable, value_ptr, min, max)); | ||
| } | ||
|
|
||
| // inline functions for add atomic Values | ||
| inline void AddAtomicString(const std::string& key, bool rewritable, | ||
| const std::vector<AtomicString*>& values_ptr_vector) { | ||
| config_map_.emplace(key, std::make_unique<AtomicStringValue>(key, nullptr, rewritable, values_ptr_vector)); | ||
| } | ||
| inline void AddAtomicStrinWithFunc(const std::string& key, const CheckFunc& checkfunc, bool rewritable, | ||
| const std::vector<AtomicString*>& values_ptr_vector) { | ||
| config_map_.emplace(key, std::make_unique<AtomicStringValue>(key, checkfunc, rewritable, values_ptr_vector)); | ||
| } | ||
| inline void AddAtomicBool(const std::string& key, const CheckFunc& checkfunc, bool rewritable, | ||
| std::atomic<bool>* value_ptr) { | ||
| config_map_.emplace(key, std::make_unique<AtomicBoolValue>(key, checkfunc, rewritable, value_ptr)); | ||
| } | ||
| template <typename T> | ||
| inline void AddAtomicNumber(const std::string& key, bool rewritable, std::atomic<T>* value_ptr) { | ||
| config_map_.emplace(key, std::make_unique<AtomicNumberValue<T>>(key, nullptr, rewritable, value_ptr)); | ||
| } | ||
| template <typename T> | ||
| inline void AddAtomicNumberWihLimit(const std::string& key, bool rewritable, std::atomic<T>* value_ptr, T min, | ||
| T max) { | ||
| config_map_.emplace(key, std::make_unique<AtomicNumberValue<T>>(key, nullptr, rewritable, value_ptr, min, max)); | ||
| } | ||
|
|
||
| private: | ||
| ConfigParser parser_; | ||
| ConfigMap config_map_; | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
在value_还是std::atomic*类型的情况下，还是使用value_->store(true);和value_->store(false);比较好