-
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 22 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 | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,13 +5,21 @@ | |||||||||||||||||||||
| #include <pingcap/kv/RegionCache.h> | ||||||||||||||||||||||
| #include <pingcap/kv/internal/type_traits.h> | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| #include <atomic> | ||||||||||||||||||||||
| #include <chrono> | ||||||||||||||||||||||
| #include <condition_variable> | ||||||||||||||||||||||
| #include <map> | ||||||||||||||||||||||
| #include <mutex> | ||||||||||||||||||||||
| #include <utility> | ||||||||||||||||||||||
| #include <vector> | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| namespace pingcap | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| namespace kv | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| constexpr auto rpc_conn_check_interval = std::chrono::minutes(10); | ||||||||||||||||||||||
| constexpr auto rpc_conn_check_interval_jitter = std::chrono::minutes(5); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| struct ConnArray | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| std::mutex mutex; | ||||||||||||||||||||||
|
|
@@ -33,31 +41,60 @@ using GRPCMetaData = std::multimap<std::string, std::string>; | |||||||||||||||||||||
| struct RpcClient | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| ClusterConfig config; | ||||||||||||||||||||||
| pd::ClientPtr pd_client; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| std::mutex mutex; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| std::map<std::string, ConnArrayPtr> conns; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Logger * log = &Logger::get("pingcap.RpcClient"); | ||||||||||||||||||||||
| std::chrono::minutes scan_interval = rpc_conn_check_interval; | ||||||||||||||||||||||
| std::atomic<bool> stopped = false; | ||||||||||||||||||||||
| std::condition_variable scan_cv; | ||||||||||||||||||||||
| std::vector<std::string> invalid_conns; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| RpcClient() = default; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| explicit RpcClient(const ClusterConfig & config_) | ||||||||||||||||||||||
| : config(config_) | ||||||||||||||||||||||
| {} | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| RpcClient(pd::ClientPtr pd_client_, const ClusterConfig & config_) | ||||||||||||||||||||||
| : config(config_) | ||||||||||||||||||||||
| , pd_client(std::move(pd_client_)) | ||||||||||||||||||||||
| {} | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| void update(const ClusterConfig & config_) | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| std::unique_lock lk(mutex); | ||||||||||||||||||||||
| std::lock_guard<std::mutex> lk(mutex); | ||||||||||||||||||||||
| config = config_; | ||||||||||||||||||||||
| conns.clear(); | ||||||||||||||||||||||
| invalid_conns.clear(); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| void run(); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| void stop(); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| void scanConns(); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| void removeConn(const std::string & addr); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| void removeInvalidConns(); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ConnArrayPtr getConnArray(const std::string & addr); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ConnArrayPtr createConnArray(const std::string & addr); | ||||||||||||||||||||||
| }; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| using RpcClientPtr = std::unique_ptr<RpcClient>; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| inline void dropConnIfNeeded(const RpcClientPtr & client, const std::string & addr, const ::grpc::Status & status) | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| if (status.error_code() == grpc::StatusCode::UNAVAILABLE) | ||||||||||||||||||||||
| client->removeConn(addr); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
Comment on lines
+88
to
+92
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. Align
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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // RpcCall holds the request and response, and delegates RPC calls. | ||||||||||||||||||||||
| template <typename T> | ||||||||||||||||||||||
| class RpcCall | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| 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,84 @@ 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 | ||
| { | ||
| scanConns(); | ||
| removeInvalidConns(); | ||
| } | ||
| catch (...) | ||
| { | ||
| log->warning(getCurrentExceptionMsg("RpcClient scan conns failed: ")); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| void RpcClient::stop() | ||
| { | ||
| stopped.store(true); | ||
| scan_cv.notify_all(); | ||
| } | ||
|
|
||
| void RpcClient::scanConns() | ||
| { | ||
| 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::lock_guard<std::mutex> lock(mutex); | ||
| for (const auto & addr : conn_snapshot) | ||
| { | ||
| if (store_addrs.find(addr) == store_addrs.end()) | ||
| invalid_conns.push_back(addr); | ||
|
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
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.
Check in L106 is deleted now.
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.
I delete the |
||
| } | ||
| } | ||
|
|
||
| void RpcClient::removeConn(const std::string & addr) | ||
| { | ||
| std::lock_guard<std::mutex> lock(mutex); | ||
| if (conns.erase(addr)) | ||
| log->information("delete invalid addr: " + addr); | ||
| } | ||
|
|
||
| void RpcClient::removeInvalidConns() | ||
| { | ||
| std::lock_guard<std::mutex> lock(mutex); | ||
| if (invalid_conns.empty()) | ||
| return; | ||
|
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. add lock after
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.
Lock has been added in L130, and it will not be released until returning from function.
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. Looks like in the latest code,
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.
invalid_conns is also used in |
||
|
|
||
| for (const auto & addr : invalid_conns) | ||
| { | ||
| if (conns.erase(addr)) | ||
| log->information("delete invalid addr: " + addr); | ||
| } | ||
|
|
||
| invalid_conns.clear(); | ||
| } | ||
|
|
||
| ConnArrayPtr RpcClient::getConnArray(const std::string & addr) | ||
| { | ||
| std::lock_guard<std::mutex> lock(mutex); | ||
|
|
@@ -39,6 +148,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.
why change this