You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brpc.apache.org by GitBox <gi...@apache.org> on 2022/12/02 06:45:18 UTC
[GitHub] [incubator-brpc] wwbmmm commented on a diff in pull request #2027: add timeout concurrency limiter
wwbmmm commented on code in PR #2027:
URL: https://github.com/apache/incubator-brpc/pull/2027#discussion_r1037831763
##########
src/brpc/policy/timeout_concurrency_limiter.cpp:
##########
@@ -0,0 +1,161 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "brpc/policy/timeout_concurrency_limiter.h"
+#include "brpc/controller.h"
+#include "brpc/errno.pb.h"
+#include <cmath>
+#include <gflags/gflags.h>
+
+namespace brpc {
+namespace policy {
+
+DEFINE_int32(timeout_cl_sample_window_size_ms, 1000,
+ "Duration of the sampling window.");
+DEFINE_int32(timeout_cl_min_sample_count, 100,
+ "During the duration of the sampling window, if the number of "
+ "requests collected is less than this value, the sampling window "
+ "will be discarded.");
+DEFINE_int32(timeout_cl_max_sample_count, 200,
+ "During the duration of the sampling window, once the number of "
+ "requests collected is greater than this value, even if the "
+ "duration of the window has not ended, the max_concurrency will "
+ "be updated and a new sampling window will be started.");
+DEFINE_double(timeout_cl_sampling_interval_ms, 0.1,
+ "Interval for sampling request in auto concurrency limiter");
+DEFINE_int32(timeout_cl_initial_avg_latency_us, 500,
+ "Initial max concurrency for gradient concurrency limiter");
+DEFINE_bool(
+ timeout_cl_enable_error_punish, true,
+ "Whether to consider failed requests when calculating maximum concurrency");
+DEFINE_double(
+ timeout_cl_fail_punish_ratio, 1.0,
+ "Use the failed requests to punish normal requests. The larger "
+ "the configuration item, the more aggressive the penalty strategy.");
+DEFINE_int32(timeout_cl_default_timeout_ms, 500,
+ "Default timeout for rpc request");
+
+TimeoutConcurrencyLimiter::TimeoutConcurrencyLimiter()
+ : _avg_latency_us(FLAGS_timeout_cl_initial_avg_latency_us),
+ _last_sampling_time_us(0) {}
+
+TimeoutConcurrencyLimiter *TimeoutConcurrencyLimiter::New(
+ const AdaptiveMaxConcurrency &) const {
+ return new (std::nothrow) TimeoutConcurrencyLimiter;
+}
+
+bool TimeoutConcurrencyLimiter::OnRequested(int current_concurrency,
+ Controller *cntl) {
+ auto timeout_ms = FLAGS_timeout_cl_default_timeout_ms;
+ if (cntl != nullptr && cntl->timeout_ms() != UNSET_MAGIC_NUM) {
+ timeout_ms = cntl->timeout_ms();
+ }
+ return current_concurrency * _avg_latency_us < timeout_ms * 1000;
Review Comment:
这里好像不需要乘以current_concurrency，我理解_avg_latency_us < timeout_ms * 1000就可以了。就是说服务过载时平响肯定会增大，增大到阈值就开始限流就可以了。
##########
src/brpc/policy/timeout_concurrency_limiter.cpp:
##########
@@ -0,0 +1,161 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "brpc/policy/timeout_concurrency_limiter.h"
+#include "brpc/controller.h"
+#include "brpc/errno.pb.h"
+#include <cmath>
+#include <gflags/gflags.h>
+
+namespace brpc {
+namespace policy {
+
+DEFINE_int32(timeout_cl_sample_window_size_ms, 1000,
+ "Duration of the sampling window.");
+DEFINE_int32(timeout_cl_min_sample_count, 100,
+ "During the duration of the sampling window, if the number of "
+ "requests collected is less than this value, the sampling window "
+ "will be discarded.");
+DEFINE_int32(timeout_cl_max_sample_count, 200,
+ "During the duration of the sampling window, once the number of "
+ "requests collected is greater than this value, even if the "
+ "duration of the window has not ended, the max_concurrency will "
+ "be updated and a new sampling window will be started.");
+DEFINE_double(timeout_cl_sampling_interval_ms, 0.1,
+ "Interval for sampling request in auto concurrency limiter");
+DEFINE_int32(timeout_cl_initial_avg_latency_us, 500,
+ "Initial max concurrency for gradient concurrency limiter");
+DEFINE_bool(
+ timeout_cl_enable_error_punish, true,
+ "Whether to consider failed requests when calculating maximum concurrency");
+DEFINE_double(
+ timeout_cl_fail_punish_ratio, 1.0,
+ "Use the failed requests to punish normal requests. The larger "
+ "the configuration item, the more aggressive the penalty strategy.");
+DEFINE_int32(timeout_cl_default_timeout_ms, 500,
+ "Default timeout for rpc request");
+
+TimeoutConcurrencyLimiter::TimeoutConcurrencyLimiter()
+ : _avg_latency_us(FLAGS_timeout_cl_initial_avg_latency_us),
+ _last_sampling_time_us(0) {}
+
+TimeoutConcurrencyLimiter *TimeoutConcurrencyLimiter::New(
+ const AdaptiveMaxConcurrency &) const {
+ return new (std::nothrow) TimeoutConcurrencyLimiter;
+}
+
+bool TimeoutConcurrencyLimiter::OnRequested(int current_concurrency,
+ Controller *cntl) {
+ auto timeout_ms = FLAGS_timeout_cl_default_timeout_ms;
+ if (cntl != nullptr && cntl->timeout_ms() != UNSET_MAGIC_NUM) {
+ timeout_ms = cntl->timeout_ms();
Review Comment:
感觉这里使用Controller的timeout_ms不太合适，那个是请求级别的，每个请求设置的timeout可能不一样，比如有的请求timeout只有10ms，然后这里avg_latency是100ms，就返回了拒绝。另外就是不是所有协议都能从server的Controller拿到client端的timeout_ms。
可以只用FLAGS_timeout_cl_default_timeout_ms这个。
--
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: dev-unsubscribe@brpc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@brpc.apache.org
For additional commands, e-mail: dev-help@brpc.apache.org