-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
feat:thread purge #2697
feat:thread purge #2697
Changes from 3 commits
a6ae2be
53c81c0
d6cb38d
526affd
13f1d46
a3f779b
6a1c80d
8bac0c6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -95,6 +95,7 @@ class PikaServer : public pstd::noncopyable { | |
bool force_full_sync(); | ||
void SetForceFullSync(bool v); | ||
void SetDispatchQueueLimit(int queue_limit); | ||
void SetSlowCmdThreadPoolFlag(bool flag); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tip Codebase Verification Ensure thread safety when toggling
Analysis chainEnsure thread safety when toggling Ensure thread safety around Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check for potential race conditions related to `slow_cmd_thread_pool_flag_`.
# Test: Search for occurrences of `slow_cmd_thread_pool_flag_` being accessed.
rg --type cpp --context 5 'slow_cmd_thread_pool_flag_' src/
Length of output: 1088 Script: #!/bin/bash
# Description: Check for thread safety mechanisms around `slow_cmd_thread_pool_flag_` usage.
# Test: Search for mutex locks or atomic operations related to `slow_cmd_thread_pool_flag_`.
rg --type cpp --context 5 'slow_cmd_thread_pool_flag_' src/ | grep -E 'std::mutex|std::lock_guard|std::atomic'
Length of output: 110 |
||
storage::StorageOptions storage_options(); | ||
std::unique_ptr<PikaDispatchThread>& pika_dispatch_thread() { | ||
return pika_dispatch_thread_; | ||
|
@@ -168,7 +169,6 @@ class PikaServer : public pstd::noncopyable { | |
void FinishMetaSync(); | ||
bool MetaSyncDone(); | ||
void ResetMetaSyncStatus(); | ||
void SetLoopDBStateMachine(bool need_loop); | ||
int GetMetaSyncTimestamp(); | ||
void UpdateMetaSyncTimestamp(); | ||
void UpdateMetaSyncTimestampWithoutLock(); | ||
|
@@ -179,7 +179,7 @@ class PikaServer : public pstd::noncopyable { | |
* PikaClientProcessor Process Task | ||
*/ | ||
void ScheduleClientPool(net::TaskFunc func, void* arg, bool is_slow_cmd); | ||
void ScheduleClientBgThreads(net::TaskFunc func, void* arg, const std::string& hash_str); | ||
|
||
// for info debug | ||
size_t ClientProcessorThreadPoolCurQueueSize(); | ||
size_t ClientProcessorThreadPoolMaxQueueSize(); | ||
|
@@ -642,6 +642,11 @@ class PikaServer : public pstd::noncopyable { | |
* acl | ||
*/ | ||
std::unique_ptr<::Acl> acl_ = nullptr; | ||
|
||
/* | ||
* fast and slow thread pools | ||
*/ | ||
bool slow_cmd_thread_pool_flag_; | ||
}; | ||
|
||
#endif |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,7 @@ Thread::~Thread() = default; | |
void* Thread::RunThread(void* arg) { | ||
auto thread = reinterpret_cast<Thread*>(arg); | ||
if (!(thread->thread_name().empty())) { | ||
SetThreadName(pthread_self(), thread->thread_name()); | ||
SetThreadName(pthread_self(), "pika"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1.修改ThreadPoolWorke 命名 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
done There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 这里直接改成pika不合适吧?Thread应该是个基类,之前的那种方式看起来更合理点。 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
这个代码 没有吧 应该已经没有改 |
||
} | ||
thread->ThreadMain(); | ||
return nullptr; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1600,6 +1600,12 @@ void ConfigCmd::ConfigGet(std::string& ret) { | |
EncodeString(&config_body, g_pika_conf->slotmigrate() ? "yes" : "no"); | ||
} | ||
|
||
if (pstd::stringmatch(pattern.data(), "slow-cmd-pool", 1)) { | ||
elements += 2; | ||
EncodeString(&config_body, "slow-cmd-pool"); | ||
EncodeString(&config_body, g_pika_conf->slow_cmd_pool() ? "yes" : "no"); | ||
} | ||
|
||
if (pstd::stringmatch(pattern.data(), "slotmigrate-thread-num", 1)!= 0) { | ||
elements += 2; | ||
EncodeString(&config_body, "slotmigrate-thread-num"); | ||
|
@@ -2129,6 +2135,7 @@ void ConfigCmd::ConfigSet(std::shared_ptr<DB> db) { | |
"requirepass", | ||
"masterauth", | ||
"slotmigrate", | ||
"slow-cmd-pool", | ||
"slotmigrate-thread-num", | ||
"thread-migrate-keys-num", | ||
"userpass", | ||
|
@@ -2288,6 +2295,19 @@ void ConfigCmd::ConfigSet(std::shared_ptr<DB> db) { | |
} | ||
g_pika_conf->SetSlotMigrate(slotmigrate); | ||
res_.AppendStringRaw("+OK\r\n"); | ||
} else if (set_item == "slow_cmd_pool") { | ||
bool SlowCmdPool; | ||
if (value == "yes") { | ||
SlowCmdPool = true; | ||
g_pika_server->SetSlowCmdThreadPoolFlag(SlowCmdPool); | ||
} else if (value == "no") { | ||
SlowCmdPool = false; | ||
} else { | ||
res_.AppendStringRaw( "-ERR Invalid argument \'" + value + "\' for CONFIG SET 'slow-cmd-pool'\r\n"); | ||
return; | ||
} | ||
g_pika_conf->SetSlowCmdPool(SlowCmdPool); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 你schedule判断是否把请求塞给慢队列的时候,会读取g_pika_conf->slow_cmd_pool,setslowcmdthreadpoolflag中会等待慢队列为空然后才会stop线程池,那么这里就应该先改pika conf,保证不再有新的请求塞到队列里,然后你在等待队列都消费完之后返回。 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
done |
||
res_.AppendStringRaw("+OK\r\n"); | ||
} else if (set_item == "slowlog-log-slower-than") { | ||
if ((pstd::string2int(value.data(), value.size(), &ival) == 0) || ival < 0) { | ||
res_.AppendStringRaw("-ERR Invalid argument \'" + value + "\' for CONFIG SET 'slowlog-log-slower-than'\r\n"); | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -43,6 +43,7 @@ void DoPurgeDir(void* arg) { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
PikaServer::PikaServer() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
: exit_(false), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
slow_cmd_thread_pool_flag_(false), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Initialization of Consider initializing - slow_cmd_thread_pool_flag_(false),
+ slow_cmd_thread_pool_flag_(g_pika_conf->slow_cmd_pool()), Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
last_check_compact_time_({0, 0}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
last_check_resume_time_({0, 0}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
repl_state_(PIKA_REPL_NO_CONNECT), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -100,6 +101,7 @@ PikaServer::PikaServer() | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
acl_ = std::make_unique<::Acl>(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
SetSlowCmdThreadPoolFlag(g_pika_conf->slow_cmd_pool()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tip Codebase Verification The search results indicate that
There is no evidence of validation logic for
Analysis chainEnsure configuration is validated before use. Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify that the configuration for `slow_cmd_pool` is validated before use.
# Test: Search for the validation logic. Expect: Validation logic before this usage.
rg --type python --context 5 $'g_pika_conf->slow_cmd_pool()'
Length of output: 92 Script: #!/bin/bash
# Description: Verify that the configuration for `slow_cmd_pool` is validated before use.
# Test: Search for the validation logic. Expect: Validation logic before this usage.
rg --type cpp --context 5 'g_pika_conf->slow_cmd_pool()'
Length of output: 1637 Script: #!/bin/bash
# Description: Verify that the configuration for `slow_cmd_pool` is validated before use.
# Test: Search for the initialization and validation logic of `slow_cmd_pool`.
rg --type cpp --context 5 'slow_cmd_pool'
Length of output: 4495 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
PikaServer::~PikaServer() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -166,12 +168,6 @@ void PikaServer::Start() { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
LOG(FATAL) << "Start PikaClientProcessor Error: " << ret | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<< (ret == net::kCreateThreadError ? ": create thread error " : ": other error"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ret = pika_slow_cmd_thread_pool_->start_thread_pool(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (ret != net::kSuccess) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
dbs_.clear(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
LOG(FATAL) << "Start PikaLowLevelThreadPool Error: " << ret | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<< (ret == net::kCreateThreadError ? ": create thread error " : ": other error"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ret = pika_dispatch_thread_->StartThread(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (ret != net::kSuccess) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
dbs_.clear(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -205,6 +201,24 @@ void PikaServer::Start() { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
LOG(INFO) << "Goodbye..."; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
void PikaServer::SetSlowCmdThreadPoolFlag(bool flag) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
slow_cmd_thread_pool_flag_ = flag; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
int ret = 0; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (flag) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ret = pika_slow_cmd_thread_pool_->start_thread_pool(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (ret != net::kSuccess) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
dbs_.clear(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
LOG(ERROR) << "Start PikaLowLevelThreadPool Error: " << ret | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
<< (ret == net::kCreateThreadError ? ": create thread error " : ": other error"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. pika_slow_cmd_thread_pool_->Schedule()的地方建议加个判断,因为现在is_slow_cmd和你开启slowcmdthreadpool是两个配置参数,如果is_slow_cmd判断为true,但是没有开slowcmdthreadpool,就会有问题。 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
done |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
while (SlowCmdThreadPoolCurQueueSize() != 0) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
std::this_thread::sleep_for(std::chrono::milliseconds(100)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pika_slow_cmd_thread_pool_->stop_thread_pool(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. stop_thread_pool应该是不会等队列里的任务被执行完成再结束,stop_thread_pool的时候队列里还有任务,那这些任务就被丢掉了。进而对应连接就不会被再次唤醒了。 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
done |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+204
to
+220
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider enhancing error handling in + try {
ret = pika_slow_cmd_thread_pool_->start_thread_pool();
if (ret != net::kSuccess) {
dbs_.clear();
LOG(ERROR) << "Start PikaLowLevelThreadPool Error: " << ret
<< (ret == net::kCreateThreadError ? ": create thread error " : ": other error");
}
+ } catch (const std::exception& e) {
+ LOG(ERROR) << "Exception caught while starting thread pool: " << e.what();
+ // Implement recovery or retry logic here
+ } Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
void PikaServer::Exit() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
g_pika_server->DisableCompact(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
exit_mutex_.unlock(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -707,17 +721,13 @@ void PikaServer::SetFirstMetaSync(bool v) { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
void PikaServer::ScheduleClientPool(net::TaskFunc func, void* arg, bool is_slow_cmd) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (is_slow_cmd) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (is_slow_cmd && g_pika_conf->slow_cmd_pool()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add safety checks to ensure + if (!pika_slow_cmd_thread_pool_) {
+ LOG(WARNING) << "Slow command thread pool is not initialized";
+ return;
+ }
if (is_slow_cmd && g_pika_conf->slow_cmd_pool()) {
pika_slow_cmd_thread_pool_->Schedule(func, arg);
return;
} Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pika_slow_cmd_thread_pool_->Schedule(func, arg); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pika_client_processor_->SchedulePool(func, arg); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
void PikaServer::ScheduleClientBgThreads(net::TaskFunc func, void* arg, const std::string& hash_str) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
pika_client_processor_->ScheduleBgThreads(func, arg, hash_str); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
size_t PikaServer::ClientProcessorThreadPoolCurQueueSize() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (!pika_client_processor_) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return 0; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,6 +33,7 @@ void RsyncWriteResp(RsyncService::RsyncResponse& response, std::shared_ptr<net:: | |
RsyncServer::RsyncServer(const std::set<std::string>& ips, const int port) { | ||
work_thread_ = std::make_unique<net::ThreadPool>(2, 100000); | ||
rsync_server_thread_ = std::make_unique<RsyncServerThread>(ips, port, 1 * 1000, this); | ||
rsync_server_thread_->set_thread_name("RsyncServer"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider using dynamic thread names for better traceability. - rsync_server_thread_->set_thread_name("RsyncServer");
+ rsync_server_thread_->set_thread_name("RsyncServer-" + std::to_string(port)); This change appends the port number to the thread name, making it easier to identify in logs and monitoring tools.
|
||
} | ||
|
||
RsyncServer::~RsyncServer() { | ||
|
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.
原子变量不用加锁保护,不要使用原子变量
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.
done