From a0a5cf7ecc43abf36fe74f1e6cd73c03ee824be2 Mon Sep 17 00:00:00 2001 From: iomgaa Date: Thu, 6 Aug 2026 10:55:51 -0400 Subject: [PATCH] fix: return 429 attempt time to the stall budget MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Independent verification found the first cut had swapped one bug for a worse one. The budgets were split by "did we send a request", so a 429 attempt counted as productive — but 429 is exempt from the retry budget, so its time burned neither budget. Against a queueing gateway that holds the request for the full timeout before answering 429, a call could hang for 301 attempts / 25.2 hours, measured, versus 301 seconds before the change. The split is now by which budget the time consumes: time that burns max_attempts is excluded from stall, time that does not (429 attempts included) belongs to stall. Measured again: back to one attempt / 301s. Only the chat loop needs this — embedding and ocr count 429 against max_attempts unconditionally, so the gap never existed there. The stall verdict moved into _stalled(), which both call sites had duplicated, to keep __call__ under the complexity gate. --- CHANGELOG.md | 4 +- research-wiki/ARCHITECTURE.md | 2 +- .../2026-08-06-issue8-stall-budget-design.md | 28 ++++++- src/polygateway/middleware/retry.py | 79 ++++++++++++++----- tests/unit/test_backpressure.py | 19 +++++ 5 files changed, 104 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f53bcb..6ba60bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,9 +8,9 @@ stall 判定改为非生产性等待口径(issue #8)。`timeout_s ≥ stall_wind ### 行为变更(**请先读这一条**) -- **stall 判定的"本地超窗"条件现在只累计非生产性等待**——429 退避、配额 wait 轮询、熔断冷却;真实尝试的耗时不再计入。两个预算自此正交: **真实尝试烧 `max_attempts`,其余一切等待烧 `stall_window_s`**。这与"429 不烧重试预算"同构。 +- **stall 判定的"本地超窗"条件现在只累计非生产性等待**——429 退避、配额 wait 轮询、熔断冷却;消耗重试预算的真实尝试不再计入。两个预算自此正交,划分依据是**谁消耗重试预算**: 烧 `max_attempts` 的时间不烧 `stall_window_s`,不烧 `max_attempts` 的时间(含 429 尝试本身)归 `stall_window_s` 治理。 - **`stall_window_s` 与 `timeout_s` 不再有任何耦合**,无需按 `timeout × retries` 放大。若你此前为绕开本 bug 把 `STALL_WINDOW_S` 调大过,现在可以回到默认值。 -- **单次调用的最坏耗时由 `stall_window_s` 抬升到 `max_attempts × timeout_s`**。这是重试预算恢复生效的正确表现,但如果你的上游有调用超时,请据此复核。 +- **单次调用的最坏耗时由 `stall_window_s` 抬升到约 `max_attempts × timeout_s`**(默认配置下 3 × `TIMEOUT_S`,再加各次退避)。这是重试预算恢复生效的正确表现,但如果你的上游有调用超时,请据此复核。**429 路径不会突破这个量级**: 429 虽免重试预算,但它的尝试耗时计入 stall 账,一次持满超时的 429 即耗尽 stall 窗口。 - 三条治理循环(chat / embedding / ocr)口径一致。**embedding 与 ocr 此前有同一缺陷**(经"先超时一次、再遇到无可用源"触发),issue 只记录了 chat 路径。 - 遥测收尾属"真实尝试"边界之内,**遥测抖动不会把一次调用推进 stalled 判决**。 diff --git a/research-wiki/ARCHITECTURE.md b/research-wiki/ARCHITECTURE.md index 2d6e18a..e44ce68 100644 --- a/research-wiki/ARCHITECTURE.md +++ b/research-wiki/ARCHITECTURE.md @@ -427,7 +427,7 @@ flowchart TB - `RedisLimiter`: 移植 CHSAnalyzer 六道闸——单条 Lua 原子检查全局并发/单源并发(ZSET 租约)/全局 RPM/单源 RPM/全局 TPM/单源 TPM;窗口 id 用 **Redis 服务器时钟**(TIME 命令)统一多进程口径。随实现移植契约测试。 - `InMemoryLimiter`: 同一契约的进程内实现(semaphore + 滑动窗口计数);单进程场景下语义等价。 - **配额满行为可配**: `wait`(等待,配 stall 判定——本地等待超窗 + 全局无进展超窗双条件才判卡死)或 `fail-fast`(立即抛)。 -- **stall 计时口径(2026-08-06 修正,issue #8,设计 `designs/2026-08-06-issue8-stall-budget-design.md`)**: 双条件的**条件 A 只累计非生产性等待**(429 退避、配额 wait 轮询、熔断冷却),真实尝试的耗时由 `StallClock.attempting()` 从 stall 账中扣除。原实现用墙钟总耗时,使真实尝试同时向重试预算与 stall 预算计费;而 stall 预算(默认 300s)小于重试预算(`max_attempts × timeout_s`),必然先耗尽——`timeout_s ≥ stall_window_s` 时一次超时即判 scope 死,`max_attempts` **静默失效**。修正后两个预算正交:**真实尝试烧 `max_attempts`,其余一切等待烧 `stall_window_s`**,与"429 不烧重试预算故 429 等待烧 stall 预算"同构。生产性边界即 `_attempt` 边界(含该次记账与遥测收尾),故遥测抖动不参与判死。`stall_window_s` 与 `timeout_s` 自此**无耦合**,无需按 `timeout × retries` 放大。三条治理循环(chat/embedding/ocr)共用 `middleware/retry.py` 的 `StallClock`。条件 B 的 `inf` 语义未动——新口径下"非生产性排队耗满窗口且 scope 从未出餐"判死本就正当。 +- **stall 计时口径(2026-08-06 修正,issue #8,设计 `designs/2026-08-06-issue8-stall-budget-design.md`)**: 双条件的**条件 A 只累计非生产性等待**(429 退避、配额 wait 轮询、熔断冷却),真实尝试的耗时由 `StallClock.attempting()` 从 stall 账中扣除。原实现用墙钟总耗时,使真实尝试同时向重试预算与 stall 预算计费;而 stall 预算(默认 300s)小于重试预算(`max_attempts × timeout_s`),必然先耗尽——`timeout_s ≥ stall_window_s` 时一次超时即判 scope 死,`max_attempts` **静默失效**。修正后两个预算正交,**划分依据是"谁消耗重试预算"而非"是否发出请求"**: 烧 `max_attempts` 的时间不烧 `stall_window_s`,不烧 `max_attempts` 的时间归 `stall_window_s`。**429 尝试因此也计入 stall 账**——它免重试预算,若其耗时又算生产性就两个预算都不烧,排队型网关(持满 timeout 才回 429)下调用可挂 25 小时(实施期独立验证实测,见设计 §3.6)。生产性边界即 `_attempt` 边界(含该次记账与遥测收尾),故遥测抖动不参与判死。`stall_window_s` 与 `timeout_s` 自此**无耦合**,无需按 `timeout × retries` 放大。三条治理循环(chat/embedding/ocr)共用 `middleware/retry.py` 的 `StallClock`。条件 B 的 `inf` 语义未动——新口径下"非生产性排队耗满窗口且 scope 从未出餐"判死本就正当。 - 全局活性信号: `mark_progress()`/`progress_age_s()`("最近一次出餐"时刻)供背压 stall 判定,移植 `CHSAnalyzer limiter.py:193`。 - **契约补强(2026-07-20,CHS 迁移缺口 G6)**: `settle()`/`release()` 幂等(重复调用无副作用);装配期守卫——`timeout_s ≤ permit 租约 TTL`(防租约先于请求过期)、`stall_window ≥ 最慢源 TTFT 上限`(防误判卡死;**issue #8 后为保守冗余**——TTFT 等待属生产性时间已不计入 stall,该误判在机制上不再可能,校验保留因其无害且不误拒合理配置),违反直接报错拒绝装配。降级方向细化(2026-07-20 M1): "报错不放行"适用于**准入侧**(try_acquire/try_enter 及选源路径消费的 source_stats/retry_after_s);已成功调用后的 settle/release 释放侧失败降级 warning——释放失败不构成放行,且不得掩盖主异常与取消。**勘误(2026-07-20 M2 设计,人类批准)**: 记账侧的 `record_success`/`record_failure`/`mark_progress` 同归此类——调用已真实完成,后端失败若冒泡会丢弃真实成功响应或掩盖原始尝试异常,故降级 warning(CHS 原版一律报错,此为有意反转;丢一次熔断记账最多延迟状态迁移且方向偏保守,epoch fencing 防污染)。 diff --git a/research-wiki/designs/2026-08-06-issue8-stall-budget-design.md b/research-wiki/designs/2026-08-06-issue8-stall-budget-design.md index 59f247c..b71119d 100644 --- a/research-wiki/designs/2026-08-06-issue8-stall-budget-design.md +++ b/research-wiki/designs/2026-08-06-issue8-stall-budget-design.md @@ -54,8 +54,10 @@ | 花在哪 | 烧哪个预算 | |---|---| -| 真实尝试(`_attempt` 内) | 重试预算 `max_attempts` | -| 其余一切等待 | stall 预算 `stall_window_s` | +| 真实尝试(`_attempt` 内),**429 除外** | 重试预算 `max_attempts` | +| 其余一切等待,**含 429 尝试本身** | stall 预算 `stall_window_s` | + +> **划分依据是"谁消耗重试预算",不是"是否发出了请求"**(2026-08-06 实施期订正,见 §3.6)。初稿按后者划分,使 429 尝试两个预算都不烧。 **"生产性"的边界即 `_attempt` 的边界**——包含该次尝试的记账(`record_success`/`mark_progress`)与遥测收尾,而不止于"等响应"。这是有意的:这些收尾是"尝试已有结论"之后的动作,不是"在等待重试机会"的停滞;把它们计入 stall 会让遥测抖动参与判死,与「遥测写失败降级不冒泡」所守的"遥测不得影响主路径判决"同精神。其耗时本也在毫秒量级。 @@ -129,9 +131,27 @@ async with clock.attempting(): # 包裹真实尝试 ### 3.5 429 饱和场景下兜底仍然有效(正确性验证) -修改后必须确认 stall 兜底没有被削弱:429 往返本身是生产性时间,不再计入 stall。 +修改后必须确认 stall 兜底没有被削弱。429 免预算使 `fails` 恒为 0,`retry.py` 的 `max(fails, 1)` 令退避恒定在 `backoff_base_s` 档(或取 `Retry-After` 提示的较大值),不随轮次增长。每轮构成为「一次 429 往返」+「一段恒定退避 sleep」,后者非生产性且每轮累加,`stalled_s` 单调逼近 `stall_window_s`,兜底有效。 -注意退避时长在纯 429 场景下**不随轮次增长**:429 免预算使 `fails` 恒为 0,`retry.py:243` 的 `max(fails, 1)` 令退避恒定在 `backoff_base_s` 档(或取 `Retry-After` 提示的较大值)。但这不影响结论——每轮的构成是「一次**快速失败**的 429 往返(网关立即拒绝,不耗 `timeout_s`,毫秒至秒级)」+「一段恒定退避 sleep(`backoff_base_s` 量级)」,后者是非生产性且**每轮都在累加**。故饱和期内非生产性时间仍占绝对多数,`stalled_s` 单调逼近 `stall_window_s`,兜底有效;触发时刻仅比修改前晚了"累计 429 往返耗时"的量级,可忽略。 +**但这个论证在初稿里依赖一个未加保护的假设**:「429 往返是快速失败,毫秒至秒级」。§3.6 处理它不成立的情形。 + +### 3.6 订正:429 尝试必须退还给 stall 账(2026-08-06 实施期,独立验证发现) + +**缺陷**:初稿按"是否发出请求"划分两个预算,于是 429 尝试的耗时算生产性。但 429 **不消耗重试预算**——它于是**两个预算都不烧**,掉进缝隙。§3.1 初稿声称的"无缝覆盖调用的全部时间"因此不成立。 + +**后果实测**(排队型网关:持满 `timeout_s` 才回 429,`timeout=300 / stall=300 / backoff_base=2 / rng=0`): + +| | 尝试次数 | 墙钟 | +|---|---|---| +| 修复前(main) | 1 | 301s | +| 初稿口径 | **301** | **90,601s ≈ 25.2 小时** | +| 订正后 | 1 | 301s | + +即初稿把一个 bug 换成了一个更严重的 bug——25 小时的挂起。 + +**订正**:划分依据改为**"谁消耗重试预算"**。429 免重试预算 → 429 尝试的耗时归 stall 治理,由 `StallClock.attempting()` yield 的句柄 `refund()` 退还。缝隙就此闭合,且这条规则比初稿更本质:两个预算按"由谁治理"划分,而非按"是否发出请求"这个表象。 + +**影响范围仅 chat**:embedding/ocr 无 429 免预算(无条件 `fails += 1`),429 照常烧重试预算,不存在缝隙,无需改动(与 §5.4 的分析一致)。 ## 4. 旧版行为审计(stall 子系统逐条) diff --git a/src/polygateway/middleware/retry.py b/src/polygateway/middleware/retry.py index b4941a9..0b3a1e7 100644 --- a/src/polygateway/middleware/retry.py +++ b/src/polygateway/middleware/retry.py @@ -75,11 +75,24 @@ def backoff_delay( return max(delay, retry_after) +class _Attempt: + """一次尝试的计时句柄;`refund()` 把它退还给 stall 账(见 `StallClock`)。""" + + __slots__ = ("productive",) + + def __init__(self) -> None: + self.productive = True + + def refund(self) -> None: + """该次尝试不消耗重试预算(429),故其耗时归 stall 治理而非重试治理。""" + self.productive = False + + class StallClock: """调用级 stall 计时器: 只累计非生产性等待(issue #8 设计 §3.1)。 - stall 预算治理的是"无人治理的等待"——429 退避、配额 wait 轮询、熔断冷却; - 真实尝试的耗时已由重试预算 `max_attempts` 治理,必须从 stall 账里扣除。 + **划分依据是"谁消耗重试预算"**,不是"是否发出了请求"。消耗 `max_attempts` + 的时间已被重试预算治理,从 stall 账扣除;不消耗它的时间无人治理,归 stall。 两者重叠计费正是 issue #8 的根因: stall 预算(默认 300s)小于重试预算 (3 × timeout_s),必然先耗尽,于是重试预算在超时场景下永远用不上。 @@ -87,7 +100,13 @@ class StallClock: "尝试已有结论"之后的动作,不是在等待重试机会;把它们计入 stall 会让遥测 抖动参与判死。 - 每次调用创建一个实例。严禁提升为实例属性: 并发调用共享会互相污染计时。 + **例外: 429 尝试须 `refund()`**。429 免重试预算(饱和期等待而非死亡),若其 + 耗时又算生产性,就掉进两个预算的缝隙——排队型网关持满 timeout 才回 429 时, + 每轮只有退避那一两秒进 stall 账,调用可挂满 `stall_window/backoff_base` 轮 + (实测 timeout=300/base=2 时达 25 小时)。退还后缝隙闭合。 + + 每次调用创建一个实例。严禁提升为实例属性: `_entered_at` 会固定在进程启动 + 时刻,使 `stalled_s()` 随进程运行时长单调增长,最终所有调用被误判 stalled。 模块级共享单元, EmbeddingClient 与 OcrClient 复用(同 `backoff_delay`)。 """ @@ -99,18 +118,20 @@ class StallClock: self._productive_s = 0.0 def stalled_s(self) -> float: - """非生产性等待累计秒数 = 调用总耗时 - 真实尝试耗时。""" + """非生产性等待累计秒数 = 调用总耗时 - 消耗重试预算的时间。""" return self._now() - self._entered_at - self._productive_s @contextlib.asynccontextmanager - async def attempting(self) -> AsyncIterator[None]: - """包裹一次真实尝试,其耗时记为生产性。""" + async def attempting(self) -> AsyncIterator[_Attempt]: + """包裹一次真实尝试,其耗时默认记为生产性(除非被 `refund()`)。""" + handle = _Attempt() started = self._now() try: - yield + yield handle finally: - # 只做算术, 不吞任何异常——CancelledError 逐字穿透(库铁律) - self._productive_s += self._now() - started + # 只做算术与取值, 不吞任何异常——CancelledError 逐字穿透(库铁律) + if handle.productive: + self._productive_s += self._now() - started def _demote_call_failures( @@ -196,6 +217,15 @@ class _Failed: immediate: bool +def _is_rate_limited(outcome: LLMResponse | _Failed) -> bool: + """429 = 服务端调度指令(gRPC pushback 语义,迭代 5): 按 Retry-After 退避但 + **不消耗重试预算**——饱和窗口里等待而非死亡;其余失败照常计数。 + + 因其免重试预算,该次尝试的耗时必须归 stall 治理(`StallClock` 的 refund)。 + """ + return isinstance(outcome, _Failed) and _failure_reason(outcome.exc) == "rate_limited" + + class RetryMW: """尝试编排器;时钟/睡眠/随机全部注入,纯确定性可测(P6)。""" @@ -252,10 +282,8 @@ class RetryMW: # 非生产性等待——真实尝试由重试预算治理,不再重复烧 stall 预算 clock = StallClock(self._now) while True: - # 调用级时间上限(迭代 5): 429 免预算后的兜底,防饱和期无限循环。 - # 与 _on_no_runnable 同款双条件(CHS 口径): 本地超窗且全局无进展才判死 - stall = self._bp.stall_window_s - if clock.stalled_s() > stall and await self._quota.progress_age_s() > stall: + # 调用级时间上限(迭代 5): 429 免预算后的兜底,防饱和期无限循环 + if await self._stalled(clock): raise AllSourcesExhausted( scope=self._scope, reason="stalled", @@ -266,13 +294,15 @@ class RetryMW: if picked is None: await self._on_no_runnable(gate_rejections, reasons, clock) continue - async with clock.attempting(): + async with clock.attempting() as attempt: outcome = await self._attempt(request, *picked, reasons, attempt_fails) + rate_limited = _is_rate_limited(outcome) + if rate_limited: + # 免了重试预算就得进 stall 账,否则这段耗时无人治理(见 StallClock) + attempt.refund() if isinstance(outcome, LLMResponse): return outcome - # 429 = 服务端调度指令(gRPC pushback 语义,迭代 5): 按 Retry-After - # 退避但不消耗重试预算——饱和窗口里等待而非死亡;其余失败照常计数 - if _failure_reason(outcome.exc) != "rate_limited": + if not rate_limited: fails += 1 if fails >= self._retry.max_attempts: raise AllSourcesExhausted( @@ -325,6 +355,16 @@ class RetryMW: await self._settle_and_release(permit, 0) return None, gate_rejections + async def _stalled(self, clock: StallClock) -> bool: + """双条件 stall 判死(CHS governance.py:270-281): 本地累计等待与全局 + 无进展**同时**超窗才判死——本地 monotonic 与后端时钟刻意不混用。 + + 本地一侧只计非生产性等待(issue #8,见 `StallClock`)。短路顺序有意为之: + 本地未超窗就不问后端,省一次 Redis 往返。 + """ + stall = self._bp.stall_window_s + return clock.stalled_s() > stall and await self._quota.progress_age_s() > stall + async def _on_no_runnable( self, gate_rejections: int, reasons: dict[str, str], clock: StallClock ) -> None: @@ -342,10 +382,7 @@ class RetryMW: retry_after_s=self._bp.poll_interval_s, per_source_reasons=reasons, ) - # 双条件 stall 判死(CHS governance.py:270-281): 本地累计等待与全局 - # 无进展**同时**超窗才判死——本地 monotonic 与后端时钟刻意不混用。 - stall = self._bp.stall_window_s - if clock.stalled_s() > stall and await self._quota.progress_age_s() > stall: + if await self._stalled(clock): names = tuple(s.name for s in self._sources) raise AllSourcesExhausted( scope=self._scope, diff --git a/tests/unit/test_backpressure.py b/tests/unit/test_backpressure.py index eb0fea8..8bb8f7c 100644 --- a/tests/unit/test_backpressure.py +++ b/tests/unit/test_backpressure.py @@ -328,6 +328,25 @@ class TestStallBudget: await mw(_REQ) assert ei.value.reason == "stalled" # 不是 retry_exhausted: 429 确实没烧重试预算 + async def test_slow_429_does_not_escape_both_budgets(self): + """排队型网关: 持满 timeout 才回 429。该耗时必须落进 stall 账。 + + 429 免重试预算, 所以它的耗时若又算生产性就**两个预算都不烧**——调用 + 会挂满 stall_window/backoff_base 轮。修复前实测 301 次尝试、25.2 小时; + 此处钉住"一轮 429 就把 stall 账推满"这个上界。 + """ + clock = FakeClock() + src, limiter = self._free_limiter(clock) + transport = ClockAdvancingTransport( + [(_STALL + 1, TransientError("429", status_code=429))] * 20, clock + ) + mw = _mw([src], limiter, [], clock=clock, sleep=BoundedSleep(), transport=transport) + with pytest.raises(AllSourcesExhausted) as ei: + await mw(_REQ) + assert ei.value.reason == "stalled" + # 一次持满超时的 429 即耗尽 stall 窗口, 不再无限排队 + assert len(transport.calls) <= 2 + async def test_cancel_inside_attempt_pierces(self): """取消发生在 `attempting()` 包裹内仍逐字穿透(库铁律)。""" clock = FakeClock()