-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
circuit breaker with half open state #2634
Conversation
src/brpc/circuit_breaker.cpp
Outdated
@@ -45,6 +46,10 @@ DEFINE_int32(circuit_breaker_max_isolation_duration_ms, 30000, | |||
"Maximum isolation duration in milliseconds"); | |||
DEFINE_double(circuit_breaker_epsilon_value, 0.02, | |||
"ema_alpha = 1 - std::pow(epsilon, 1.0 / window_size)"); | |||
DEFINE_int32(circuit_breaker_half_open_window_size, 10, |
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.
这个功能应该是可选的吧,比如half_open_window_size为0表示关闭该功能。默认为关闭,和之前的行为保持兼容。
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.
改了
src/brpc/circuit_breaker.cpp
Outdated
@@ -45,6 +46,10 @@ DEFINE_int32(circuit_breaker_max_isolation_duration_ms, 30000, | |||
"Maximum isolation duration in milliseconds"); | |||
DEFINE_double(circuit_breaker_epsilon_value, 0.02, | |||
"ema_alpha = 1 - std::pow(epsilon, 1.0 / window_size)"); | |||
DEFINE_int32(circuit_breaker_half_open_window_size, 0, | |||
"Half open window sample size."); |
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.
Can this be more descriptive?
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
谢谢,您的邮件我已收到!
|
LGTM |
src/brpc/circuit_breaker.cpp
Outdated
+ 1 == FLAGS_circuit_breaker_half_open_window_size) { | ||
_half_open.store(false, butil::memory_order_relaxed); | ||
_half_open_success_count.store(0, butil::memory_order_relaxed); | ||
return true; |
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.
Should we remove the return here and let this latency passed to _long_window/_short_window.OnCallEnd below instead?
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.
What problem does this PR solve?
Issue Number:
Problem Summary:
What is changed and the side effects?
Changed:
Side effects:
Performance effects(性能影响):
Breaking backward compatibility(向后兼容性):
Check List: