-
Notifications
You must be signed in to change notification settings - Fork 56
Remove useless grpc connections #233
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
Changes from 23 commits
ccc48d4
ee16376
80a14ea
a3ecf4d
6badc3b
7a5e808
a07dd67
9451c37
6ff1ddc
7264109
148caa0
ed20a9c
93e6e1c
cd31303
88f5a7f
1530578
6969757
f27b1e5
f627bc2
798b04a
637d619
b3e060e
ddb2aed
b9cf4b4
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 |
|---|---|---|
| @@ -1,9 +1,40 @@ | ||
| #include <pingcap/Exception.h> | ||
| #include <pingcap/kv/Rpc.h> | ||
|
|
||
| #include <random> | ||
| #include <unordered_set> | ||
|
|
||
| namespace pingcap | ||
| { | ||
| namespace kv | ||
| { | ||
| namespace | ||
| { | ||
| std::unordered_set<std::string> getStoreAddresses(const pd::ClientPtr & pd_client) | ||
| { | ||
| std::unordered_set<std::string> store_addrs; | ||
| const auto stores = pd_client->getAllStores(true); | ||
| store_addrs.reserve(stores.size()); | ||
| for (const auto & store : stores) | ||
| { | ||
| if (!store.address().empty()) | ||
| store_addrs.emplace(store.address()); | ||
| } | ||
| return store_addrs; | ||
| } | ||
|
|
||
| std::chrono::seconds getRandomScanInterval(std::chrono::minutes scan_interval) | ||
| { | ||
| const auto min_seconds = std::chrono::duration_cast<std::chrono::seconds>(scan_interval); | ||
| const auto max_seconds = std::chrono::duration_cast<std::chrono::seconds>( | ||
| scan_interval + rpc_conn_check_interval_jitter); | ||
|
|
||
| thread_local std::mt19937_64 generator(std::random_device{}()); | ||
| std::uniform_int_distribution<std::chrono::seconds::rep> distribution(min_seconds.count(), max_seconds.count()); | ||
| return std::chrono::seconds(distribution(generator)); | ||
| } | ||
| } // namespace | ||
|
|
||
| ConnArray::ConnArray(size_t max_size, const std::string & addr, const ClusterConfig & config_) | ||
| : address(addr) | ||
| , index(0) | ||
|
|
@@ -22,6 +53,75 @@ std::shared_ptr<KvConnClient> ConnArray::get() | |
| return vec[index]; | ||
| } | ||
|
|
||
| void RpcClient::run() | ||
| { | ||
| while (!stopped.load()) | ||
| { | ||
| { | ||
| const auto wait_interval = getRandomScanInterval(scan_interval); | ||
| std::unique_lock lock(mutex); | ||
| scan_cv.wait_for(lock, wait_interval, [this] { | ||
| return stopped.load(); | ||
| }); | ||
| } | ||
|
|
||
| if (stopped.load()) | ||
| return; | ||
|
|
||
| try | ||
| { | ||
| scanAndRemoveInvalidConns(); | ||
| } | ||
| catch (...) | ||
| { | ||
| log->warning(getCurrentExceptionMsg("RpcClient scan conns failed: ")); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| void RpcClient::stop() | ||
| { | ||
| stopped.store(true); | ||
| scan_cv.notify_all(); | ||
| } | ||
|
|
||
| void RpcClient::scanAndRemoveInvalidConns() | ||
| { | ||
| std::vector<std::string> conn_snapshot; | ||
| { | ||
| std::lock_guard<std::mutex> lock(mutex); | ||
| conn_snapshot.reserve(conns.size()); | ||
| for (const auto & conn : conns) | ||
| conn_snapshot.emplace_back(conn.first); | ||
| } | ||
|
|
||
| if (conn_snapshot.empty() || !pd_client || pd_client->isMock()) | ||
|
Contributor
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. it looks to me that we can check
Contributor
Author
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.
fixed |
||
| return; | ||
|
|
||
| const auto store_addrs = getStoreAddresses(pd_client); | ||
| std::vector<std::string> invalid_conns; | ||
| for (const auto & addr : conn_snapshot) | ||
| { | ||
| if (store_addrs.find(addr) == store_addrs.end()) | ||
| invalid_conns.push_back(addr); | ||
| } | ||
|
|
||
| if (invalid_conns.empty()) | ||
| return; | ||
|
|
||
| for (const auto & addr : invalid_conns) | ||
| { | ||
| removeConn(addr); | ||
| } | ||
| } | ||
|
|
||
| void RpcClient::removeConn(const std::string & addr) | ||
| { | ||
| std::lock_guard<std::mutex> lock(mutex); | ||
| if (conns.erase(addr)) | ||
| log->information("delete invalid addr: " + addr); | ||
| } | ||
|
|
||
| ConnArrayPtr RpcClient::getConnArray(const std::string & addr) | ||
| { | ||
| std::lock_guard<std::mutex> lock(mutex); | ||
|
|
@@ -39,6 +139,5 @@ ConnArrayPtr RpcClient::createConnArray(const std::string & addr) | |
| conns[addr] = conn_array; | ||
| return conn_array; | ||
| } | ||
|
|
||
| } // namespace kv | ||
| } // namespace pingcap | ||
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.
Align
dropConnIfNeeded()with the shared removal predicate.shouldRemoveConnOnStatus()treats bothUNAVAILABLEandCANCELLEDas removable, butdropConnIfNeeded()only handlesUNAVAILABLE. SinceonSendFail()uses this helper, unary failures and stream setup failures withCANCELLEDwon’t invalidate the connection, whileStreamReader::finish()will.Proposed fix
inline void dropConnIfNeeded(const RpcClientPtr & client, const std::string & addr, const ::grpc::Status & status) { - if (status.error_code() == grpc::StatusCode::UNAVAILABLE) + if (shouldRemoveConnOnStatus(status)) client->markConnInvalid(addr); }📝 Committable suggestion
🤖 Prompt for AI Agents