Skip to content

Require stable Freno recovery - #37

Open
pradeepkintali wants to merge 4 commits into
mainfrom
pkintali/a1-consecutive-healthy-checks
Open

pradeepkintali wants to merge 4 commits into
mainfrom
pkintali/a1-consecutive-healthy-checks

Conversation

@pradeepkintali

@pradeepkintali pradeepkintali commented Sep 23, 2026 •

Copy link
Copy Markdown

Summary

  • Add stable_recovery: false as an explicit, default-off rollout switch.
  • Preserve first-success release and timeout circuit-breaker failure accounting
    for existing callers when upgrading the gem.
  • With stable_recovery: true, retain the initially healthy fast path, but
    require three consecutive passing checks after rejection. Another rejection
    resets recovery progress.
  • With the switch enabled, treat WaitedTooLong as normal sustained throttling,
    rather than a circuit-breaker failure. Transport/decision errors still fail
    the circuit breaker in both modes.
  • Include switch state and recovery progress in instrumentation.

Rollout and rollback

Wire stable_recovery to the consuming application's feature switch. This gem
does not automatically discover an application feature-flag service.

  1. Upgrade consumers with the switch false to preserve legacy behavior.
  2. Have app owners validate retry/defer handling and blocking-time budgets.
    The companion server/Puppet rollout uses a 30-second recovery window, whereas
    existing client max_wait_seconds defaults to 10. Keep the existing default;
    approve a larger budget only for appropriate workloads.
  3. Enable one reviewed consumer and monitor wait/timeout instrumentation,
    transport errors, protected work throughput, retries, and backlog.
  4. Expand only after validating sustained rejection, flapping, and recovery.

Set stable_recovery: false to restore both legacy release and timeout
circuit-breaker behavior. Setting only required_consecutive_successes: 1
does not restore the latter. A package-version rollback remains available.
Recovery progress is local to each throttle call.

Server-side required dependencies are independently gated by configuration in
github/freno#196 and the initially empty enforcement allowlist in
github/puppet#44086. Never bypass protection merely because a wait budget expires.

Validation

October 15 local validation, using Ruby 3.3 in an isolated container:

  • bundle exec rake test: 46 runs, 181 assertions, 0 failures/errors/skips.
  • bundle exec rubocop lib/freno/throttler.rb test/freno/throttler_test.rb: no offenses.
  • Added regression coverage for legacy first-success recovery, legacy timeout
    circuit accounting, and boolean switch validation. Existing stable-recovery
    tests now opt in explicitly.
  • git diff --check passed.

After a rejection, require consecutive healthy checks before releasing work and keep normal sustained throttling out of the circuit breaker failure path.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 0fbc396f-32f9-449a-8537-6264e8665c31
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 0fbc396f-32f9-449a-8537-6264e8665c31
@pradeepkintali
pradeepkintali marked this pull request as ready for review September 28, 2026 19:03
Copilot AI balanced review requested due to automatic review settings September 28, 2026 19:03

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Documentation remains contradictory, and the configurable recovery threshold lacks behavioral coverage.

Review effort: Balanced
Findings: 4 Low severity

Open (4)
What changed in this PR

Adds stable Freno recovery while preserving the healthy fast path.

Changes:

  • Requires consecutive healthy checks after throttling.
  • Updates circuit-breaker and recovery instrumentation behavior.
  • Adds validation, tests, documentation, and minor style cleanups.
File Description
lib/​freno/​throttler.rb Implements stable recovery and instrumentation.
test/​freno/​throttler_test.rb Tests recovery and circuit-breaker behavior.
README.md Documents the new configuration and semantics.
lib/​freno/​client.rb Simplifies range syntax.
lib/​freno/​client/​errors.rb Expands the error class declaration.
lib/​freno/​client/​preconditions.rb Expands the precondition error declaration.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread lib/freno/throttler.rb Outdated
Comment thread test/freno/throttler_test.rb
Correct the documented configuration and circuit-breaker behavior, clarify instrumentation semantics, and cover non-default recovery thresholds.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 0fbc396f-32f9-449a-8537-6264e8665c31
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: dea961fa-5766-415f-a0f5-2562c55a0385
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants