feat: add error ratio-based circuit breaking policy to api-breaker plugin by yeganeahmadnejad · Pull Request #13764 · apache/apisix

yeganeahmadnejad · 2026-07-30T11:26:18Z

What this PR does / why we need it

This PR implements error ratio-based circuit breaking (unhealthy-ratio policy) for the api-breaker plugin, providing more intelligent and adaptive circuit breaking behavior based on error rates within a sliding time window, rather than just consecutive failure counts.

Closes #12763

Context: reviving #12765

This is a revival of #12765 by @HaoTien, which was closed by the stale-bot for inactivity on 2026-07-24 despite:

Passing all CI checks (build, lint, docs, kubernetes-discovery, tars, etc.)
~20 rounds of review with @Baoyuantop, addressing sliding-window-reset, half-open-failure-fallback, and over-limit half-open call test coverage
A CHANGES_REQUESTED review from @moonming on 2026-03-16 asking for a summary of the design decisions made during review, which @HaoTien answered in full on 2026-03-17, followed by two more pings for re-review that went unanswered

The commit here carries over the final reviewed tree from #12765 unchanged (rebased onto current master; no functional changes), with commit authorship preserved for @HaoTien. Opening a fresh PR since the original was auto-closed and I don't have push access to reopen it or its branch.

Current Limitations

The existing failure count-based approach only considers consecutive failures
It doesn't account for the overall error rate in relation to total requests
May be too sensitive during low traffic periods or not sensitive enough during high traffic periods

New Features Added

Error ratio-based circuit breaking: New unhealthy-ratio policy that triggers circuit breaker based on error rate within a sliding time window
Configurable parameters: Support for error ratio threshold, minimum request threshold, sliding window size, etc.
Circuit breaker states: Explicit CLOSED, OPEN, and HALF_OPEN states
Backward compatibility: Existing configurations continue to work unchanged (unhealthy-count remains the default policy)

New Configuration Parameters

Parameter	Type	Default	Description
`policy`	string	`"unhealthy-count"`	Circuit breaker policy
`unhealthy.error_ratio`	number	`0.5`	Error rate threshold (0-1) to trigger circuit breaker
`unhealthy.min_request_threshold`	integer	`10`	Minimum requests needed before evaluating error rate
`unhealthy.sliding_window_size`	integer	`300`	Sliding window size in seconds for error rate calculation
`unhealthy.half_open_max_calls`	integer	`3`	Number of permitted calls in half-open state
`healthy.success_ratio`	number	`0.6`	Success rate threshold to close circuit breaker from half-open state

Example Configuration

{
  "plugins": {
    "api-breaker": {
      "break_response_code": 503,
      "policy": "unhealthy-ratio",
      "max_breaker_sec": 60,
      "unhealthy": {
        "http_statuses": [500, 502, 503, 504],
        "error_ratio": 0.5,
        "min_request_threshold": 10,
        "sliding_window_size": 300,
        "half_open_max_calls": 3
      },
      "healthy": {
        "http_statuses": [200, 201, 202],
        "success_ratio": 0.6
      }
    }
  }
}

Types of changes

New feature (non-breaking change which adds functionality)
Documentation update

How Has This Been Tested?

Schema validation tests for new parameters
Functional tests for error ratio calculation
Circuit breaker state transition tests (CLOSED → OPEN → HALF_OPEN → CLOSED/OPEN)
Sliding window expiration/reset tests
Half-open failure fallback tests
Over-limit half-open call rejection tests
Backward-compatibility tests for the existing unhealthy-count policy

This is the same test coverage that was previously reviewed and passed full CI on #12765.

@Baoyuantop

…ugin Add a new `unhealthy-ratio` policy to the api-breaker plugin, using a sliding time window to trip the breaker based on error rate rather than raw consecutive failure counts. This is more resilient to traffic-volume swings than the existing `unhealthy-count` policy, which remains the default and is unchanged for backward compatibility. The breaker now models CLOSED / OPEN / HALF_OPEN states explicitly under the ratio policy: it opens once error rate and minimum request volume thresholds are met within the window, waits out `max_breaker_sec`, then allows a limited number of half-open test calls before fully closing (on sufficient success rate) or reopening (on any failure). This revives apache#12765, which implemented and addressed all review feedback from @Baoyuantop and @moonming but was auto-closed for inactivity before a final merge decision. Rebased onto current master; no functional changes from the last reviewed revision of that PR. Co-authored-by: tianhao53 <tianhao53@jd.com>

membphis

Blocking findings are noted inline.

membphis · 2026-07-30T13:32:27Z

+                core.log.info("Circuit breaker transitioned from OPEN to HALF_OPEN")
+
+                -- Clean up transition lock
+                shared_buffer:delete(transition_key)


P1 — half_open_max_calls is not enforced under concurrency. The request that transitions OPEN -> HALF_OPEN returns here without incrementing half_open_calls, and deleting the lock immediately lets requests that already observed OPEN acquire it again. The regression test with a limit of 2 currently expects three 200 responses. Please make the transition atomic, count the transition request, and decide recovery only after all admitted probes complete.

Fixed in 6f9af8c. The winning request now falls through into the same admission code path used by later probes instead of returning early, so it's counted as probe #1. The transition lock is left to expire on its own 1s TTL instead of being deleted immediately, so a request racing in during that window is conservatively rejected as still-open rather than risking another uncounted admission.

membphis · 2026-07-30T13:32:27Z

+    local unhealthy_key = gen_unhealthy_key(ctx)
+
+    shared_buffer:set(window_start_key, current_time)
+    shared_buffer:set(total_requests_key, 0)


P1 — This is a tumbling-window reset, not a sliding window, and it is racy across workers. Concurrent requests that observe an expired or missing start time can each zero the counters, losing increments from completed requests; recent failures just before the boundary are also discarded wholesale. Please use bounded time buckets with atomic updates and add boundary-concurrency coverage.

Fixed in 6f9af8c. Replaced the single reset-on-expiry counter pair with fixed time buckets named by their own epoch (cb-breq--<bucket_size>- / cb-bfail--<bucket_size>-). Aging out old data is now implicit — an epoch outside the last N buckets just isn't summed — so there's no shared 'is it time to reset' decision for concurrent requests to race on, and each increment is a single atomic shared_buffer:incr. Added a harness-based regression covering window aging (Scenario 6 in the linked test run) alongside the existing sliding-window .t test.

membphis · 2026-07-30T13:32:27Z

+end
+
+local function gen_total_requests_key(ctx)
+    return "cb-total-" .. core.request.get_host(ctx) .. ctx.var.uri


P1 — These statistics are keyed by the raw URI and have no normal TTL. On a wildcard or high-cardinality route, every distinct path creates persistent cb-total, cb-window, and failure entries, which can fill plugin-api-breaker and evict unrelated breaker state. Please key by a stable route/service configuration identity and expire or otherwise bound the buckets.

Fixed in 6f9af8c. Ratio-policy keys are now scoped by conf_type/conf_id/conf_version (gen_breaker_id) instead of host+URI — same approach limit-count/limit-conn use to bound cardinality and to start fresh on a config change. Every key now also carries a bounded TTL (get_state_ttl / get_bucket_ttl).

bzp2010 · 2026-07-30T13:57:55Z

+            shared_buffer:delete(gen_half_open_calls_key(ctx))
+            shared_buffer:delete(gen_half_open_success_key(ctx))
+        end
+    elseif is_success then


https://github.com/apache/apisix/pull/13764/changes#diff-ae3e886348fc6297466ddb8cf3bd153451146d7bbb5ff475d0fc1f684a235ea4R451-R457

If the recorded response status code is neither a success nor a failure, the number of half-open requests may be exhausted. This can result in a permanent API outage.

In any case, https://github.com/apache/apisix/pull/13764/changes#diff-ae3e886348fc6297466ddb8cf3bd153451146d7bbb5ff475d0fc1f684a235ea4R451
The access phase has already incremented the half-open counter, but the log phase does not reset the increment value for exceptions.

As a result, the state may get stuck in a "half-open" state, and new requests will always be rejected due to the "half-open" request limit. However, the log phase will never update the state machine based on unmarked status codes.

Fixed in 6f9af8c. Introduced a dedicated half-open 'completed' counter, incremented for every probe that reaches the log phase regardless of how its status is classified (success/failure/neither). The close-vs-reopen decision now fires once completed >= half_open_max_calls, so a run of unclassified status codes can no longer strand the breaker in HALF_OPEN forever. Verified in harness Scenario 5 (two unclassified 404 probes still resolve the half-open evaluation).

bzp2010 · 2026-07-30T14:03:30Z

+    shared_buffer:set(state_key, state)
+    shared_buffer:set(last_change_key, current_time)


The shdict setting in the new code never specifies a TTL, so shdict may gradually fill up and stop accepting new entries, which could lead to undefined behavior or, at the very least, cause the functionality to fail.

Fixed in 6f9af8c. Every shared_buffer:set/incr call in the ratio path now passes an exptime (get_state_ttl for state/half-open bookkeeping, get_bucket_ttl for window buckets).

bzp2010 · 2026-07-30T14:09:11Z

+
+            if transition_success then
+                -- Transition to HALF_OPEN
+                set_circuit_breaker_state(ctx, HALF_OPEN)


The request that sets the half-open state does not increment the half-open counter itself; theoretically, that request is already in and is to the half-open state, so half_open_max_calls actually allows for n + 1 half-open requests.

The return statement below skips the increment code in https://github.com/apache/apisix/pull/13764/changes#diff-ae3e886348fc6297466ddb8cf3bd153451146d7bbb5ff475d0fc1f684a235ea4R451.

Fixed in 6f9af8c, same root cause as your comment above on the half_open_max_calls enforcement — the transitioning request now falls through into the shared admission/increment path instead of returning early, so it counts as probe #1 rather than granting an extra n+1'th slot.

bzp2010 · 2026-07-30T14:19:33Z

+| unhealthy.sliding_window_size                          | integer        | 否     | 300    | [10, 3600]      | 滑动时间窗口大小，以秒为单位。用于统计错误率的时间范围。                                 |
+| unhealthy.half_open_max_calls | integer        | 否     | 3      | [1, 20]         | 在半开启状态下允许通过的请求数量。用于测试服务是否恢复正常。                             |
+| healthy.http_statuses                                  | array[integer] | 否     | [200]  | [200, ..., 499] | 上游服务处于健康状态时的 HTTP 状态码。                                                   |
+| healthy.successes                                      | integer        | 否     | 3      | >=1             | 上游服务触发健康状态的连续正常请求次数。                                                 |


The "healthy.successes" does not exist.

Fixed in 6f9af8c — removed the stray healthy.successes row from the unhealthy-ratio table (that field only applies to the unhealthy-count policy).

@membphis

Fixes three P1 concurrency/correctness issues raised by @membphis and @bzp2010 on the unhealthy-ratio policy: - The request that transitioned the breaker from OPEN to HALF_OPEN returned without counting itself as a probe, and immediately deleted the transition lock, letting other racing requests slip through uncounted too. half_open_max_calls was effectively unenforceable under concurrency. The winning request now falls through into the same admission path as later probes, and the lock is left to expire on its own short TTL instead of being deleted early. - The sliding window was a single counter pair reset by whichever request happened to notice it had expired, racing with concurrent readers/writers and silently discarding data at the boundary. Replaced with fixed time buckets named by their own epoch, so aging out old data is implicit (an epoch outside the window is just not summed) rather than an explicit, racy reset. - Ratio-policy shared-dict keys were scoped by host+URI with no TTL, so a high-cardinality or wildcard route could grow unbounded state and evict unrelated breaker entries. Keys are now scoped by the plugin's configuration identity (conf_type/conf_id/conf_version, the same approach limit-count and limit-conn use) and everything now carries a bounded TTL. Also fixes a permanent half-open lockup: a probe response that matched neither the configured healthy nor unhealthy status list previously left the breaker stuck in HALF_OPEN forever, since only classified responses advanced the state machine while access() had already consumed one of the limited probe slots. A dedicated "completed" counter (incremented for every probe that reaches the log phase, independent of classification) now drives the close/reopen decision. Removes a stray `healthy.successes` row that had leaked into the `unhealthy-ratio` docs table in the Chinese docs (that field belongs only to the unhealthy-count policy). Updates the half-open concurrency regression test, which had pinned the buggy behavior (3 admissions against a limit of 2, and a stale pre-recovery failure count corrupting the next open/close cycle) as expected output. Verified with a standalone harness that mocks ngx.shared/core and replays the state machine (including true concurrent access() calls before any log() resolves) across open/half-open/close/reopen and window-aging scenarios.

yeganeahmadnejad · 2026-07-30T19:13:21Z

Thanks @membphis and @bzp2010 for the thorough review — all 7 findings addressed in 6f9af8c (replied inline on each thread with specifics):

half_open_max_calls not enforced under concurrency / n+1 admission: the OPEN→HALF_OPEN transitioning request now counts itself as the first probe instead of returning early; the transition lock expires on its own short TTL instead of being deleted immediately.
Racy tumbling-window reset: replaced with fixed time buckets named by their own epoch, so aging out old data doesn't require any request to "reset" shared counters.
Unbounded per-URI key cardinality + no TTL: ratio-policy state is now scoped by conf_type/conf_id/conf_version (matching limit-count/limit-conn) with bounded TTLs throughout.
Permanent half-open lockup on unclassified status codes: added a dedicated "completed" counter that advances regardless of classification, so the close/reopen decision can't stall waiting for a status that will never come.
docs/zh stray healthy.successes row: removed from the unhealthy-ratio table.

Also updated the half-open concurrency test (t/plugin/api-breaker2.t), which had pinned the buggy behavior (3 admissions against a limit of 2) as expected output.

Verified with a standalone harness that mocks ngx.shared/core and replays the state machine — including genuinely concurrent access() calls arriving before any log() resolves — across open/half-open/close/reopen and window-aging scenarios. Happy to take another look at anything.

dosubot Bot added size:XXL This PR changes 1000+ lines, ignoring generated files. enhancement New feature or request plugin labels Jul 30, 2026

membphis reviewed Jul 30, 2026

View reviewed changes

bzp2010 reviewed Jul 30, 2026

View reviewed changes

		shared_buffer:set(state_key, state)
		shared_buffer:set(last_change_key, current_time)

Uh oh!

Conversation

yeganeahmadnejad commented Jul 30, 2026

What this PR does / why we need it

Context: reviving #12765

Current Limitations

New Features Added

New Configuration Parameters

Example Configuration

Types of changes

How Has This Been Tested?

Uh oh!

membphis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yeganeahmadnejad commented Jul 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants