Prhub

#37494 [Bugfix] Skip absent radix lock during cache cleanup

原始 PR 作者 YAMY1234 合并时间 2026-09-02 13:50 文件变更 2 提交数 1 评论 10 代码增减 +48 / -1

执行摘要

修复合成请求清理时跳过不存在的 radix 节点锁导致的 CI 失败。

根据 PR body,PP 动态分块分析会创建拥有 KV 内存但不匹配或锁定 radix 树节点的合成请求。当前统一缓存的 no-insert 清理路径会无条件调用锁释放函数,导致在管道层级完成同步前中止分析,并引发 CI 失败(关联 PR #31470)。

这是一个小的、必要的 bugfix,解决了明确的 CI 失败和功能逻辑缺陷。核心开发者应精读以理解在统一缓存清理路径中处理无锁合成请求的边界条件处理方式。

讨论亮点

虽然没有正式的 review 评论讨论,但从作者与 GitHub Actions 的交互中可以看到,作者通过多次使用 /rerun-test/rerun-group 命令,重跑了多个相关的单元测试和 radix 缓存集成测试(如 test_unified_radix_cache_kl_full.py, test_unified_radix_cache_kl_swa.py 等),所有测试均通过,确认了修复的有效性和兼容性。

实现拆解

  1. 修改核心清理逻辑:在 python/sglang/srt/mem_cache/unified_radix_cache.pycache_finished_req 方法中,在调用 _dec_req_lock 释放锁之前,增加 if req.last_node is not None: 的条件判断。这确保只有当请求确实持有了 radix 树节点锁时,才执行释放操作,防止对无锁请求进行操作而引发错误。
  2. 编写回归测试:新增 test/registered/unit/mem_cache/test_unified_radix_lock_ref.py 文件,通过 unittestMagicMock 构建一个模拟的 UnifiedRadixCache 和请求对象。测试验证当 req.last_nodeNone 时,cache_finished_req 方法会正确调用 free_kv_row 释放 KV 行,但不会调用 _dec_req_lock,从而精确覆盖修复的边界条件。
文件 模块 状态 重要度
python/sglang/srt/mem_cache/unified_radix_cache.py 缓存管理 modified 5.07
test/registered/unit/mem_cache/test_unified_radix_lock_ref.py 缓存锁测试 added 6.43

关键符号

cache_finished_req test_no_insert_without_last_node_skips_lock_release

关键源码片段

python/sglang/srt/mem_cache/unified_radix_cache.py core-logic

包含核心修复的源码文件,在缓存清理的关键方法中增加了对请求锁持有状态的检查。

# ... 前面代码省略 ...
        else:
            self.free_kv_row(req.kv, [(req.kv.cache_protected_len, kv_len_to_handle)])
​
        # Synthetic profiling requests may own KV without locking a tree node.
        # 仅当请求确实持有了 radix 树节点锁时,才执行释放操作,
        # 防止对 PP 动态分块分析创建的无锁合成请求进行操作而引发错误。
        if req.last_node is not None:
            self._dec_req_lock(req, skip_swa=req.swa_prefix_lock_released)
​
        if is_insert and result is not None and result.last_device_node is not None:
            req.last_node = result.last_device_node
​
        # cleanup
        for comp in self._components_tuple:
            comp.cleanup_after_caching_req(
                req, is_finished=True, insert_result=result, insert_params=insert_params
            )
# ... 后续代码省略 ...

评论区精华

修复验证与测试重跑 正确性

作者在提交修复后,多次通过 `/rerun-test` 和 `/rerun-group` 指令重跑了相关的单元测试(`test_unified_radix_lock_ref.py`)和 radix 缓存集成测试套件(`radix_cache/unified_radix_tree` 组),以确保修复正确且未引入回归。

结论:所有重跑的测试(包括 CPU 单元测试和多种 GPU 环境下的 radix 缓存测试)均通过,确认修复有效且兼容。 · 已解决

风险与影响

  1. 逻辑变更风险:修改的是缓存清理路径上的核心锁释放逻辑。虽然改动很小(增加一个 if 判断),但影响的是资源释放的关键步骤。如果 last_node 的语义在正常请求流程中发生变化,可能影响锁的正确释放。
  2. 测试覆盖风险:新增的单元测试是 CPU-only 的 mock 测试,未能在 GPU 环境中验证对真实 radix 树操作的影响。不过,作者通过重跑多个已有的 GPU radix 缓存测试套件(见评论区)进行了集成验证,部分缓解了此风险。
  1. 用户影响:修复了使用 PP 动态分块分析功能时可能出现的 CI 中断问题,使该功能路径恢复稳定。对于直接使用动态分块分析的用户,这是一个必要的修复。
  2. 系统影响:增强了统一缓存清理路径对异常/合成请求的鲁棒性,减少了因边界条件未处理导致的流程中断。变更范围被严格限定在 cache_finished_req 方法的单一条件分支,影响面可控。
  3. 团队影响:为类似的无锁请求处理场景提供了清晰的修复模式,并补充了对应的单元测试用例。
核心清理路径变更 依赖 last_node 语义正确性

关联 Issue

未识别关联 Issue

当前没有检测到明确关联的 Issue 链接,后续同步到相关引用后会出现在这里。

完整报告

参与讨论