bzp2010 commented on code in PR #13764:
URL: https://github.com/apache/apisix/pull/13764#discussion_r3683335111


##########
apisix/plugins/api-breaker.lua:
##########
@@ -133,13 +267,67 @@ local _M = {
     schema = schema,
 }
 
-
 function _M.check_schema(conf)
     return core.schema.check(schema, conf)
 end
 
+-- Circuit breaker state management functions
+local function get_circuit_breaker_state(ctx)
+    local state_key = gen_state_key(ctx)
+    local state, err = shared_buffer:get(state_key)
+    if err then
+        core.log.warn("failed to get circuit breaker state: ", err)
+        return CLOSED
+    end
+    return state or CLOSED
+end
+
+local function set_circuit_breaker_state(ctx, state)
+    local state_key = gen_state_key(ctx)
+    local last_change_key = gen_last_state_change_key(ctx)
+    local current_time = ngx.time()
 
-function _M.access(conf, ctx)
+    shared_buffer:set(state_key, state)
+    shared_buffer:set(last_change_key, current_time)

Review Comment:
   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.



##########
apisix/plugins/api-breaker.lua:
##########
@@ -195,8 +383,145 @@ function _M.access(conf, ctx)
     return
 end
 
+-- Ratio-based circuit breaker
+local function ratio_based_access(conf, ctx)
+    -- Check and reset sliding window first to ensure consistent state
+    check_and_reset_window(ctx, conf)
 
-function _M.log(conf, ctx)
+    local current_state = get_circuit_breaker_state(ctx)
+    local current_time = ngx.time()
+
+    -- Handle OPEN state
+    if current_state == OPEN then
+        local last_change_key = gen_last_state_change_key(ctx)
+        local last_change_time, err = shared_buffer:get(last_change_key)
+        if err then
+            core.log.warn("failed to get last change time: ", err)
+            return conf.break_response_code or 503,
+                    conf.break_response_body or "Service temporarily 
unavailable"
+        end
+
+        local wait_duration = conf.max_breaker_sec or 300
+        if last_change_time and (current_time - last_change_time) >= 
wait_duration then
+            -- Use atomic operation to ensure only one request transitions to 
HALF_OPEN
+            local transition_key = "cb-transition-" .. 
core.request.get_host(ctx) .. ctx.var.uri
+            local transition_success
+            transition_success, err = shared_buffer:add(transition_key, 1, 1)
+
+            if err then
+                core.log.warn("failed to add transition lock: ", err)
+            end
+
+            if transition_success then
+                -- Transition to HALF_OPEN
+                set_circuit_breaker_state(ctx, HALF_OPEN)

Review Comment:
   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`.



##########
docs/zh/latest/plugins/api-breaker.md:
##########
@@ -30,34 +29,70 @@ description: 本文介绍了 Apache APISIX api-breaker 插件的相关操作,
 
 `api-breaker` 插件实现了 API 熔断功能,从而帮助我们保护上游业务服务。
 
+该插件支持两种熔断策略:
+
+- **按错误次数熔断(`unhealthy-count`)**:当连续失败次数达到阈值时触发熔断
+- **按错误比例熔断(`unhealthy-ratio`)**:当在滑动时间窗口内的错误率达到阈值时触发熔断
+
 :::note 注意
 
-关于熔断超时逻辑,由代码逻辑自动按**触发不健康状态**的次数递增运算:
+**按错误次数熔断(`unhealthy-count`)**:
 
 当上游服务返回 `unhealthy.http_statuses` 配置中的状态码(默认为 `500`),并达到 `unhealthy.failures` 
预设次数时(默认为 3 次),则认为上游服务处于不健康状态。
 
 第一次触发不健康状态时,熔断 2 秒。超过熔断时间后,将重新开始转发请求到上游服务,如果继续返回 `unhealthy.http_statuses` 
状态码,记数再次达到 `unhealthy.failures` 预设次数时,熔断 4 秒。依次类推(2,4,8,16,……),直到达到预设的 
`max_breaker_sec`值。
 
 当上游服务处于不健康状态时,如果转发请求到上游服务并返回 `healthy.http_statuses` 配置中的状态码(默认为 `200`),并达到 
`healthy.successes` 次时,则认为上游服务恢复至健康状态。
 
+**按错误比例熔断(`unhealthy-ratio`)**:
+
+该策略基于滑动时间窗口统计错误率。当在 `sliding_window_size` 时间窗口内,请求总数达到 `min_request_threshold` 
且错误率超过 `error_ratio` 时,熔断器进入开启状态,持续 `max_breaker_sec` 秒。
+
+熔断器有三种状态:
+
+- **关闭(CLOSED)**:正常转发请求
+- **开启(OPEN)**:直接返回熔断响应,不转发请求
+- **半开启(HALF_OPEN)**:允许少量请求通过以测试服务是否恢复
+
 :::
 
 ## 属性
 
-| 名称                    | 类型           | 必选项 | 默认值     | 有效值          | 描述     
                        |
-| ----------------------- | -------------- | ------ | ---------- | 
--------------- | -------------------------------- |
-| break_response_code     | integer        | 是   |           | [200, ..., 599] 
| 当上游服务处于不健康状态时返回的 HTTP 错误码。                 |
-| break_response_body     | string         | 否   |           |                 
| 当上游服务处于不健康状态时返回的 HTTP 响应体信息。                   |
-| break_response_headers  | array[object]  | 否   |           | 
[{"key":"header_name","value":"can contain Nginx $var"}] | 当上游服务处于不健康状态时返回的 
HTTP 响应头信息。该字段仅在配置了 `break_response_body` 属性时生效,并能够以 `$var` 的格式包含 APISIX 变量,比如 
`{"key":"X-Client-Addr","value":"$remote_addr:$remote_port"}`。 |
-| max_breaker_sec         | integer        | 否   | 300        | >=3            
 | 上游服务熔断的最大持续时间,以秒为单位。                 |
-| unhealthy.http_statuses | array[integer] | 否   | [500]      | [500, ..., 
599] | 上游服务处于不健康状态时的 HTTP 状态码。               |
-| unhealthy.failures      | integer        | 否   | 3          | >=1            
 | 上游服务在一定时间内触发不健康状态的异常请求次数。 |
-| healthy.http_statuses   | array[integer] | 否   | [200]      | [200, ..., 
499] | 上游服务处于健康状态时的 HTTP 状态码。                 |
-| healthy.successes       | integer        | 否   | 3          | >=1            
 | 上游服务触发健康状态的连续正常请求次数。   |
+| 名称                   | 类型          | 必选项 | 默认值            | 有效值              
                                                                                
                                                                                
                                                                                
             | 描述                                                               
                    |
+| ---------------------- | ------------- | ------ | ----------------- | 
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 | 
--------------------------------------------------------------------------------------
 |
+| break_response_code    | integer       | 是     |                   | [200, 
..., 599]                                                                       
                                                                                
                                                                                
                           | 当上游服务处于不健康状态时返回的 HTTP 错误码。                         
                |
+| break_response_body    | string        | 否     |                   |         
                                                                                
                                                                                
                                                                                
                         | 当上游服务处于不健康状态时返回的 HTTP 响应体信息。                         
            |
+| break_response_headers | array[object] | 否     |                   | 
[{"key":"header_name","value":"can contain Nginx $var"}] | 当上游服务处于不健康状态时返回的 
HTTP 响应头信息。该字段仅在配置了 `break_response_body` 属性时生效,并能够以 `$var` 的格式包含 APISIX 
变量,比如`{"key":"X-Client-Addr","value":"$remote_addr:$remote_port"}`。 |           
                                                                             |
+| max_breaker_sec        | integer       | 否     | 300               | >=3     
                                                                                
                                                                                
                                                                                
                         | 上游服务熔断的最大持续时间,以秒为单位。适用于两种熔断策略。                       
    |
+| policy                 | string        | 否     | "unhealthy-count" | 
["unhealthy-count", "unhealthy-ratio"]                                          
                                                                                
                                                                                
                                 | 熔断策略。`unhealthy-count` 
为按错误次数熔断,`unhealthy-ratio` 为按错误比例熔断。 |
+
+### 按错误次数熔断(policy = "unhealthy-count")
+
+| 名称                    | 类型           | 必选项 | 默认值 | 有效值          | 描述         
                                      |
+| ----------------------- | -------------- | ------ | ------ | --------------- 
| -------------------------------------------------- |
+| unhealthy.http_statuses | array[integer] | 否     | [500]  | [500, ..., 599] 
| 上游服务处于不健康状态时的 HTTP 状态码。           |
+| unhealthy.failures      | integer        | 否     | 3      | >=1             
| 上游服务在一定时间内触发不健康状态的异常请求次数。 |
+| healthy.http_statuses   | array[integer] | 否     | [200]  | [200, ..., 499] 
| 上游服务处于健康状态时的 HTTP 状态码。             |
+| healthy.successes       | integer        | 否     | 3      | >=1             
| 上游服务触发健康状态的连续正常请求次数。           |
+
+### 按错误比例熔断(policy = "unhealthy-ratio")
+
+| 名称                                                   | 类型           | 必选项 | 
默认值 | 有效值          | 描述                                                         
                            |
+| ------------------------------------------------------ | -------------- | 
------ | ------ | --------------- | 
----------------------------------------------------------------------------------------
 |
+| unhealthy.http_statuses                                | array[integer] | 否  
   | [500]  | [500, ..., 599] | 上游服务处于不健康状态时的 HTTP 状态码。                         
                        |
+| unhealthy.error_ratio                                  | number         | 否  
   | 0.5    | [0, 1]          | 触发熔断的错误率阈值。例如 0.5 表示错误率达到 50% 时触发熔断。            
               |
+| unhealthy.min_request_threshold                        | integer        | 否  
   | 10     | >=1             | 在滑动时间窗口内触发熔断所需的最小请求数。只有请求数达到此阈值时才会评估错误率。        
 |
+| 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             | 上游服务触发健康状态的连续正常请求次数。                            
                     |

Review Comment:
   The "healthy.successes" does not exist.



##########
apisix/plugins/api-breaker.lua:
##########
@@ -264,4 +589,107 @@ function _M.log(conf, ctx)
     return
 end
 
+-- Ratio-based logging
+local function ratio_based_log(conf, ctx)
+    local upstream_status = core.response.get_upstream_status(ctx)
+    if not upstream_status then
+        return
+    end
+
+    local current_state = get_circuit_breaker_state(ctx)
+
+    -- Increment total request counter
+    local total_requests_key = gen_total_requests_key(ctx)
+    local total_requests, err = shared_buffer:incr(total_requests_key, 1, 0)
+    if err then
+        core.log.warn("failed to increment total requests: ", err)
+    end
+
+    -- Handle response based on status
+    local is_failure = core.table.array_find(conf.unhealthy.http_statuses, 
upstream_status)
+    local is_success = not is_failure and
+            core.table.array_find(conf.healthy.http_statuses, upstream_status)
+
+    if is_failure then
+        -- Increment failure counter
+        local unhealthy_key = gen_unhealthy_key(ctx)
+        local unhealthy_count, err = shared_buffer:incr(unhealthy_key, 1, 0)
+        if err then
+            core.log.warn("failed to increment unhealthy count: ", err)
+        end
+
+        core.log.info("Request failed - status: ", upstream_status,
+            " total: ", total_requests,
+            " failures: ", unhealthy_count)
+
+        -- If in HALF_OPEN state and got a failure, immediately go back to OPEN
+        if current_state == HALF_OPEN then
+            set_circuit_breaker_state(ctx, OPEN)
+            core.log.warn("Circuit breaker returned to OPEN state due to 
failure in HALF_OPEN")
+            -- Clean up half-open counters
+            shared_buffer:delete(gen_half_open_calls_key(ctx))
+            shared_buffer:delete(gen_half_open_success_key(ctx))
+        end
+    elseif is_success then

Review Comment:
   
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.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to