You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "TangSiyang2001 (via GitHub)" <gi...@apache.org> on 2024/10/24 10:02:11 UTC
[PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
TangSiyang2001 opened a new pull request, #42413:
URL: https://github.com/apache/doris/pull/42413
## Proposed changes
As title.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
Posted by "TangSiyang2001 (via GitHub)" <gi...@apache.org>.
TangSiyang2001 commented on PR #42413:
URL: https://github.com/apache/doris/pull/42413#issuecomment-2449871598
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
Posted by "TangSiyang2001 (via GitHub)" <gi...@apache.org>.
TangSiyang2001 commented on PR #42413:
URL: https://github.com/apache/doris/pull/42413#issuecomment-2472913469
run p0
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
Posted by "TangSiyang2001 (via GitHub)" <gi...@apache.org>.
TangSiyang2001 commented on PR #42413:
URL: https://github.com/apache/doris/pull/42413#issuecomment-2472912930
run compile
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
Posted by "TangSiyang2001 (via GitHub)" <gi...@apache.org>.
TangSiyang2001 commented on code in PR #42413:
URL: https://github.com/apache/doris/pull/42413#discussion_r1836029560
##########
cloud/src/rate-limiter/rate_limiter.cpp:
##########
@@ -41,36 +46,92 @@ void RateLimiter::init(google::protobuf::Service* service) {
int64_t max_qps_limit = std::stoll(v.substr(p + 1));
if (max_qps_limit > 0) {
rpc_name_to_max_qps_limit[rpc_name] = max_qps_limit;
- LOG(INFO) << "set rpc: " << rpc_name << " max_qps_limit: " << max_qps_limit;
}
} catch (...) {
- LOG(WARNING) << "failed to set max_qps_limit to rpc: " << rpc_name
+ LOG(WARNING) << "failed to parse max_qps_limit to rpc: " << rpc_name
<< " config: " << v;
}
}
}
+ return rpc_name_to_max_qps_limit;
+}
+
+template <typename Callable>
+void for_each_rpc_name(google::protobuf::Service* service, Callable cb) {
auto method_size = service->GetDescriptor()->method_count();
for (auto i = 0; i < method_size; ++i) {
std::string rpc_name = service->GetDescriptor()->method(i)->name();
- int64_t max_qps_limit = config::default_max_qps_limit;
+ cb(rpc_name);
+ }
+}
- auto it = rpc_name_to_max_qps_limit.find(rpc_name);
- if (it != rpc_name_to_max_qps_limit.end()) {
+void RateLimiter::init(google::protobuf::Service* service) {
+ auto rpc_name_to_specific_limit = parse_specific_qps_limit(config::specific_max_qps_limit);
+ std::unique_lock write_lock(mutex_);
+ for_each_rpc_name(service, [&](const std::string& rpc_name) {
+ auto it = rpc_name_to_specific_limit.find(rpc_name);
+ int64_t max_qps_limit = config::default_max_qps_limit;
+ if (it != rpc_name_to_specific_limit.end()) {
max_qps_limit = it->second;
}
limiters_[rpc_name] = std::make_shared<RpcRateLimiter>(rpc_name, max_qps_limit);
+ });
+ for (const auto& [k, _] : rpc_name_to_specific_limit) {
+ rpc_with_specific_limit_.insert(k);
}
}
std::shared_ptr<RpcRateLimiter> RateLimiter::get_rpc_rate_limiter(const std::string& rpc_name) {
- // no need to be locked, because it is only modified during initialization
auto it = limiters_.find(rpc_name);
if (it == limiters_.end()) {
return nullptr;
}
return it->second;
}
+bool RateLimiter::set_rate_limit(int64_t qps_limit) {
+ std::lock_guard lock(mutex_);
+ auto filter = [this](const auto& kv) { return !rpc_with_specific_limit_.contains(kv.first); };
Review Comment:
nope, just as it means literally
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
Posted by "TangSiyang2001 (via GitHub)" <gi...@apache.org>.
TangSiyang2001 commented on code in PR #42413:
URL: https://github.com/apache/doris/pull/42413#discussion_r1836027560
##########
cloud/src/rate-limiter/rate_limiter.cpp:
##########
@@ -41,36 +46,92 @@ void RateLimiter::init(google::protobuf::Service* service) {
int64_t max_qps_limit = std::stoll(v.substr(p + 1));
if (max_qps_limit > 0) {
rpc_name_to_max_qps_limit[rpc_name] = max_qps_limit;
- LOG(INFO) << "set rpc: " << rpc_name << " max_qps_limit: " << max_qps_limit;
}
} catch (...) {
- LOG(WARNING) << "failed to set max_qps_limit to rpc: " << rpc_name
+ LOG(WARNING) << "failed to parse max_qps_limit to rpc: " << rpc_name
<< " config: " << v;
}
}
}
+ return rpc_name_to_max_qps_limit;
+}
+
+template <typename Callable>
+void for_each_rpc_name(google::protobuf::Service* service, Callable cb) {
Review Comment:
There's no need to add a container, this function is only called once when doing init.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
Posted by "TangSiyang2001 (via GitHub)" <gi...@apache.org>.
TangSiyang2001 commented on PR #42413:
URL: https://github.com/apache/doris/pull/42413#issuecomment-2470421580
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #42413:
URL: https://github.com/apache/doris/pull/42413#discussion_r1816371925
##########
cloud/test/rate_limiter_test.cpp:
##########
@@ -17,8 +17,11 @@
#include "rate-limiter/rate_limiter.h"
Review Comment:
warning: 'rate-limiter/rate_limiter.h' file not found [clang-diagnostic-error]
```cpp
#include "rate-limiter/rate_limiter.h"
^
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
Posted by "gavinchou (via GitHub)" <gi...@apache.org>.
gavinchou commented on code in PR #42413:
URL: https://github.com/apache/doris/pull/42413#discussion_r1835747319
##########
cloud/src/rate-limiter/rate_limiter.cpp:
##########
@@ -41,36 +46,92 @@ void RateLimiter::init(google::protobuf::Service* service) {
int64_t max_qps_limit = std::stoll(v.substr(p + 1));
if (max_qps_limit > 0) {
rpc_name_to_max_qps_limit[rpc_name] = max_qps_limit;
- LOG(INFO) << "set rpc: " << rpc_name << " max_qps_limit: " << max_qps_limit;
}
} catch (...) {
- LOG(WARNING) << "failed to set max_qps_limit to rpc: " << rpc_name
+ LOG(WARNING) << "failed to parse max_qps_limit to rpc: " << rpc_name
<< " config: " << v;
}
}
}
+ return rpc_name_to_max_qps_limit;
+}
+
+template <typename Callable>
+void for_each_rpc_name(google::protobuf::Service* service, Callable cb) {
Review Comment:
`rpc_name`s are fixed after starting the service, consider storing the names in a container which can be a member of MetaServiceImpl
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
Posted by "TangSiyang2001 (via GitHub)" <gi...@apache.org>.
TangSiyang2001 commented on code in PR #42413:
URL: https://github.com/apache/doris/pull/42413#discussion_r1836033539
##########
cloud/src/meta-service/meta_service_http.cpp:
##########
@@ -331,6 +338,128 @@ static HttpResponse process_alter_iam(MetaServiceImpl* service, brpc::Controller
return http_json_reply(resp.status());
}
+static HttpResponse process_adjust_rate_limit(MetaServiceImpl* service, brpc::Controller* cntl) {
+ const auto& uri = cntl->http_request().uri();
+ auto qps_limit_str = std::string {http_query(uri, "qps_limit")};
+ auto rpc_name = std::string {http_query(uri, "rpc_name")};
+ auto instance_id = std::string {http_query(uri, "instance_id")};
+
+ auto process_invalid_arguments = [&]() -> HttpResponse {
+ return http_json_reply(MetaServiceCode::INVALID_ARGUMENT,
+ fmt::format("invalid argument: qps_limit(required)={}, "
+ "rpc_name(optional)={}, instance_id(optional)={}",
+ qps_limit_str, rpc_name, instance_id));
+ };
+
+ static auto parse_qps_limit =
+ [](const std::string& qps_limit_str) -> std::variant<int64_t, HttpResponse> {
+ DCHECK(!qps_limit_str.empty());
+ int64_t qps_limit = -1;
+ try {
+ qps_limit = std::stoll(qps_limit_str);
+ } catch (const std::exception& ex) {
+ return http_json_reply(
+ MetaServiceCode::INVALID_ARGUMENT,
+ fmt::format("param `qps_limit` is not a legal int64 type:{}", ex.what()));
+ }
+ if (qps_limit < 0) {
+ return http_json_reply(MetaServiceCode::INVALID_ARGUMENT,
+ "`qps_limit` should not be less than 0");
+ }
+ return qps_limit;
+ };
+
+ auto process_set_qps_limit = [&](std::function<bool(int64_t)> cb) -> HttpResponse {
+ return std::visit(
+ [&](auto&& parse_result) {
+ using T = std::decay_t<decltype(parse_result)>;
+ if constexpr (std::is_same_v<T, HttpResponse>) {
+ return parse_result;
+ } else {
+ if (cb(parse_result)) {
+ return http_json_reply(MetaServiceCode::OK,
+ "sucess to adjust rate limit");
+ }
+ return http_json_reply(
+ MetaServiceCode::INVALID_ARGUMENT,
+ fmt::format("failed to adjust rate limit for qps_limit={}, "
+ "rpc_name={}, instance_id={}, plz ensure correct "
+ "rpc/instance name",
+ qps_limit_str, rpc_name, instance_id));
+ }
+ },
+ parse_qps_limit(qps_limit_str));
+ };
+
+ auto set_global_qps_limit = [process_set_qps_limit, service]() {
+ return process_set_qps_limit([service](int64_t qps_limit) {
+ return service->rate_limiter()->set_rate_limit(qps_limit);
+ });
+ };
+
+ auto set_rpc_qps_limit = [&]() {
+ return process_set_qps_limit([&](int64_t qps_limit) {
+ return service->rate_limiter()->set_rate_limit(qps_limit, rpc_name);
+ });
+ };
+
+ auto set_instance_qps_limit = [&]() {
+ return process_set_qps_limit([&](int64_t qps_limit) {
+ return service->rate_limiter()->set_instance_rate_limit(qps_limit, instance_id);
+ });
+ };
+
+ auto set_instance_rpc_qps_limit = [&]() {
+ return process_set_qps_limit([&](int64_t qps_limit) {
+ return service->rate_limiter()->set_rate_limit(qps_limit, rpc_name, instance_id);
+ });
+ };
+
+ // for 8 element in true table of params, register processor cb
Review Comment:
Maybe I'll add some comments, just register different callback for different params pattern, and the "abuse" of lamda are just meant to adapt for this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #42413:
URL: https://github.com/apache/doris/pull/42413#issuecomment-2472801859
PR approved by anyone and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
Posted by "TangSiyang2001 (via GitHub)" <gi...@apache.org>.
TangSiyang2001 commented on PR #42413:
URL: https://github.com/apache/doris/pull/42413#issuecomment-2449756601
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
Posted by "TangSiyang2001 (via GitHub)" <gi...@apache.org>.
TangSiyang2001 commented on PR #42413:
URL: https://github.com/apache/doris/pull/42413#issuecomment-2440527114
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #42413:
URL: https://github.com/apache/doris/pull/42413#discussion_r1829342138
##########
cloud/src/meta-service/meta_service_http.cpp:
##########
@@ -331,6 +338,128 @@ static HttpResponse process_alter_iam(MetaServiceImpl* service, brpc::Controller
return http_json_reply(resp.status());
}
+static HttpResponse process_adjust_rate_limit(MetaServiceImpl* service, brpc::Controller* cntl) {
Review Comment:
warning: function 'process_adjust_rate_limit' exceeds recommended size/complexity thresholds [readability-function-size]
```cpp
static HttpResponse process_adjust_rate_limit(MetaServiceImpl* service, brpc::Controller* cntl) {
^
```
<details>
<summary>Additional context</summary>
**cloud/src/meta-service/meta_service_http.cpp:340:** 91 lines including whitespace and comments (threshold 80)
```cpp
static HttpResponse process_adjust_rate_limit(MetaServiceImpl* service, brpc::Controller* cntl) {
^
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #42413:
URL: https://github.com/apache/doris/pull/42413#issuecomment-2434838512
Thank you for your contribution to Apache Doris.
Don't know what should be done next? See [How to process your PR](https://cwiki.apache.org/confluence/display/DORIS/How+to+process+your+PR)
Since 2024-03-18, the Document has been moved to [doris-website](https://github.com/apache/doris-website).
See [Doris Document](https://cwiki.apache.org/confluence/display/DORIS/Doris+Document).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
Posted by "gavinchou (via GitHub)" <gi...@apache.org>.
gavinchou merged PR #42413:
URL: https://github.com/apache/doris/pull/42413
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
Posted by "TangSiyang2001 (via GitHub)" <gi...@apache.org>.
TangSiyang2001 commented on PR #42413:
URL: https://github.com/apache/doris/pull/42413#issuecomment-2437623533
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #42413:
URL: https://github.com/apache/doris/pull/42413#issuecomment-2472801762
PR approved by at least one committer and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
Posted by "TangSiyang2001 (via GitHub)" <gi...@apache.org>.
TangSiyang2001 commented on PR #42413:
URL: https://github.com/apache/doris/pull/42413#issuecomment-2472913757
run external
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
Posted by "TangSiyang2001 (via GitHub)" <gi...@apache.org>.
TangSiyang2001 commented on PR #42413:
URL: https://github.com/apache/doris/pull/42413#issuecomment-2472550513
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
Posted by "TangSiyang2001 (via GitHub)" <gi...@apache.org>.
TangSiyang2001 commented on code in PR #42413:
URL: https://github.com/apache/doris/pull/42413#discussion_r1836030033
##########
cloud/src/rate-limiter/rate_limiter.cpp:
##########
@@ -93,6 +154,35 @@ bool RpcRateLimiter::get_qps_token(const std::string& instance_id,
return qps_token->get_token(get_bvar_qps);
}
+void RpcRateLimiter::set_max_qps_limit(int64_t max_qps_limit) {
+ std::lock_guard<bthread::Mutex> l(mutex_);
+ max_qps_limit_ = max_qps_limit;
+ auto filter = [this](const auto& kv) {
+ return !instance_with_specific_limit_.contains(kv.first);
+ };
+ for (auto& [k, v] : qps_limiter_ | std::views::filter(std::move(filter))) {
+ v->set_max_qps_limit(max_qps_limit);
+ }
+}
+
+bool RpcRateLimiter::set_max_qps_limit(int64_t max_qps_limit, const std::string& instance_id) {
Review Comment:
just inheritate the original naming, will be mod
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
Posted by "TangSiyang2001 (via GitHub)" <gi...@apache.org>.
TangSiyang2001 commented on PR #42413:
URL: https://github.com/apache/doris/pull/42413#issuecomment-2457150394
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
Posted by "gavinchou (via GitHub)" <gi...@apache.org>.
gavinchou commented on code in PR #42413:
URL: https://github.com/apache/doris/pull/42413#discussion_r1835749363
##########
cloud/src/rate-limiter/rate_limiter.h:
##########
@@ -35,12 +38,32 @@ class RateLimiter {
public:
RateLimiter() = default;
~RateLimiter() = default;
+
void init(google::protobuf::Service* service);
+
std::shared_ptr<RpcRateLimiter> get_rpc_rate_limiter(const std::string& rpc_name);
+ void for_each_rpc_limiter(
+ std::function<void(std::string_view, std::shared_ptr<RpcRateLimiter>)> cb);
+
+ // set global default rate limit, will not infulence rpc and instance specific qps limit setting
+ bool set_rate_limit(int64_t qps_limit);
+
+ // set rpc level rate limit, will not infulence instance specific qps limit setting
+ bool set_rate_limit(int64_t qps_limit, const std::string& rpc_name);
+
+ // set instance level rate limit for specific rpc
+ bool set_rate_limit(int64_t qps_limit, const std::string& rpc_name,
+ const std::string& instance_id);
+
+ // set instance level rate limit globally, will influence settings for the same instance of specific rpc
+ bool set_instance_rate_limit(int64_t qps_limit, const std::string& instance_id);
+
private:
// rpc_name -> RpcRateLimiter
std::unordered_map<std::string, std::shared_ptr<RpcRateLimiter>> limiters_;
+ std::unordered_set<std::string> rpc_with_specific_limit_;
Review Comment:
add comment, it seems to be "This collection contains the RPCs that have had a limit set by the specified name"
##########
cloud/src/rate-limiter/rate_limiter.h:
##########
@@ -75,12 +107,17 @@ class RpcRateLimiter {
int64_t max_qps_limit_;
};
+ void for_each_qps_token(std::function<void(std::string_view, std::shared_ptr<QpsToken>)> cb);
+
+ // Todo: Recycle outdated instance_id
+
private:
bthread::Mutex mutex_;
// instance_id -> QpsToken
std::unordered_map<std::string, std::shared_ptr<QpsToken>> qps_limiter_;
+ std::unordered_set<std::string> instance_with_specific_limit_;
Review Comment:
ditto
##########
cloud/src/rate-limiter/rate_limiter.h:
##########
@@ -35,12 +38,32 @@ class RateLimiter {
public:
RateLimiter() = default;
~RateLimiter() = default;
+
void init(google::protobuf::Service* service);
+
std::shared_ptr<RpcRateLimiter> get_rpc_rate_limiter(const std::string& rpc_name);
+ void for_each_rpc_limiter(
Review Comment:
add comment even though it seems simple, describe: behavior, assumptions, params and return value
##########
cloud/src/rate-limiter/rate_limiter.cpp:
##########
@@ -93,6 +154,35 @@ bool RpcRateLimiter::get_qps_token(const std::string& instance_id,
return qps_token->get_token(get_bvar_qps);
}
+void RpcRateLimiter::set_max_qps_limit(int64_t max_qps_limit) {
+ std::lock_guard<bthread::Mutex> l(mutex_);
+ max_qps_limit_ = max_qps_limit;
+ auto filter = [this](const auto& kv) {
+ return !instance_with_specific_limit_.contains(kv.first);
+ };
+ for (auto& [k, v] : qps_limiter_ | std::views::filter(std::move(filter))) {
+ v->set_max_qps_limit(max_qps_limit);
+ }
+}
+
+bool RpcRateLimiter::set_max_qps_limit(int64_t max_qps_limit, const std::string& instance_id) {
Review Comment:
"limit" already implies "max", "max" in the name seems redundant
##########
cloud/src/rate-limiter/rate_limiter.h:
##########
@@ -58,15 +81,24 @@ class RpcRateLimiter {
*/
bool get_qps_token(const std::string& instance_id, std::function<int()>& get_bvar_qps);
- // Todo: Recycle outdated instance_id
+ std::string_view rpc_name() const { return rpc_name_; }
Review Comment:
add comment for every function: behavior, assumptions, params and return value
##########
cloud/src/meta-service/meta_service_http.cpp:
##########
@@ -331,6 +338,128 @@ static HttpResponse process_alter_iam(MetaServiceImpl* service, brpc::Controller
return http_json_reply(resp.status());
}
+static HttpResponse process_adjust_rate_limit(MetaServiceImpl* service, brpc::Controller* cntl) {
+ const auto& uri = cntl->http_request().uri();
+ auto qps_limit_str = std::string {http_query(uri, "qps_limit")};
+ auto rpc_name = std::string {http_query(uri, "rpc_name")};
+ auto instance_id = std::string {http_query(uri, "instance_id")};
+
+ auto process_invalid_arguments = [&]() -> HttpResponse {
+ return http_json_reply(MetaServiceCode::INVALID_ARGUMENT,
+ fmt::format("invalid argument: qps_limit(required)={}, "
+ "rpc_name(optional)={}, instance_id(optional)={}",
+ qps_limit_str, rpc_name, instance_id));
+ };
+
+ static auto parse_qps_limit =
+ [](const std::string& qps_limit_str) -> std::variant<int64_t, HttpResponse> {
+ DCHECK(!qps_limit_str.empty());
+ int64_t qps_limit = -1;
+ try {
+ qps_limit = std::stoll(qps_limit_str);
+ } catch (const std::exception& ex) {
+ return http_json_reply(
+ MetaServiceCode::INVALID_ARGUMENT,
+ fmt::format("param `qps_limit` is not a legal int64 type:{}", ex.what()));
+ }
+ if (qps_limit < 0) {
+ return http_json_reply(MetaServiceCode::INVALID_ARGUMENT,
+ "`qps_limit` should not be less than 0");
+ }
+ return qps_limit;
+ };
+
+ auto process_set_qps_limit = [&](std::function<bool(int64_t)> cb) -> HttpResponse {
Review Comment:
abuse lambdas, too many indentions and hard to read
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
Posted by "gavinchou (via GitHub)" <gi...@apache.org>.
gavinchou commented on code in PR #42413:
URL: https://github.com/apache/doris/pull/42413#discussion_r1835746298
##########
cloud/src/meta-service/meta_service_http.cpp:
##########
@@ -331,6 +338,128 @@ static HttpResponse process_alter_iam(MetaServiceImpl* service, brpc::Controller
return http_json_reply(resp.status());
}
+static HttpResponse process_adjust_rate_limit(MetaServiceImpl* service, brpc::Controller* cntl) {
+ const auto& uri = cntl->http_request().uri();
+ auto qps_limit_str = std::string {http_query(uri, "qps_limit")};
+ auto rpc_name = std::string {http_query(uri, "rpc_name")};
+ auto instance_id = std::string {http_query(uri, "instance_id")};
+
+ auto process_invalid_arguments = [&]() -> HttpResponse {
+ return http_json_reply(MetaServiceCode::INVALID_ARGUMENT,
+ fmt::format("invalid argument: qps_limit(required)={}, "
+ "rpc_name(optional)={}, instance_id(optional)={}",
+ qps_limit_str, rpc_name, instance_id));
+ };
+
+ static auto parse_qps_limit =
+ [](const std::string& qps_limit_str) -> std::variant<int64_t, HttpResponse> {
+ DCHECK(!qps_limit_str.empty());
+ int64_t qps_limit = -1;
+ try {
+ qps_limit = std::stoll(qps_limit_str);
+ } catch (const std::exception& ex) {
+ return http_json_reply(
+ MetaServiceCode::INVALID_ARGUMENT,
+ fmt::format("param `qps_limit` is not a legal int64 type:{}", ex.what()));
+ }
+ if (qps_limit < 0) {
+ return http_json_reply(MetaServiceCode::INVALID_ARGUMENT,
+ "`qps_limit` should not be less than 0");
+ }
+ return qps_limit;
+ };
+
+ auto process_set_qps_limit = [&](std::function<bool(int64_t)> cb) -> HttpResponse {
+ return std::visit(
+ [&](auto&& parse_result) {
+ using T = std::decay_t<decltype(parse_result)>;
+ if constexpr (std::is_same_v<T, HttpResponse>) {
+ return parse_result;
+ } else {
+ if (cb(parse_result)) {
+ return http_json_reply(MetaServiceCode::OK,
+ "sucess to adjust rate limit");
+ }
+ return http_json_reply(
+ MetaServiceCode::INVALID_ARGUMENT,
+ fmt::format("failed to adjust rate limit for qps_limit={}, "
+ "rpc_name={}, instance_id={}, plz ensure correct "
+ "rpc/instance name",
+ qps_limit_str, rpc_name, instance_id));
+ }
+ },
+ parse_qps_limit(qps_limit_str));
+ };
+
+ auto set_global_qps_limit = [process_set_qps_limit, service]() {
+ return process_set_qps_limit([service](int64_t qps_limit) {
+ return service->rate_limiter()->set_rate_limit(qps_limit);
+ });
+ };
+
+ auto set_rpc_qps_limit = [&]() {
+ return process_set_qps_limit([&](int64_t qps_limit) {
+ return service->rate_limiter()->set_rate_limit(qps_limit, rpc_name);
+ });
+ };
+
+ auto set_instance_qps_limit = [&]() {
+ return process_set_qps_limit([&](int64_t qps_limit) {
+ return service->rate_limiter()->set_instance_rate_limit(qps_limit, instance_id);
+ });
+ };
+
+ auto set_instance_rpc_qps_limit = [&]() {
+ return process_set_qps_limit([&](int64_t qps_limit) {
+ return service->rate_limiter()->set_rate_limit(qps_limit, rpc_name, instance_id);
+ });
+ };
+
+ // for 8 element in true table of params, register processor cb
Review Comment:
this part is confusing
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
Posted by "gavinchou (via GitHub)" <gi...@apache.org>.
gavinchou commented on code in PR #42413:
URL: https://github.com/apache/doris/pull/42413#discussion_r1835747702
##########
cloud/src/rate-limiter/rate_limiter.cpp:
##########
@@ -41,36 +46,92 @@ void RateLimiter::init(google::protobuf::Service* service) {
int64_t max_qps_limit = std::stoll(v.substr(p + 1));
if (max_qps_limit > 0) {
rpc_name_to_max_qps_limit[rpc_name] = max_qps_limit;
- LOG(INFO) << "set rpc: " << rpc_name << " max_qps_limit: " << max_qps_limit;
}
} catch (...) {
- LOG(WARNING) << "failed to set max_qps_limit to rpc: " << rpc_name
+ LOG(WARNING) << "failed to parse max_qps_limit to rpc: " << rpc_name
<< " config: " << v;
}
}
}
+ return rpc_name_to_max_qps_limit;
+}
+
+template <typename Callable>
+void for_each_rpc_name(google::protobuf::Service* service, Callable cb) {
auto method_size = service->GetDescriptor()->method_count();
for (auto i = 0; i < method_size; ++i) {
std::string rpc_name = service->GetDescriptor()->method(i)->name();
- int64_t max_qps_limit = config::default_max_qps_limit;
+ cb(rpc_name);
+ }
+}
- auto it = rpc_name_to_max_qps_limit.find(rpc_name);
- if (it != rpc_name_to_max_qps_limit.end()) {
+void RateLimiter::init(google::protobuf::Service* service) {
+ auto rpc_name_to_specific_limit = parse_specific_qps_limit(config::specific_max_qps_limit);
+ std::unique_lock write_lock(mutex_);
+ for_each_rpc_name(service, [&](const std::string& rpc_name) {
+ auto it = rpc_name_to_specific_limit.find(rpc_name);
+ int64_t max_qps_limit = config::default_max_qps_limit;
+ if (it != rpc_name_to_specific_limit.end()) {
max_qps_limit = it->second;
}
limiters_[rpc_name] = std::make_shared<RpcRateLimiter>(rpc_name, max_qps_limit);
+ });
+ for (const auto& [k, _] : rpc_name_to_specific_limit) {
+ rpc_with_specific_limit_.insert(k);
}
}
std::shared_ptr<RpcRateLimiter> RateLimiter::get_rpc_rate_limiter(const std::string& rpc_name) {
- // no need to be locked, because it is only modified during initialization
auto it = limiters_.find(rpc_name);
if (it == limiters_.end()) {
return nullptr;
}
return it->second;
}
+bool RateLimiter::set_rate_limit(int64_t qps_limit) {
+ std::lock_guard lock(mutex_);
+ auto filter = [this](const auto& kv) { return !rpc_with_specific_limit_.contains(kv.first); };
Review Comment:
is it "filter out"?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #42413:
URL: https://github.com/apache/doris/pull/42413#discussion_r1814686722
##########
cloud/src/rate-limiter/rate_limiter.h:
##########
@@ -19,10 +19,13 @@
#include <brpc/server.h>
Review Comment:
warning: 'brpc/server.h' file not found [clang-diagnostic-error]
```cpp
#include <brpc/server.h>
^
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
Posted by "gavinchou (via GitHub)" <gi...@apache.org>.
gavinchou commented on code in PR #42413:
URL: https://github.com/apache/doris/pull/42413#discussion_r1835746116
##########
cloud/src/meta-service/meta_service_http.cpp:
##########
@@ -331,6 +338,128 @@ static HttpResponse process_alter_iam(MetaServiceImpl* service, brpc::Controller
return http_json_reply(resp.status());
}
+static HttpResponse process_adjust_rate_limit(MetaServiceImpl* service, brpc::Controller* cntl) {
+ const auto& uri = cntl->http_request().uri();
+ auto qps_limit_str = std::string {http_query(uri, "qps_limit")};
+ auto rpc_name = std::string {http_query(uri, "rpc_name")};
+ auto instance_id = std::string {http_query(uri, "instance_id")};
+
+ auto process_invalid_arguments = [&]() -> HttpResponse {
+ return http_json_reply(MetaServiceCode::INVALID_ARGUMENT,
+ fmt::format("invalid argument: qps_limit(required)={}, "
+ "rpc_name(optional)={}, instance_id(optional)={}",
+ qps_limit_str, rpc_name, instance_id));
+ };
+
+ static auto parse_qps_limit =
Review Comment:
it seems that abuse using lambdas
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
Posted by "gavinchou (via GitHub)" <gi...@apache.org>.
gavinchou commented on PR #42413:
URL: https://github.com/apache/doris/pull/42413#issuecomment-2451991569
Add some usage examples in description.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #42413:
URL: https://github.com/apache/doris/pull/42413#discussion_r1824386758
##########
cloud/src/meta-service/meta_service_http.cpp:
##########
@@ -331,6 +338,122 @@ static HttpResponse process_alter_iam(MetaServiceImpl* service, brpc::Controller
return http_json_reply(resp.status());
}
+static HttpResponse process_adjust_rate_limit(MetaServiceImpl* service, brpc::Controller* cntl) {
Review Comment:
warning: function 'process_adjust_rate_limit' exceeds recommended size/complexity thresholds [readability-function-size]
```cpp
static HttpResponse process_adjust_rate_limit(MetaServiceImpl* service, brpc::Controller* cntl) {
^
```
<details>
<summary>Additional context</summary>
**cloud/src/meta-service/meta_service_http.cpp:340:** 90 lines including whitespace and comments (threshold 80)
```cpp
static HttpResponse process_adjust_rate_limit(MetaServiceImpl* service, brpc::Controller* cntl) {
^
```
</details>
##########
cloud/test/meta_service_http_test.cpp:
##########
@@ -1454,4 +1454,94 @@ TEST(MetaServiceHttpTest, TxnLazyCommit) {
}
}
+TEST(MetaServiceHttpTest, AdjustRateLimit) {
+ HttpContext ctx;
+ {
+ auto [status_code, content] =
+ ctx.query<std::string>("adjust_rate_limit", "qps_limit=10000");
+ ASSERT_EQ(status_code, 200);
+ }
+ {
+ auto [status_code, content] =
Review Comment:
warning: function 'TEST' exceeds recommended size/complexity thresholds [readability-function-size]
```cpp
}
^
```
<details>
<summary>Additional context</summary>
**cloud/test/meta_service_http_test.cpp:1464:** 82 lines including whitespace and comments (threshold 80)
```cpp
}
^
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
Posted by "TangSiyang2001 (via GitHub)" <gi...@apache.org>.
TangSiyang2001 commented on PR #42413:
URL: https://github.com/apache/doris/pull/42413#issuecomment-2449750260
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
Posted by "TangSiyang2001 (via GitHub)" <gi...@apache.org>.
TangSiyang2001 commented on PR #42413:
URL: https://github.com/apache/doris/pull/42413#issuecomment-2472913282
run cloud_p0
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [feature](meta-service) Support querying and adjusting rpc qps limit on meta service [doris]
Posted by "TangSiyang2001 (via GitHub)" <gi...@apache.org>.
TangSiyang2001 commented on PR #42413:
URL: https://github.com/apache/doris/pull/42413#issuecomment-2438262887
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org