Require stable Freno recovery

README.md

@@ -202,14 +202,17 @@ module Freno
 
     DEFAULT_WAIT_SECONDS = 0.5
     DEFAULT_MAX_WAIT_SECONDS = 10
+    DEFAULT_REQUIRED_CONSECUTIVE_SUCCESSES = 3
 
     def initialize(client: nil,
                     app: nil,
                     mapper: Mapper::Identity,
                     instrumenter: Instrumenter::Noop,
                     circuit_breaker: CircuitBreaker::Noop,
                     wait_seconds: DEFAULT_WAIT_SECONDS,
-                    max_wait_seconds: DEFAULT_MAX_WAIT_SECONDS)
+                    max_wait_seconds: DEFAULT_MAX_WAIT_SECONDS,
+                    required_consecutive_successes: DEFAULT_REQUIRED_CONSECUTIVE_SUCCESSES,
+                    stable_recovery: false)
 
 
       @client           = client
@@ -219,6 +222,8 @@ module Freno
       @circuit_breaker  = circuit_breaker
       @wait_seconds     = wait_seconds
       @max_wait_seconds = max_wait_seconds
+      @required_consecutive_successes = required_consecutive_successes
+      @stable_recovery = stable_recovery
 
       yield self if block_given?
 
@@ -238,6 +243,50 @@ You optionally provide the time you want the throttler to sleep in case the chec
 If replication lags badly, you can control until when you want to keep sleeping
 and retrying the check by setting `max_wait_seconds`. When that times out, the throttle will raise a `Freno::Throttler::WaitedTooLong` error.
 
+Stable recovery is opt-in (`stable_recovery: false` by default), preserving
+legacy recovery and timeout circuit-breaker behavior when upgrading the gem.
+With `stable_recovery: true`, an initially healthy check proceeds immediately. After any failed check,
+`required_consecutive_successes` passing checks are required before the block
+runs. Passing samples must be consecutive; another rejection resets the count.
+This prevents an oscillating metric from releasing work on a single healthy
+trough.
+
+With stable recovery enabled, `WaitedTooLong` represents normal sustained
+throttling and does not count as a circuit-breaker failure. Freno transport and
+decision errors still fail the circuit breaker.
+
+#### Rolling out stable recovery
+
+Gate `stable_recovery` with the consuming application's feature switch and enable
+it for one reviewed workload first:
+
+```ruby
+Freno::Throttler.new(
+  client: freno_client,
+  app: :my_app,
+  stable_recovery: true,
+  required_consecutive_successes: 3,
+  max_wait_seconds: 45
+)
+```
+
+The example wait budget accommodates the proposed server-side 30-second
+continuous recovery window plus client stabilization; it does not guarantee
+recovery during sustained overload. Keep the default 10-second budget for
+existing callers unless their owners explicitly approve a longer blocking
+period. Callers with a shorter budget must safely defer/retry `WaitedTooLong`
+without executing the protected work.
+
+Observe `throttler.waited`, `throttler.waited_too_long`, client errors, job
+throughput and backlog. Recovery instrumentation includes `stable_recovery` and
+consecutive-success progress. Switch `stable_recovery` back to false to restore
+both legacy recovery and timeout circuit-breaker behavior. Setting only
+`required_consecutive_successes: 1` does not restore the latter.
+
+Recovery progress is local to a single `throttle` call, not shared between
+calls, workers, or processes. Deploy the compatible server before enabling its
+primary dependencies; server enforcement and this client switch are independent.
+
 #### Instrumenting the throttler
 
 You can also configure the throttler with an `instrumenter` collaborator to subscribe to events happening during the `throttle` call.
@@ -287,7 +336,10 @@ The throttler can also receive a `circuit_breaker` object to implement resilienc
 
 With that information it receives, the circuit breaker determines whether or not to allow the next request. A circuit is said to be open when the next request is not allowed; and it's said to be closed when the next request is allowed
 
-If the throttler waited too long, or an unexpected error happened; the circuit breaker will receive a `failure`. If in contrast it succeeded, the circuit breaker will receive a `success` message.
+If an unexpected Freno transport or decision error happens, the circuit breaker
+receives a `failure`. Sustained throttling that raises `WaitedTooLong` also fails
+the circuit breaker unless `stable_recovery: true`. When a check succeeds, the
+circuit breaker receives a `success` message.
 
 Once the circuit is open, the throttler will not try to throttle calls, an instead throw a `Freno::Throttler::CircuitOpen`
 

lib/freno/client.rb

@@ -233,7 +233,7 @@ def decorated(request)
         outermost = to_decorate[0]
         current = outermost
 
-        (to_decorate[1..]).each do |decorator|
+        to_decorate[1..].each do |decorator|
           current.request = decorator
           current = current.request
         end

lib/freno/client/errors.rb

@@ -1,5 +1,6 @@
 # frozen_string_literal: true
 
 module Freno
-  Error = Class.new(StandardError)
+  class Error < StandardError
+  end
 end

lib/freno/client/preconditions.rb

@@ -5,7 +5,8 @@ class Client
     module Preconditions
       module_function
 
-      PreconditionNotMet = Class.new(ArgumentError)
+      class PreconditionNotMet < ArgumentError
+      end
 
       class Checker
         attr_reader :errors

lib/freno/throttler.rb

@@ -35,6 +35,7 @@ module Freno
   class Throttler
     DEFAULT_WAIT_SECONDS = 0.5
     DEFAULT_MAX_WAIT_SECONDS = 10
+    DEFAULT_REQUIRED_CONSECUTIVE_SUCCESSES = 3
     REQUIRED_ARGS = %i[
       client
       app
@@ -43,6 +44,7 @@ class Throttler
       circuit_breaker
       wait_seconds
       max_wait_seconds
+      required_consecutive_successes
     ].freeze
 
     attr_accessor :client,
@@ -51,7 +53,9 @@ class Throttler
                   :instrumenter,
                   :circuit_breaker,
                   :wait_seconds,
-                  :max_wait_seconds
+                  :max_wait_seconds,
+                  :required_consecutive_successes,
+                  :stable_recovery
 
     # Initializes a new instance of the throttler
     #
@@ -99,14 +103,22 @@ class Throttler
     #     seconds the throttler will wait in total for replicas to catch-up
     #     before raising a `WaitedTooLong` error.
     #
+    #  - `:required_consecutive_successes`: The number of consecutive passing
+    #     checks required after a failed check when stable recovery is enabled.
+    #
+    #  - `:stable_recovery`: Opt into consecutive-success recovery and treating
+    #     WaitedTooLong as throttling rather than a circuit-breaker failure.
+    #
     def initialize(
       client: nil,
       app: nil,
       mapper: Mapper::Identity,
       instrumenter: Instrumenter::Noop,
       circuit_breaker: CircuitBreaker::Noop,
       wait_seconds: DEFAULT_WAIT_SECONDS,
-      max_wait_seconds: DEFAULT_MAX_WAIT_SECONDS
+      max_wait_seconds: DEFAULT_MAX_WAIT_SECONDS,
+      required_consecutive_successes: DEFAULT_REQUIRED_CONSECUTIVE_SUCCESSES,
+      stable_recovery: false
     )
       @client           = client
       @app              = app
@@ -115,6 +127,8 @@ def initialize(
       @circuit_breaker  = circuit_breaker
       @wait_seconds     = wait_seconds
       @max_wait_seconds = max_wait_seconds
+      @required_consecutive_successes = required_consecutive_successes
+      @stable_recovery = stable_recovery
 
       yield self if block_given?
 
@@ -151,11 +165,11 @@ def initialize(
     #
     # - "throttler.called" each time this method is called
     # - "throttler.succeeded" when the stores were ok, before yielding the block
-    # - "throttler.waited" when the stores were not ok, after waiting
-    #   `wait_seconds`
-    # - "throttler.waited_too_long" when the stores were not ok, but the
-    #   thottler already waited at least `max_wait_seconds`, right before
-    #   raising `WaitedTooLong`
+    # - "throttler.waited" after waiting `wait_seconds` because stores were not
+    #   ok or recovery had not yet reached the consecutive-success threshold
+    # - "throttler.waited_too_long" when stores remain unavailable or recovery
+    #   stabilization exceeds `max_wait_seconds`, right before raising
+    #   `WaitedTooLong`
     # - "throttler.freno_errored" when there was an error with freno, before
     #   raising `ClientError`.
     # - "throttler.circuit_open" when the circuit breaker does not allow the
@@ -165,6 +179,8 @@ def throttle(context = nil, **options)
       store_names = mapper.call(context)
       instrument(:called, store_names: store_names)
       waited = 0
+      throttled = false
+      consecutive_successes = 0
 
       while true
         unless circuit_breaker.allow_request?
@@ -173,19 +189,47 @@ def throttle(context = nil, **options)
         end
 
         if all_stores_ok?(store_names, **options)
-          instrument(:succeeded, store_names: store_names, waited: waited)
-          circuit_breaker.success
-          break
+          consecutive_successes += 1
+          if !stable_recovery || !throttled || consecutive_successes >= required_consecutive_successes
+            instrument(
+              :succeeded,
+              store_names: store_names,
+              waited: waited,
+              consecutive_successes: consecutive_successes,
+              stable_recovery: stable_recovery
+            )
+            circuit_breaker.success
+            break
+          end
+        else
+          throttled = true
+          consecutive_successes = 0
         end
 
         if waited + wait_seconds > max_wait_seconds
-          instrument(:waited_too_long, store_names: store_names, waited: waited, max: max_wait_seconds)
-          circuit_breaker.failure
+          instrument(
+            :waited_too_long,
+            store_names: store_names,
+            waited: waited,
+            max: max_wait_seconds,
+            consecutive_successes: consecutive_successes,
+            required_consecutive_successes: required_consecutive_successes,
+            stable_recovery: stable_recovery
+          )
+          circuit_breaker.failure unless stable_recovery
           raise WaitedTooLong.new(waited_seconds: waited, max_wait_seconds: max_wait_seconds)
         else
           wait
           waited += wait_seconds
-          instrument(:waited, store_names: store_names, waited: waited, max: max_wait_seconds)
+          instrument(
+            :waited,
+            store_names: store_names,
+            waited: waited,
+            max: max_wait_seconds,
+            consecutive_successes: consecutive_successes,
+            required_consecutive_successes: required_consecutive_successes,
+            stable_recovery: stable_recovery
+          )
         end
       end
 
@@ -204,6 +248,12 @@ def validate_args
       unless max_wait_seconds > wait_seconds
         errors << "max_wait_seconds (#{max_wait_seconds}) has to be greather than wait_seconds (#{wait_seconds})"
       end
+      unless required_consecutive_successes.is_a?(Integer) && required_consecutive_successes.positive?
+        errors << "required_consecutive_successes (#{required_consecutive_successes}) must be a positive integer"
+      end
+      unless [true, false].include?(stable_recovery)
+        errors << "stable_recovery (#{stable_recovery}) must be true or false"
+      end
 
       raise ArgumentError, errors.join("\n") if errors.any?
     end