Add lumping_interval Feature for Semian Circuit Breakers #606
Conversation
abishanan-shopify
force-pushed
the
lumping-interval
branch
9 times, most recently
from
77226a1 to
079c0d0
Compare
lib/semian/circuit_breaker.rb
Outdated
| @@ -26,6 +27,11 @@ def initialize(name, exceptions:, success_threshold:, error_threshold:, | |||
| @error_timeout = error_timeout | |||
| @exceptions = exceptions | |||
| @half_open_resource_timeout = half_open_resource_timeout | |||
| @lumping_interval = lumping_interval | |||
|
|
|||
| if @lumping_interval >= @error_threshold_timeout && @lumping_interval > 0 | |||
There was a problem hiding this comment.
do we need the && here? I think what would make more sense is to prevent negative values for all parameters instead
lib/semian/circuit_breaker.rb
Outdated
| if error_threshold_timeout_enabled | ||
| @errors.reject! { |err_time| err_time + @error_threshold_timeout < time } | ||
| end | ||
|
|
||
| @errors << time | ||
| if @lumping_interval == 0 || @errors.empty? || @errors.last < time - @lumping_interval |
There was a problem hiding this comment.
is the first condition necessary here? 🤔
abishanan-shopify
force-pushed
the
lumping-interval
branch
from
079c0d0 to
a27c582
Compare
fix notation for index for SlidingWindow and edge case for empty array
abishanan-shopify
force-pushed
the
lumping-interval
branch
from
a27c582 to
00c9d2d
Compare
abishanan-shopify
changed the title
|
Regarding the visual, I love it! But I also feel like it's sending the wrong message 
The feature is helping us not open the circuit too soon on a network blip. So I feel like the visuals should show the errors happening within the first second, followed by a period of calmness. And how the lumping_interval helped us not open the circuit in that situation |
README.md
Outdated
| the circuit is half-open (supported for MySQL, Net::HTTP and Redis). | ||
| - **lumping_interval**. Interval for timeframe of errors to be lumped and recorded as one. |
There was a problem hiding this comment.
| - **lumping_interval**. Interval for timeframe of errors to be lumped and recorded as one. | |
| - **lumping_interval**. If provided, errors within this interval will be lumped and recorded as one. |
There was a problem hiding this comment.
would also be good to mention that it's in seconds.
Updated with a more relevant visual! 👍 |
Changes:
push_time,create_circuit_breaker, andinitializefunctions to incorporate lumping intervalsRationale:
When it comes to real-world applications, errors can sometimes occur in short-term bursts due to insignificant network disruptions, outages, or other small "blips" in the network that rapidly self-resolve.
Under the current implementation, each error, no matter how close together in time, counts individually. This means that a short burst of errors, no matter how far apart they are in time, will affect the circuit breaker by accumulating the error count. This has a few issues:
Implementation: