Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

loading pikiwidb.conf overuses atomic variables #329

Open
panlei-coder opened this issue May 25, 2024 · 4 comments
Open

loading pikiwidb.conf overuses atomic variables #329

panlei-coder opened this issue May 25, 2024 · 4 comments
Assignees
Labels
☢️ Bug Something isn't working

Comments

@panlei-coder
Copy link
Collaborator

panlei-coder commented May 25, 2024

Description

`class PConfig {
public:
PConfig();
~PConfig() = default;
bool LoadFromFile(const std::string& file_name);
const std::string& ConfigFileName() const { return config_file_name_; }
void Get(const std::string&, std::vectorstd::string*) const;
Status Set(std::string, const std::string&, bool force = false);

public:
std::atomic_uint32_t timeout = 0;
// auth
AtomicString password;
AtomicString master_auth;
AtomicString master_ip;
std::map<std::string, std::string> aliases;
std::atomic_uint32_t max_clients = 10000; // 10000
std::atomic_uint32_t slow_log_time = 1000; // 1000 microseconds
std::atomic_uint32_t slow_log_max_len = 128; // 128
std::atomic_uint32_t master_port; // replication
AtomicString include_file; // the template config
std::vector modules; // modules
std::atomic_int32_t fast_cmd_threads_num = 4;
std::atomic_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;

std::atomic_bool daemonize = false;
AtomicString pid_file = "./pikiwidb.pid";
AtomicString ip = "127.0.0.1";
std::atomic_uint16_t port = 9221;
std::atomic_uint16_t raft_port_offset = 10;
AtomicString db_path = "./db/";
AtomicString log_dir = "stdout"; // the log directory, differ from redis
AtomicString log_level = "warning";
AtomicString run_id;
std::atomic<size_t> databases = 16;
std::atomic_uint32_t worker_threads_num = 2;
std::atomic_uint32_t slave_threads_num = 2;
std::atomic<size_t> db_instance_num = 3;
std::atomic_bool use_raft = true;

std::atomic_uint32_t rocksdb_max_subcompactions = 0;
// default 2
std::atomic_int rocksdb_max_background_jobs = 4;
// default 2
std::atomic<size_t> rocksdb_max_write_buffer_number = 2;
// default 2
std::atomic_int rocksdb_min_write_buffer_number_to_merge = 2;
// default 64M
std::atomic<size_t> rocksdb_write_buffer_size = 64 << 20;
std::atomic_int rocksdb_level0_file_num_compaction_trigger = 4;
std::atomic_int rocksdb_num_levels = 7;
std::atomic_bool rocksdb_enable_pipelined_write = false;
std::atomic_int rocksdb_level0_slowdown_writes_trigger = 20;
std::atomic_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
// ...
}`
在 PConfig 类中过度使用了原子变量,像 use_raft、db_instance_num 等变量在加载了配置文件之后,只存在读的操作,不会修改,就没有竞争的问题存在,那么使用原子变量会导致较大的性能问题,需要重新考虑这些变量是否适合使用原子变量。
类似于 Pika 的这个问题:OpenAtomFoundation/pika#2653

@panlei-coder panlei-coder added the ☢️ Bug Something isn't working label May 25, 2024
@luky116
Copy link
Collaborator

luky116 commented Jun 1, 2024

@lihuan

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically.


@suffering

@mirthfulLee
Copy link
Contributor

  1. The second parameter of these AddXXX method is a bool rewritable, which should means whether this field is changable. For those fields that rewritable == false, unwrap its value out of atomic;
PConfig::PConfig() {
  AddBool("daemonize", &CheckYesNo, false, &daemonize);
  AddString("ip", false, {&ip});
.......
  AddNumber("rocksdb-level0-stop-writes-trigger", false, &rocksdb_level0_stop_writes_trigger);
  AddNumber("rocksdb-level0-slowdown-writes-trigger", false, &rocksdb_level0_slowdown_writes_trigger);
}

runid, port and log_level are not rewritable, but they are set in PikiwiDB::Init, which is only called when the pikiwidb start

  1. For rewritable fields, use several lock to protect them. (Actually, if one field can only be rewritten by Set and is independent with other fields, it is also okay to use primitive types without lock protection.)

@mirthfulLee
Copy link
Contributor

Not fully completed yet, needs more time for further analysis and optimization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
☢️ Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants