Skip to content

【No.7】feat(lora): immutable version publication, session binding and safe reclamation - #382

Open
ying123ww wants to merge 19 commits into
redai-studio:mainfrom
ying123ww:feat/task7-immutable-lora-versioning
Open

ying123ww wants to merge 19 commits into
redai-studio:mainfrom
ying123ww:feat/task7-immutable-lora-versioning

Conversation

@ying123ww

Copy link
Copy Markdown

What

实现 No.7:不可变 LoRA 版本的在线发布、Session 绑定与安全回收。

本 PR 在现有 Relax + SGLang LoRA adapter 路径上增加版本化 lifecycle,使在线权重更新期间:

  • 旧 Agent Session 继续使用首次生成时绑定的旧版本;
  • 新版本只有在两个目标 SGLang 引擎均确认就绪后才成为新的默认版本;
  • 新 Session 在 commit 后绑定新版本;
  • publication 期间不需要全局暂停 generation,也不需要清空 SGLang KV cache;
  • 旧版本只有在 Session 引用释放、后端在途请求结束,并且两个引擎均确认物理卸载后,才真正释放容量。

RFC:Relates to #369


Why

现有 LoRA adapter 更新使用固定名称覆盖,无法表达:

old Session -> A
new Session -> B

这种 Agentic / fully-async 场景下需要的 policy consistency。

如果 Session 的多个工具轮次、retry 或 abort/resume 跨越一次权重更新,直接使用“当前最新 adapter”会使同一个 Session 在不同轮次实际执行不同 policy。

本 PR 将三个阶段明确分开:

load candidate
      ↓
publish default
      ↓
reclaim retired version

并分别定义对应的 ownership:

Relax Session ref
    -> 决定什么时候允许开始 reclaim

SGLang native request ref
    -> 决定什么时候允许完成 physical unload

因此 Relax 不需要复制一套后端 request lifecycle,同时又能保证旧 Session 和已经被 SGLang 接纳的请求不会被提前卸载。


How

1. 不可变版本身份

每次 publication 使用:

version_key = (deployment_epoch, version_id)

lora_name =
  relax_policy_lora@<epoch>-<version_id>-<digest_prefix>

request.lora_path = lora_name

version_id 表示一次 publication event,digest 表示该版本的实际内容,两者不再混为同一个 identity。

Registry 使用:

allocate(digest, version_id=...)

维护 (deployment_epoch, version_id) -> digest。

语义为:

  • 同 ID + 同 digest:幂等;
  • 同 ID + 不同 digest:VERSION_CONFLICT;
  • 新 ID 即使内容和历史版本完全相同,也仍然是新的 publication。

因此:

v1 = A
v2 = B
v3 = A

中的 v3 会正常经过容量准入、加载、READY 和 commit,而不会因为 digest 与 v1 相同被错误当成旧版本 replay。

已完成版本的迟到 replay 只返回 NO_OP,不会重新加载,也不会把 default 回滚到历史版本。


2. staged publication

训练侧首先通过现有 adapter export/gather 路径得到一份冻结 snapshot。

snapshot 固定:

  • config;
  • tensor 内容;
  • tensor name;
  • dtype;
  • shape;
  • 完整 digest / manifest。

后续摘要计算和 DCS/NCCL 传输全部使用同一份 snapshot,不再读取持续变化的 live 参数。

publication 顺序为:

Registry.allocate()
        ↓
claim_publication()
        ↓
E0/E1 begin
        ↓
PREPARED
        ↓
bucketed NCCL transfer
        ↓
end
        ↓
READY_LOCAL
        ↓
status confirmation
        ↓
Registry.mark_published()

claim_publication() 保证一个 attempt 只有一个 transport owner,避免不确定状态下重复驱动 NCCL collective。

READY receipt 会校验:

  • version ID;
  • digest;
  • lora name;
  • attempt;
  • engine incarnation。

只有 E0/E1 均属于同一个 publication attempt 并确认 READY 后,Registry 才允许 commit。


3. 唯一 default 切换点

LoRAVersionRegistry.mark_published() 是唯一的 default linearization point:

A: PUBLISHED -> RETIRED
B: LOADING   -> PUBLISHED
default      = B
default_revision += 1

因此:

E0 READY B
E1 尚未 READY B

时,default 仍然是 A。

只有两个目标都完成 READY 并成功 commit 后,新 Session 才会看到 B。


4. Session 首次生成绑定

Session 创建时不立即选择版本。

第一次真正执行 generation 时:

Session
   ↓
bind_latest(session_id)
   ↓
当前 published default

Registry 在同一个 actor turn 内读取 default 并记录 Session binding,因此 bind 与 publication commit 之间存在确定顺序:

bind before commit -> A
bind after commit  -> B

绑定保存在 _SessionRecord 中,而不是某个单独 IR。

后续:

  • tool turn;
  • retry;
  • temporary abort/resume;
  • 同一 Session 的后续 generation;

均继续使用相同的 immutable lora_path,不再重新读取最新 default。

首次绑定使用 Session 自己持有的共享 task,并通过 asyncio.shield() 等待;取消某一个 IR waiter 不会取消整个 binding,也不会让后续请求重新选择版本。


5. 禁止底层透明重发

对于 versioned generation:

max_retries = 1
fallback_to_local = False

Router retry 也被关闭。

原因是 HTTP timeout / Ray reply loss 并不能证明后端没有接受请求。

例如:

distributed POST
      ↓
backend 已接受
      ↓
Ray reply 丢失

这时不能再由 local HTTP client 自动发送第二次。

是否重新执行 generation 只能由 Agentic IR lifecycle 根据自身状态明确决定。


6. 安全回收

新版本 B commit 后,A 只进入:

RETIRED

仍然允许已绑定 A 的旧 Session 继续生成。

只有当没有 Session binding 后:

A: RETIRED -> RECLAIMING

每个 SGLang engine 随后执行:

unregister exact lora_name
        ↓
阻止新的 acquire
        ↓
wait_for_unload(lora_id)
        ↓
等待原生 request ref = 0
        ↓
physical unload

只有两个目标均确认卸载完成后:

A -> RECLAIMED

逻辑容量才真正释放。

因此:

capacity = 2

A RETIRED + still referenced
B PUBLISHED

时,C 会在任何 Begin / NCCL 操作之前得到 CAPACITY_ERROR。


7. SGLang lifecycle 修复

本 PR 继续复用 SGLang 原生 request refcount 和 LoRA registry 作为后端 request lifetime authority,只修复会破坏该 contract 的具体问题,包括:

  • acquire 后、dispatch 前失败需要正确 release;
  • 已 dispatch / terminal 状态不明时不能因为本地取消提前 release;
  • abort send 不等价于 scheduler terminal;
  • parallel-sampling children 共享正确的 LoRA ownership;
  • duplicate terminal / stale cleanup 不能 double-release;
  • disconnected unload caller 不能取消后台 wait_for_unload;
  • managed immutable adapter 不能被 ordinary overwrite / unload 修改;
  • reclaim 后的旧名字不能通过 implicit reload 复活;
  • stale publication / unload attempt 不能影响新的 attempt。

因此 Relax 只负责 logical policy lifecycle,engine-local lora_id、实际 request lifetime 和 physical unload 仍由 SGLang 管理。


8. KV isolation

Session 请求传递的是 immutable logical lora_name。

每个 SGLang engine 在本地将其解析为自己的 fresh lora_id:

logical version A
      ↓
E0 -> local lora_id X
E1 -> local lora_id Y

E0/E1 的本地 ID 不要求相同。

SGLang 的 LoRA identity 会进入 prefix cache namespace,因此不同版本不会共享错误的 KV cache。

Relax RadixTreeMiddleware 当前没有 LoRA version namespace,因此首版不与 versioned publication 组合使用;SGLang 自身 KV cache 保持开启。


Testing

CPU / fault / concurrency

当前 rebase 后 targeted suites:

201 passed, 1 skipped

唯一 skip 为 opt-in GPU experiment。

全仓:

pre-commit run --all-files

通过。

覆盖场景包括:

  • 同版本重复 publication;
  • 同 version ID 不同 digest conflict;
  • A -> B -> 新 ID 的 A;
  • 已完成旧版本迟到 replay 不回滚 default;
  • split-fleet READY;
  • publication driver 重入;
  • duplicate / stale control request;
  • manifest / incarnation mismatch;
  • collective 配对;
  • partial failure 与 ambiguous cleanup;
  • capacity refusal before Begin/NCCL;
  • concurrent first-bind;
  • binding waiter cancellation;
  • old/new Session binding consistency;
  • IR abort/resume 和后续轮次;
  • exact lora_path propagation;
  • distributed POST reply loss 不触发第二次 local send;
  • pre-dispatch failure;
  • abort failure;
  • duplicate terminal;
  • parallel child ownership;
  • Session release failure;
  • delayed reclaim;
  • managed-name ordinary unload / overwrite / implicit reload protection。

主要回归命令:

pytest \
  tests/agentic/test_lora_version_registry.py \
  tests/distributed/checkpoint_service/test_lora_publication.py \
  tests/backends/sglang/test_lora_staged_publication.py \
  tests/backends/sglang/test_lora_request_lifecycle.py \
  tests/test_agentic_rollout.py \
  tests/integration/test_lora_gpu_report.py

GPU acceptance

此前限定 GPU acceptance 已完成真实:

adapter snapshot
    ↓
DCS / NCCL publication
    ↓
E0 / E1
    ↓
Registry / Session binding
    ↓
actual SGLang generation

验证内容包括:

  • A/B 独立 numerical / logprob baseline;
  • old Session 保持 A;
  • commit 后 new Session 使用 B;
  • B 只有单引擎 READY 时 default 仍为 A;
  • A-warm -> B 与 B-warm -> A 双向 KV isolation;
  • publication 期间正常 generation 持续推进;
  • capacity=2;
  • 长 A request 阻塞 physical reclaim;
  • 每个 engine physical unload exactly once;
  • failed candidate cleanup;
  • retry;
  • completed publication replay。

历史确定性联合实验完成:

97 generations
0 request failures
max logprob error = 0
tolerance = 0.002

publication 总耗时 5.854 s,其中包含人为保持的 3 s READY window。

publication 开始到 E0 READY 的传输/准备阶段为 2.752 s,该阶段 E0/E1 各完成 9 个正常请求。

最长无完成间隔:

0.335 s

预设上限为:

2 s

请求延迟:

p50 = 0.271 s
p95 = 0.318 s
p99 = 0.322 s

512-token A 请求在 reclaim 期间正常完成,之后才允许物理卸载;两个 engine 各卸载一次。

双向 KV 实验中,首次跨版本目标请求保持 cold,再次同版本请求命中自身缓存,数值均与统一 cold baseline 匹配。


Scope

当前首版验证范围:

  • Dense text LoRA;
  • Qwen3 fixture;
  • 两个独立 SGLang engines;
  • TP=1、DP=1;
  • n=1;
  • single publisher;
  • no speculative decoding;
  • no PD disaggregation;
  • no elastic fleet change;
  • no Relax RadixTreeMiddleware;
  • GPU numerical acceptance 使用 deterministic inference + Triton attention,并关闭 scheduler overlap。

本 PR 不声称已经完成:

  • 完整 Agent 子进程 + 真实 tool interaction 的 GPU E2E;
  • 完整 Megatron training job / optimizer step 联合 GPU qualification;
  • multi-node / long-running large-load qualification;
  • deterministic mode 的完整 throughput cost;
  • 所有 TP/PP/PD/elastic execution profiles。

其中部分执行路径从代码结构上可能可以扩展,但本 PR 只声明实际验证过的范围。


Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Refactoring
  • Performance improvement
  • CI/CD or build changes

Checklist

yxy and others added 14 commits September 27, 2026 18:50
Single writer for the Task 7 LoRA version control plane: version identity
(epoch-scoped immutable names), publication state (LOADING -> PUBLISHED ->
RETIRED -> RECLAIMING -> RECLAIMED, plus the retryable/fatal failure split),
logical capacity admission, the default version, the authoritative
session_id -> version_id reference map, and reclaim eligibility.

Every transition is one atomic actor turn, so "bind before commit -> A" and
"bind after commit -> B" are well defined, duplicate allocations of the same
exact content are no-ops instead of a second load, and a third live version is
refused before any transport starts.

Co-Authored-By: Claude Code <noreply@anthropic.com>
A Session picks its LoRA version on its first generation, not at creation: the
first bind is shared through one task and every waiter awaits it shielded, so a
cancelled IR cannot cancel the binding and later tool turns, retries and
resumes reuse the same lora_path. Each generation now carries that exact
lora_path to the engine, and a versioned Session without a binding fails closed
instead of silently generating with the base model.

Session close releases the Registry reference once (after the backend drain and
after an in-flight bind converges), so a retired version becomes reclaimable
only when no Session can request it again. The Registry actor is created before
the SessionShards and destroyed with them, giving each deployment a fresh epoch.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Task 7 module 3. The DCS publisher now owns immutable version publication
instead of overwriting one fixed adapter name:

- freeze one AdapterSnapshot (owned storage) per sync and derive the exact
  manifest/digest from it, so identity and transmitted bytes can never diverge
- stage the publication as Begin -> Bucket* -> End on the existing
  /update_lora_from_distributed endpoint, committing only when every engine
  reported READY_LOCAL
- classify engine replies: a clean 4xx refusal may end retryable after an
  all-engines-ABSENT cleanup, anything ambiguous fails the run closed
- skip pause_generation/flush_cache once a version is live, so Sessions bound
  to the previous version keep generating during a publication
- reclaim retired versions with no bound Session, without retrying an
  unconfirmed unload
- first Agentic sync keeps the legacy one-shot wire but loads under the
  immutable v1 name, giving the Registry its first PUBLISHED version
- add --enable-versioned-lora-publication and --lora-publication-bucket-size
  (64 MiB default, min'ed with --update-weight-buffer-size), reject the
  RadixTreeMiddleware combination whose logprob cache is not version aware
- give the versioned engine 3 LoRA slots (current + retired + candidate)

Legacy non-Agentic adapter mode keeps its exact wire format, fixed name and
tolerance-based delta skip.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Task 7 module 4. The engine side of the staged transport, plus the
regenerated docker patch (v0.5.17 and latest stay byte-identical, verified
to apply to a pristine v0.5.17 tree and to reproduce the working files):

- UpdateLoRAFromDistributedReqInput grows op=legacy|begin|bucket|end|unload
  and the attempt/key/checksum fields; the existing endpoint keeps serving
  legacy one-shot updates unchanged
- the TP worker owns the staged stash: Begin validates the config and refuses
  cleanly, Bucket always joins the collective before classifying (duplicate ->
  drain -> idempotent success, new -> stash, stale -> drain then error after
  the collective, never revive an aborted attempt), End is an engine-local
  load gate that verifies the manifest checksums and drops the stash on any
  clean refusal
- a failed load or a receive error is reported clean=False (unknown, so the
  publisher fails closed), while a proven-absent candidate is HTTP 400
- op=unload is one idempotent name-scoped verb covering both cleanup and
  reclaim: discard the staged candidate, then unregister/wait/unload by name
- the tokenizer holds the candidate identity, serializes Begin/End against
  the LoRA registry under lora_update_lock, and rejects base/full-weight
  updates while a staged candidate exists
- release the logical LoRA refcount when a pending request is discarded
  before dispatch, so a dropped Session cannot pin an old version forever
- add test/registered/lora/test_lora_staged_publication.py (16 CPU tests)

Deviations from the RFC are deliberate and named in the code comments:
op=unload instead of end+abort, worker-owned stash/classification, no LRU
eviction on staged End (the publisher's registry keeps capacity exact).

Co-Authored-By: Claude Code <noreply@anthropic.com>
A retry reuses the immutable name (name = version + digest), so a name-scoped
unload cannot tell an old attempt's cleanup from the fresh instance a newer
attempt loaded. Both engines now refuse to act on a superseded attempt:

- the tokenizer keeps lora_name -> highest accepted attempt_id and ignores an
  attempt-scoped unload below it, leaving the staged record, the registry entry
  and the workers alone; the entry is dropped once the name is unloaded
- the worker compares the request against the newer of its in-flight stash key
  and the terminal marker of the last finished attempt, so a stale cleanup
  neither discards the current stash nor unloads the loaded instance
- the publisher sends attempt_id on attempt-scoped cleanups
  (cleanup_unpublished/best_effort_cleanup) and still omits it for reclaim: a
  published version never gets another attempt, so reclaim stays name-scoped

Also keep the docker patch runtime-only: the SGLang staged-publication tests
move to tests/backends/sglang/test_lora_staged_publication.py (Relax header,
no SGLang CI registration) so the patch carries no test file; they travel
upstream with the patch if it is ever submitted.

110 passed, 1 skipped across the Task 7 CPU suites; the regenerated patch
applies to a pristine v0.5.17 and reproduces the working files (47 sections).

Co-Authored-By: Claude Code <noreply@anthropic.com>
…ict leaves evidence

The bucket receive path returned success=False with clean=False and no log
line, so a broken collective reached the publisher as an opaque 500 and the
engine side kept no trace of why. Log it with the traceback; the verdict and
its clean/ambiguous classification are unchanged.

Co-Authored-By: Claude Code <noreply@anthropic.com>
The engine verified the sender's manifest with SHA256 over the tensor bytes
alone, while RFC 9.4 -- and Relax's tensor_manifest_hash -- digest
dtype + shape + contiguous bytes. Every real publication therefore failed
the value check on all 392 tensors ("rank0 staged adapter sync MISMATCH")
with a clean=False 400 that no unit test caught, because each side was only
ever tested against its own definition of the digest.

The engine now hashes the three RFC fields, and the Relax-side test builds
the manifest with Relax's tensor_manifest_hash to assert the two definitions
agree on the wire.

Found on the exp4 GPU acceptance run; the docker patch is regenerated from
the live SGLang tree (47 sections, verified against a pristine v0.5.17).

Co-Authored-By: Claude Code <noreply@anthropic.com>
# 🐛 Bug Fix

- Grant one transport driver per publication attempt so reentrant publish calls
  cannot replay NCCL broadcasts. Require protocol v2 before staged transfer and
  coalesce duplicate engine control calls without cancelling owned operations.
- Retain cleanup tombstones across attempts, preserve capacity for uncertain
  failures, and prevent direct retries from bypassing fleet failure fences.
- Validate the fixed E0/E1 fleet, manifest identity, engine incarnations and
  default revision before commit. Prepare GPU LoRA slots before READY and use
  the staged protocol for bootstrap as well as subsequent publications.
- Protect managed adapter names from legacy replacement and ordinary unload.
  Keep native request-refcount waits alive when unload callers disconnect.
- Disable HTTP and router retries for versioned generation, and retain the
  Session-owned binding task when an individual IR waiter is cancelled.
- Deploy the Relax changes together with the updated SGLang v0.5.17 patch;
  staged protocol v1 is rejected before transfer.

---

# ✅ Tests

- Cover publisher reentry and duplicate receiver control delivery together,
  plus cleanup-before-Begin, ambiguous/missing ACKs, incarnation changes,
  capacity fences, managed-name protection and cancellation races.
- Pass 129 targeted tests across registry, publisher, Session and patched
  SGLang suites in batches, using an isolated SGLang source tree.
- Pass pre-commit run --all-files --show-diff-on-failure on the exact staged
  tree, and verify forward/reverse applicability of the SGLang patch.
- GPU/NCCL integration validation was not run: no Ray cluster address was
  provided for this task. Collective matching was tested with transport doubles.
# 🐛 Bug Fix

- Retain dispatched SGLang request state and LoRA references until scheduler
  termination, including when abort dispatch fails.
- Bind reference ownership to request lifecycles and share one acquire across
  parallel-sampling children until the final child terminates.
- Fence stale cleanup and avoid duplicate releases from terminal handlers.
- Preserve completed publication identities after retirement and reclamation so
  replay cannot reload an old adapter or roll back the published default.
- Retry idempotent Session reference releases; retain the Session and surface an
  explicit error when release remains unconfirmed, allowing cleanup to retry.

---

# ✅ Tests

- Add CPU regressions for failed abort dispatch, late lifecycle cleanup,
  duplicate terminal replies, parallel children, completed publication replay,
  lost release acknowledgements, and retrying failed Session finalization.
- Validate 140 CPU tests, then rerun the five native lifecycle tests after
  strengthening terminal-ack coverage to use the real abort response handler.
- Validate the SGLang patch in both forward and reverse directions.
- Run all-file pre-commit checks against the exact staged tree in an isolated
  worktree to preserve unrelated working changes.
- Skip GPU numerical, KV-cache, NCCL, and sustained-traffic experiments as agreed.
# 🐛 Bug Fix

- Surface Session cleanup failures through Group errors, health and debug state
  from the cleanup owner even when a waiter disconnects.
- Retain failed Groups and Sessions, and allow the public drop_group entry point
  to retry a failed shared task after the Registry recovers.
- Compact confirmed SGLang publication unloads by releasing full records,
  manifests and completed control tasks while preserving terminal fences.
- Keep full ownership records when unload completion is unconfirmed.

---

# ♻️ Refactor

- Remove the redundant single-terminal worker field; use per-attempt state.
- Use one staged publication path for bootstrap and subsequent updates.

---

# ✅ Tests

- Cover observable background cleanup errors, public Group drop retries and
  concurrent callers after Registry recovery.
- Cover repeated publication cache compaction and unconfirmed unload retention.
- Require collective participants in fault doubles; verify real receiver
  incarnation rejection cannot produce a successful source broadcast.
- Pass 143 CPU regressions plus the final receiver-incarnation regression.
- Pass all-file pre-commit on the exact staged tree in an isolated worktree,
  and validate forward and reverse application of the SGLang patch.
- Skip GPU/NCCL hardware experiments as agreed.
# 🐛 Bug Fix

- Surface retained Session cleanup failures to runtime waiters without releasing
  ownership before backend termination and physical cleanup are confirmed.

---

# ✅ Tests

- Add repeatable GPU acceptance drivers for immutable adapter publication,
  bidirectional KV isolation, capacity limits, failure recovery and reclamation.
- Compare deterministic publication traffic and warm requests against independent
  cold A/B logprob baselines with fixed tolerance and generation progress limits.
- Cover cleanup failure propagation and production IR abort/resume and later
  turns across publication, retaining A for old Sessions and B for new Sessions.
- Validate 97 GPU generations without failures or logprob deviations, plus
  bidirectional cache probes and delayed reclamation of an in-flight request.
- Keep personal reports, generated experiment output, official-site documentation
  and environment-specific startup fixes outside this PR.
# 🐛 Bug Fix

- Forward Megatron Bridge adapter conversion keyword options, including
  exclude_adapter_base_prefixes, through the adapter snapshot wrapper.
- Avoid joining the current async-loop thread during shutdown callbacks;
  preserve bounded joining for external callers and use the project logger.
- Document that shutdown from the loop thread requests stopping without
  synchronously waiting for that thread to exit.

---

# ✅ Tests

- Cover adapter option forwarding with and without snapshot context and
  with both legacy calls and keyword-bearing export calls.
- Exercise shutdown on the actual loop thread and from an external thread,
  including repeated shutdown after clearing the global loop reference.
- Pass nine focused utility and Controller restart regression tests.
- Pass pre-commit run --all-files --show-diff-on-failure.
# 🐛 Bug Fix

- Require an explicit publication version ID in the registry and publisher;
  use the captured training sync sequence instead of a digest-derived identity.
- Permit a new version to publish historical adapter content after capacity
  admission, while completed retries of the old ID cannot roll back default.
- Reject different content for an existing ID before engine RPCs or reclamation.
- Retain attempt fencing, two-engine readiness and reference-protected cleanup.

---

# ✅ Tests

- Cover A-to-B-to-new-A publication, capacity refusal until A is released,
  old Session bindings, late replay, and content conflicts in every state.
- Verify explicit IDs across DeviceDirect and the Ray registry client and
  update existing lifecycle, protocol and GPU drivers to supply stable IDs.
- Validate 173 CPU cases across regression runs; publisher rerun passes all
  33 cases after correcting its event filter. Skip opt-in GPU execution and
  a bucketing module without megatron.core; no new GPU result is claimed.
- Pass all-file pre-commit checks. Keep personal reports and raw GPU artifacts
  outside the commit.
# 🐛 Bug Fix

- Add an opt-out for local fallback after a distributed POST failure and use
  it together with one HTTP attempt for versioned generation requests.
- Preserve the original exception when backend acceptance is uncertain;
  retain the existing fallback default for other callers.
- Keep the complete immutable adapter digest in cached Session bindings.
- Clarify that Publisher supports explicit same-ID retry while re-entering
  the production weight-update entrypoint creates a new sync version.

---

# ✅ Tests

- Inject loss of a Ray reply after remote HTTP acceptance and verify that
  versioned generation never sends a second local request.
- Cover default fallback, local first dispatch, concurrent binding and
  full identity preservation after a later adapter publication.
- Pass 149 targeted CPU tests and all-file pre-commit checks; GPU experiments
  were not rerun for this change.
@rai-studio-bot

rai-studio-bot commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Nyanpasu 审查看板

审查状态: ✅ 已通过

审查版本: 03a369b

目标分支: main

复审完成,结论:通过。作者在 bcb285e→03a369b 的四个提交中逐项修复了全部 5 个发现(F1 importorskip 守卫、F2 两引擎 fleet 启动期校验、F3 forget_sessions 墓碑清理、F4 容量拒绝降级为跳过并保留致命语义、F5 真实入口的免暂停时间线回归测试),每个修复均经代码复核与本地可运行测试验证(registry 48 passed、utils 含新增校验测试 31 passed、无 sglang 环境收集跳过复现)。四个新提交的增量审查未发现新问题;范围按新 inventory 重新裁定(30 文件全部接收)。备注:该 fork PR 的 CI workflows 处于 action_required(需维护者批准才会运行),03a369b 的全套 CI 结果需在批准后确认;讨论线程因账号权限无法标记 resolved,各线程内已附验证回复。

审查阶段进度范围与结果
常规审查 ✅ 已完成 全量常规审查于 a61bbd2/bcb285e 完成(5 个发现);本轮对 bcb285e→03a369b 增量(4 提交,+387/−18,9 文件)逐 hunk 复核:5 个修复全部确认有效且未引入新问题(容量跳过的守卫条件、forget 的调用顺序与幂等性、continue_generation 仅在暂停过时发送)。本地运行 tests/agentic/test_lora_version_registry.py(48 passed,含新增 forget 测试)与 tests/utils/{test_arguments_versioned_lora,test_async_utils,test_http_utils}.py(31 passed);无 sglang 环境下两份 sglang 依赖测试文件均正确跳过收集。
深度审查 ✅ 已完成 独立设计对照与测试套件审计于上一轮完成(产出 F4/F5 与必要性审计)。本轮验证两个深度发现(F4/F5)的修复落实:F4 采用良性跳过语义并保留 bootstrap/协议错误致命边界(入口级测试覆盖);F5 补齐真实 update_weights_for_rollout 入口的 pause/flush 时间线断言。生产/测试必要性审计结论不变(新增代码为定向修复与配套测试,无新增独立机制)。GPU 端到端复验仍依赖 opt-in 三卡 harness,本轮未独立运行(局限如实记录)。

审查发现

待处理
编号 严重性 问题状态规则来源
暂无待处理的记录。
已解决或已取代
编号 严重性 问题状态规则来源
F1 High severity test_lora_staged_publication.py 在无 SGLang 的 CI 环境仍会收集失败,需补 importorskip 守卫 ✅ 已解决 CI Tests (Python 3.10/3.11/3.12) 环境不安装 sglang(ci.yml 仅装 CPU torch,requirements.txt 无 sglang)
F2 Medium severity --enable-versioned-lora-publication 缺两引擎 fleet 的启动期校验,错误配置到首次同步才失败 ✅ 已解决 RFC #369 首版支持配置:固定两个独立 SGLang 引擎 E0/E1(每个 TP=1、DP=1)
F3 Low severity LoRAVersionRegistry.closed_sessions 墓碑集合无界增长(长训练运行下的维护性问题) ✅ 已解决 —
F4 Medium severity 容量拒绝在旧 Session 仍存活时使训练 run 致命终止,而非跳过该次发布(触发条件为特性标称场景) ✅ 已解决 RFC #369:旧 Session 跨权重更新继续使用绑定版本(§摘要/§1);PR 描述 §6 仅声明 CAPACITY_ERROR 拒绝,未声明训练循环后果
F5 Medium severity versioned_staged 免暂停/免 flush 门控缺常规 CI 回归测试(核心行为主张仅由 opt-in GPU 验收覆盖) ✅ 已解决 RFC #369 §11.2:base 同步后 adapter-only 发布不得全局暂停生成或清空 KV cache
提交范围 · 接收 30 · 建议移出 0 · 待确认 0

接收 30 个文件 · 建议移出 0 个文件 · 待确认 0 个文件。移出与待确认部分暂停深审,不代表审查通过。

文件结论仓库维护必要性依据替代去向或方案
relax/agentic/pipeline/__init__.py
relax/agentic/pipeline/runtime.py
relax/agentic/session/lora_version.py
relax/agentic/session/service.py
relax/distributed/checkpoint_service/backends/device_direct.py
relax/distributed/checkpoint_service/lora_publication.py
relax/distributed/ray/rollout.py
relax/backends/sglang/sglang_engine.py
relax/utils/arguments.py
relax/utils/async_utils.py
relax/utils/http_utils.py
relax/utils/megatron_bridge_utils.py
接收 These files implement the required Task-7 behavior itself: the immutable version registry and session binding (lora_version.py, service.py, runtime.py), the staged publication transport and orchestration (lora_publication.py, device_direct.py), the no-retry transport semantics (http_utils.py, rollout.py, runtime.py), engine capacity configuration (sglang_engine.py), CLI gating (arguments.py) and export fixes needed by the frozen snapshot (megatron_bridge_utils.py, async_utils.py). All are reachable from existing production callers (AgenticSessionShard, DeviceDirectBackend.push_weights path, SGLangBackendAdapter.generate). PR #382 requirement (RFC #369, Task 7); existing callers relax/agentic/session/service.py, relax/distributed/checkpoint_service/backends/device_direct.py, relax/agentic/pipeline/runtime.py. Keeping any of this outside the repository (PR artifact / experiment repo) would not implement the feature; the legacy fixed-name adapter path cannot express old-session/new-session version coexistence.
docker/patch/sglang/v0.5.17.patch
接收 The engine-side half of the contract (native LoRA request refcount fixes, staged load-from-tensors publication, exact-name unload fencing, acquire/release lifecycle) can only ship through the repo's pinned SGLang patch, which is the established mechanism for SGLang customization (docker/patch/sglang/*). The Relax-side code calls the RPCs this patch adds. PR #382 requirement (RFC #369 §7); repo convention docker/patch/sglang/v0.5.17.patch consumed by the training image build. Moving the engine changes to an external branch would break the pinned-patch build contract and leave relax/distributed/checkpoint_service/lora_publication.py calling RPCs that do not exist.
tests/agentic/__init__.py
tests/agentic/lora_helpers.py
tests/agentic/test_lora_version_registry.py
tests/backends/sglang/test_lora_request_lifecycle.py
tests/backends/sglang/test_lora_staged_publication.py
tests/distributed/checkpoint_service/test_lora_publication.py
tests/test_agentic_rollout.py
tests/utils/test_async_utils.py
tests/utils/test_http_utils.py
tests/utils/test_megatron_bridge_utils.py
tests/utils/test_arguments_versioned_lora.py
接收 CPU-runnable regression tests for the new production behavior (registry semantics, staged publication fault handling, session binding, no-replay POST, loop shutdown, export kwargs). They run in normal CI (Tests Python 3.10-3.12) and protect real entry points, not a simulator; the PR lists them as the maintained regression suites. The new tests/utils/test_arguments_versioned_lora.py covers the two-engine fleet validation added for review finding F2. PR #382 Testing section; repo layout tests/ with per-module suites. Dropping them or keeping them as PR-run-only evidence would leave the new lifecycle without any in-repo regression coverage.
tests/integration/lora_gpu/instrumentation/sitecustomize.py
tests/integration/lora_gpu/kv.py
tests/integration/lora_gpu/resources.py
tests/integration/lora_gpu/run.py
tests/integration/lora_gpu/support.py
tests/integration/test_lora_gpu_report.py
接收 Opt-in three-GPU acceptance harness wired through pytest with explicit env-gated skip (RELAX_LORA_GPU_ACCEPTANCE=1), matching the repo's GPU-test convention. It drives the real production stack (materialize_adapter_snapshot, LoRA publication path, SGLangBackendAdapter.generate against live engines) rather than a second implementation, and test_lora_gpu_report.py keeps its comparison/order logic covered in CPU CI. It is the only automated way to re-qualify the publication lifecycle after an SGLang patch upgrade, which is a real recurring regression risk for this repo. PR #382 GPU acceptance section; repo convention of opt-in GPU tests (skipif with explicit reason) and the sglang patch upgrade workflow. Keeping the harness only as PR evidence would lose the reusable re-qualification path; the driver is deliberately structured as a re-runnable harness, not a one-off verdict dump.
精简审查与验证依据
审查范围进度结论
生产代码 ✅ 已完成 已对照独立最小设计逐项审查生产机制的必要性(参考产物:reference-design.md / failure-model.json / evidence.json,独立子任务 base-tree 推导、无实现信息污染)。核心结论:新增机制均有可达失败模式支撑,无经证实的可删除项;两处作者独有机制(router 重试关闭、distributed POST 回退禁用)是独立设计遗漏而实现正确补上的必要项。 本轮增量(4 个修复提交)未新增独立机制,审计结论维持。
测试 ✅ 已完成 已按测试族逐组审计检测力与必要性(子任务 248d5153:可运行族建立基线并做定向变异——70 通过基线、11/11 变异被预期测试捕获、行为保持重构对照通过、基线复原;torch/sglang 受限族以断言可达性精读替代并记录局限)。结论:全部保留,无 mock-only 套件;两处可选合并记录如下,不构成阻塞;一处覆盖缺口已作为 F5 发布。 本轮新增 6 个测试(容量跳过/致命边界/免暂停时间线 3 组入口级测试、forget 排线 3 个、引擎数校验 1 文件、registry forget 2 个),全部为对已发布发现的定向覆盖,必要性明确。

生产代码的必要性与替代方案

范围必须保留的契约更简单的方案结论依据与限制
LoRAVersionRegistry 状态机 + LoRAPublisher staged 协议(lora_version.py、lora_publication.py 及引擎侧控制面) RFC #369 R1-R6:版本身份/default 唯一切换/容量准入/回收围栏;调用方 AgenticSessionShard(绑定)与 DeviceDirectBackend(发布)。 独立参考设计的更精简 PolicyVersionTable(无 attempt CAS、无 incarnation receipt、无 FAILED_RETRYABLE/FATAL 区分,失败留待下次同步清扫)。 保留 参考设计自身的 failure-model 将引擎重启、回复丢失、歧义卸载列为 high 风险;实现的 attempt 围栏/READY receipt 校验/reclaim_fatal 恰好逐一对应这些可达失败,且失败语义更保守(clean 确认才 RETRYABLE,否则 FATAL 并占住容量)。参考设计的验证为模型级(720 排列不变量),生产布线未在其环境执行;本环境无 GPU/torch,删除实验不可行,保留判断基于调用链与失败模式证据。
retry_publication API 及 allocate 的 FAILED_RETRYABLE 重试分支(lora_version.py:264/281) LoRAPublicationError 消息与 device_direct 注释中声明的「同 version_id+digest 显式重试」契约。 删除 retry_publication 与该分支:生产驱动每次同步 weight_version 递增,失败版本 ID 永不重用,此路径生产不可达(仅 allocate 与测试调用)。 保留 grep 证实仅 allocate/tests 引用;它是被显式文档化的 API 边界(device_direct 注释承认驱动不在原地重试),引擎侧 attempt 递增围栏语义依赖该路径定义。删除省约 40 行但失去已定义的重试契约;记录为已审查的保留而非精简项。
无重发传输三件套:max_retries=1 + fallback_to_local=False(runtime.py/http_utils.py)与 router disable_retries(rollout.py:4543-4547) RFC #369 R7:版本化生成不得在响应丢失后被透明重发;调用方 SGLangBackendAdapter.generate(仅 lora_path 非空时启用)。 无更小方案——这即最小机制;独立参考设计只做了 max_retries=1,遗漏了 sglang-router 的自动重试层与 distributed POST Ray 失败后的本地回退层。 保留 已核实 _post(max_retries=1) 恰好单次尝试;部署镜像的 sglang-router 0.3.2 fork 具备 disable_retries 字段且 Rust builder 以 .retries(!disable_retries) 消费;slime router 无重试逻辑。两处作者独有机制为必要补充。
closed_sessions 墓碑集合(lora_version.py:156/474) release 早于首次 bind 到达时,晚到 bind 不得复活已关闭 Session 的引用(SESSION_CLOSED)。 shard 在删除本地 _SessionRecord 后显式调用 forget_sessions(ids),或将墓碑限定在引用版本被 RECLAIMED 前。 保留 release-before-bind 顺序在 bind/finalize 交错下可达(测试覆盖);墓碑语义本身必要。无界增长已作为 F3 发布改进建议,不重复立项。

测试的必要性与替代方案

范围必须保留的契约更简单的方案结论依据与限制
tests/agentic/test_lora_version_registry.py(46 测试) Registry 原子决策层:身份/容量/commit CAS/绑定/回收/失败态;被测试直接驱动真实 LoRAVersionRegistry。 无更小方案可保持检测力:7 个定向变异(无 READY 双确认 commit、忽略绑定回收、丢弃 digest 冲突、迟到 commit 复活 RETIRED、去除 attempt CAS、容量不拒绝、去除墓碑检查)各被恰好一个预期测试捕获;等价重构对照通过。 保留 子任务在一次性工作区对真实套件执行的变异实验记录(基线 46 通过,变异后按预期失败,复原后 46 通过)。
tests/distributed/checkpoint_service/test_lora_publication.py(24 测试) LoRAPublisher staged 协议编排、clean-vs-fatal 判定、回收 fan-out、device_direct 入口;_FakeEngines 仅记录生产侧发出的 wire payload(传输替身,非逻辑复制)。 test_first_sync_uses_the_same_staged_protocol 与 happy-path 流程重复、仅多一个 bind_latest 断言,可合并入 happy-path;非阻塞的可选精简。 合并 子任务精读对比:合并后保留全部输入边界与断言覆盖;因节省有限且不影响检测,不单独立项,仅记录为可选。
tests/backends/sglang/test_lora_staged_publication.py(约 30 测试)与 test_lora_request_lifecycle.py(4 测试) 引擎侧 staged 状态机 + tokenizer 控制簿记 + 两处跨实现测试(publisher 驱动真实 tokenizer/worker 状态机);原生请求 refcount 跨取消/重复终态/陈旧清理/并行子请求。 加强项:为 staged_publication 补 importorskip 守卫(与同目录 test_genrm_offload_drain / test_sglang_engine / test_sigterm_eviction 及本 PR 的 request_lifecycle 一致)——即 F1;lifecycle 已有守卫,四个测试无重复,保留。 保留 子任务在无 sglang 环境复现收集错误,确认守卫缺失即 F1;本环境无法运行 torch/sglang 族,以断言可达性精读替代,记录为局限。
tests/test_agentic_rollout.py Task-7 段(16 测试)与 utils 三件(async/http/megatron_bridge) Session 绑定生命周期、wire 传播、无重发三层、清理所有权/重试、进度线失败传播;utils 覆盖环路关闭与导出选项传播。三个无重发测试分别钉住 payload 选项、分布式派发不回退、_post 单次语义三个不同层,不可合并。 考察过合并三个 no-replay 测试:会丢失分层检测(M8/M9 变异分别由 utils 层捕获);F5 段与 utils 段为互补入口,保留两组。 保留 子任务变异 M8(fallback_to_local 被忽略)与 M9(4xx 重试)各由预期 utils 测试捕获,证明分层必要。
GPU 验收 harness(tests/integration/lora_gpu/* 与 test_lora_gpu_report.py,opt-in 三卡) 唯一端到端证据:发布期间生成持续推进(max_no_progress ≤ 2s)、对独立预声明 A/B 基线的数值等价(预声明 EPS,无事后容差)、KV 体制保持、容量拒绝、回收次序;非模拟器,驱动真实生产栈。 考察过移出仓库(PR 工件化):会失去 SGLang patch 升级后唯一的复检通道;另记录可选项——test_lora_gpu_report 的 3 个纯逻辑测试可解除对 torch 的导入链依赖以便无 torch 环境收集(节省极小,不立项)。 保留 范围裁定阶段已确认其 pytest 化 + env 门控符合仓库 GPU 测试约定;本环境无 GPU/sglang,harness 未独立运行,其验收数字以 PR 报告为据(局限已记录)。
Powered by Nyanpasu with glm-5.3[1m] xhigh, please check the suggestions carefully.

# ✅ Tests

- Guard the three SGLang module imports with importorskip so CPU CI without
  SGLang skips this lifecycle module instead of failing during collection.
- Verify the missing-SGLang case skips; the targeted suite with the matching
  patched SGLang passes 174 tests with one opt-in GPU skip.
- Pass all-file pre-commit checks; leave core implementation unchanged.

@rai-studio-bot rai-studio-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

结论:需要修改(1 个阻塞项,2 个非阻塞建议)

常规审查已完成:不可变版本身份、staged publication 状态机、Session 绑定与安全回收、无透明重发的传输语义(max_retries=1 + fallback_to_local=False + 路由重试关闭),以及 sglang patch 引擎侧生命周期(attempt 围栏、原生 refcount 复用、managed 名称保护)与 RFC #369 的要求一致,生产代码未发现阻塞缺陷。阻塞项在测试侧:bcb285e 只为 test_lora_request_lifecycle.py 加了 importorskip,test_lora_staged_publication.py 的模块级 sglang 导入仍未守卫,而 Tests (Python 3.10/3.11/3.12) 所在 CI 环境不安装 sglang,收集仍会失败(已用无 sglang 环境复现;a61bbd2 上三个 Tests job 的失败日志与之相互印证)。另有 2 条非阻塞建议(参数校验补两引擎约束、Registry 墓碑集合无界增长)见行内评论。

深度审查(独立设计对照与测试检测力/必要性审计)仍在进行,结论随后在看板更新。

看板:#382 (comment)

Powered by Nyanpasu with glm-5.3[1m] xhigh, please check the suggestions carefully.

from types import SimpleNamespace

import torch
from sglang.srt.lora.utils import verify_lora_tensor_checksums

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

High severity P1 · 为无 SGLang 的 CI 环境补充 importorskip 收集守卫

bcb285e 为 test_lora_request_lifecycle.py 补了 importorskip 守卫,但本文件仍有未受保护的模块级 from sglang... 导入,CI 的 Tests (Python 3.10/3.11/3.12) 仍会失败:

  • 该 CI workflow 只安装 CPU torch(.github/workflows/ci.yml 的 Install dependencies 步骤),requirements.txt 中只有 sglang-router、没有 sglang,这些 runner 上 import sglang 必然失败。
  • a61bbd2 上的 CI 日志显示三个 Tests job 均以 ModuleNotFoundError: No module named 'sglang' 的收集错误失败;由于 pytest 带 -x,收集停在按字母序第一个出错的文件(当时是未加守卫的 test_lora_request_lifecycle.py)。该文件修复后,下一个未守卫的就是本文件。
  • 本地已复现(torch stub、无 sglang 环境):pytest --collect-only tests/backends/sglang/test_lora_staged_publication.py → ModuleNotFoundError: No module named 'sglang'(首个失败导入即第 23 行)。

建议套用与 test_lora_request_lifecycle.py 相同的守卫模式。注意 UpdateLoRAFromDistributedReqInput 等是本 PR 的 sglang patch 新增符号,守卫需落在 sglang.srt.* 子模块粒度(patch 只随镜像安装,CPU runner 上整包缺失):

Suggested change
from sglang.srt.lora.utils import verify_lora_tensor_checksums
import pytest
pytest.importorskip("sglang.srt.lora.utils", exc_type=ImportError)
pytest.importorskip("sglang.srt.managers.io_struct", exc_type=ImportError)
pytest.importorskip("sglang.srt.managers.tokenizer_control_mixin", exc_type=ImportError)
pytest.importorskip("sglang.srt.model_executor.model_runner", exc_type=ImportError)
from sglang.srt.lora.utils import verify_lora_tensor_checksums
Suggested change
from sglang.srt.lora.utils import verify_lora_tensor_checksums
import pytest
pytest.importorskip("sglang.srt.lora.utils", exc_type=ImportError)
pytest.importorskip("sglang.srt.managers.io_struct", exc_type=ImportError)
pytest.importorskip("sglang.srt.managers.tokenizer_control_mixin", exc_type=ImportError)
pytest.importorskip("sglang.srt.model_executor.model_runner", exc_type=ImportError)
from sglang.srt.lora.utils import verify_lora_tensor_checksums

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

已验证修复(55f4c1b,head 03a369b):与建议一致的 importorskip 四个 sglang.srt.* 子模块守卫已就位。本地在无 sglang 环境(torch stub)下 --collect-only 复核:该文件现在正确跳过收集,不再报 ModuleNotFoundError。CI 的 Tests (Python 3.10/3.11/3.12) 收集阻塞解除。

Comment thread relax/utils/arguments.py
# The middleware caches rollout logprobs per prompt prefix and only GCs them by the
# base serving weight_version, which an adapter-only publication deliberately does not
# advance. A request on LoRA version B would then deterministically reuse A's cached
# logprobs for the same prefix — wrong policy data, not just stale routing.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Medium severity P2 · 校验 --enable-versioned-lora-publication 要求的两引擎 fleet

版本化发布路径把目标 fleet 固定为两个引擎,但该约束没有进入参数校验,配置错误的 fleet 会启动成功、直到第一次 adapter 同步才崩溃:

  • LoRAVersionRegistry.target_engines = frozenset({"engine0", "engine1"})(relax/agentic/session/lora_version.py:159);
  • LoRAPublisher._require_fleet 硬编码 {"engine0", "engine1"}(relax/distributed/checkpoint_service/lora_publication.py:635);
  • _StagedEngineFanout 要求 set(engines) == {0, 1},否则 ValueError(relax/distributed/checkpoint_service/backends/device_direct.py:1447)。

而这里的校验只覆盖三个 flag 组合与 RadixTreeMiddleware 冲突。例如 --rollout-num-gpus 4 --rollout-num-gpus-per-engine 1 的 fully-async agentic 运行能通过校验并正常启动,然后在第一次权重同步的 _publish_lora_adapter_versioned 内部抛 ValueError——训练已开始后才失败。RFC #369 的首版支持配置明确固定 E0/E1 两引擎,建议与其他组合校验一致地 fail-fast:

Suggested change
# logprobs for the same prefix — wrong policy data, not just stale routing.
if getattr(args, "rollout_num_gpus", 0) // getattr(args, "rollout_num_gpus_per_engine", 1) != 2:
raise ValueError(
"--enable-versioned-lora-publication requires exactly two rollout engines (E0/E1) in "
"this first version; got "
f"{getattr(args, 'rollout_num_gpus', 0) // getattr(args, 'rollout_num_gpus_per_engine', 1)} "
"from --rollout-num-gpus / --rollout-num-gpus-per-engine."
)
if getattr(args, "use_slime_router", False) and "RadixTreeMiddleware" in (
Suggested change
# logprobs for the same prefix — wrong policy data, not just stale routing.
if getattr(args, "rollout_num_gpus", 0) // getattr(args, "rollout_num_gpus_per_engine", 1) != 2:
raise ValueError(
"--enable-versioned-lora-publication requires exactly two rollout engines (E0/E1) in "
"this first version; got "
f"{getattr(args, 'rollout_num_gpus', 0) // getattr(args, 'rollout_num_gpus_per_engine', 1)} "
"from --rollout-num-gpus / --rollout-num-gpus-per-engine."
)
if getattr(args, "use_slime_router", False) and "RadixTreeMiddleware" in (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

已验证修复(55f4c1b):slime_validate_args 现要求 rollout_num_gpus 整除 rollout_num_gpus_per_engine 且商恰为 2(还正确处理了非法/零值),fail-fast 位置与建议一致;新增 tests/utils/test_arguments_versioned_lora.py 覆盖该校验。本地运行该文件与 utils 套件共 31 passed。

"""

key = self._key(session_id)
self.closed_sessions.add(key)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Low severity P3 · closed_sessions 墓碑集合在 Registry actor 内无界增长

release() 为每个结束的 Session 在 closed_sessions 里留下墓碑(防止晚到的 first-bind 复活引用,语义本身是对的),但该集合从不清理:rollout 每个 sample 会创建一个 Session,长训练运行中会累积百万级条目,单写 Registry actor 的内存占用无界增长(self.versions 同样按 publication 事件累积,只是量级小得多)。

墓碑只在「release 之后、本地 _SessionRecord 删除之前」这个窗口内有正确性作用(record 删除后 shard 不可能再为该 Session 发起新 bind)。可以考虑:

  • shard 在 del self._session_records[...] 之后调用一个显式的 forget_sessions(ids);或
  • Registry 在版本被 RECLAIMED 且其 session_bindings 为空时允许清理对应墓碑(需确认所有晚到 bind 路径已收敛)。

首版容量规模下影响很小,不阻塞;仅作为长跑部署的维护性记录。

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

已验证修复(03a369b):forget_sessions 批量清理墓碑,且调用方顺序正确——先完成全部 Session(bind task 收敛、release 确认、本地 record 删除),再 forget,最后才 pop group;forget RPC 失败时 group 保留可重试,difference_update 保证幂等;对仍含活跃绑定的批次抛 SESSION_STILL_BOUND 拒绝清理。配套测试覆盖 20 组循环边界、ACK 丢失重试、drop waiter 取消等场景(registry 套件本地 48 passed,含 2 个新 forget 测试)。无界增长问题解决。

# 🐛 Bug Fix

- Reject versioned publication configurations unless the rollout GPU budget
  divides into exactly two complete engines; allow two GPUs per engine.
- Reject missing or nonpositive per-engine counts with a clear startup error.

---

# ✅ Tests

- Skip staged LoRA publication collection when optional SGLang modules cannot
  import, matching the lifecycle test guard.
- Verify both SGLang modules skip under simulated missing SGLang with -x.
- Add 11 argument cases covering valid fleets, incomplete and invalid fleets,
  and unchanged validation for non-versioned mode; all pass after completing
  the fully-async fixture. The other 183 targeted cases pass with one opt-in
  GPU entry skipped. All-file pre-commit checks pass.
- Leave closed-session tombstones and lifecycle implementation unchanged.

@rai-studio-bot rai-studio-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

结论:仍需修改(新增 1 个 P2,P1 未解决)

深度审查的设计对照已完成:独立最小设计与本实现核心同构(版本化命名、单写 Registry、双引擎就绪才切 default、首代绑定、max_retries=1),作者还正确补上了独立设计遗漏的两处重发漏洞(router 层重试与 distributed POST 回退)。新增 1 个 P2:容量拒绝在旧 Session 仍存活时会使训练 run 致命终止而非跳过该次发布(触发条件即本特性的标称场景,详见行内评论);此前的 P1(CI 收集失败)在当前 head 仍未解决。测试套件的独立审计仍在进行,结论将在看板汇总。

看板:#382 (comment)

Powered by Nyanpasu with glm-5.3[1m] xhigh, please check the suggestions carefully.

try:
publication = self._registry.allocate(snapshot.digest, version_id=version_id)
except LoRAVersionError as exc:
if exc.code != "CAPACITY_ERROR" or not self.reclaim_once():

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Medium severity P2 · 容量拒绝在旧 Session 仍存活时会使训练 run 致命终止,而非跳过该次发布

这是深度设计对照得出的行为问题:当 A(RETIRED,仍被存活 Session 绑定) + B(PUBLISHED) 占满 2 个逻辑槽位时,下一次发布的 allocate 抛 CAPACITY_ERROR,reclaim_once() 因 A 仍被引用返回 False,于是这里直接 raise。该异常在整个训练链路上无人捕获:

publish() → _publish_lora_adapter_versioned(push_error 仅为先恢复 generation 再重抛)→ update_weights_for_rollout → MegatronTrainRayActor.update_weights_fully_async(actor.py:2329,无捕获)→ Actor._execute_training(components/actor.py:315,无捕获)→ 训练后台循环 except 后 re-raise(components/actor.py:243,除非 use_health_check),训练 run 终止。

触发条件正是本特性的标称场景:一个跨越两次发布的旧 Session(RFC #369 的前提就是 Session 跨权重更新;PR 自身的验收 harness 也构造了「A 仍被引用时发布 C 得到 CAPACITY_ERROR」——resources.py:42 捕获了该异常继续运行,但生产训练循环没有这层捕获)。Agent episode 的工具轮次/retry/abort-resume 没有跨步上界,一次慢 Session 即可在下一次权重同步时终止整个训练作业。

两个方向二选一(需要维护者定夺语义):

  1. 良性跳过(推荐讨论):在 _publish_lora_adapter_versioned 捕获 CAPACITY_ERROR,记 log/metric 后返回 SKIPPED 结果——本步不发布、继续用 B 生成,下一步以新 version_id 重试(weight_version 已递增,注册表从未见过被拒版本,状态干净)。fully-async 模式本身接受策略 staleness,这与 _check_services_health 对服务不可用的降级模式一致;同时应把「发布被旧 Session 阻塞」暴露为可观测指标。
  2. 显式声明 fail-closed:若维持现语义(拒绝发布即失败),至少在 --enable-versioned-lora-publication 的 help 与 PR 描述中写明「旧 Session 阻塞回收时训练会失败」,让使用者可预期。

不建议保持现状:PR 描述 §6 只说明了「C 会在 Begin/NCCL 之前得到 CAPACITY_ERROR」,未说明这对训练循环意味着致命失败。

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

已验证修复(3dd5851,采纳方向 1):_publish_lora_adapter_versioned 捕获 CAPACITY_ERROR,仅在 base_sync_done and adapter_loaded(已有在跑策略)时降级为 SKIPPED——日志记录被跳过的 sync id、锁正常释放、all_reduce 同步 0(不失败)、registry 从未见过被拒候选(无残留条目),下一步以新 version_id 重试;bootstrap 与 PUBLICATION_BLOCKED/VERSION_CONFLICT/歧义引擎错误仍为致命(有专门测试钉住)。配套测试还验证了旧 Session 绑定保持、无 Begin/NCCL RPC、释放后下一次同步成功发布并回收 v1。同时 7fdc029 修正了未暂停时不再调用 /continue_generation。训练 run 不会再因慢 Session 终止。

@rai-studio-bot rai-studio-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

最终结论:需要修改(深度审查已完成)

常规与深度审查均已完成。生产实现(registry 状态机、staged publication、Session 绑定与回收、无透明重发)经独立设计对照确认为满足 RFC #369 的必要机制,无经证实的冗余;测试套件经 11 个定向变异验证检测力(全部被预期测试捕获),无 mock-only 套件。当前未解决问题:1 个 P1(test_lora_staged_publication.py 未加 importorskip 守卫,无 sglang 的 CI 收集失败)、3 个 P2(两引擎 fleet 校验缺失;容量拒绝在旧 Session 存活时使训练 run 致命终止;versioned_staged 免暂停分支缺 CPU 回归测试)、1 个 P3(Registry 墓碑无界增长),详见各行内讨论与看板。

看板:#382 (comment)

Powered by Nyanpasu with glm-5.3[1m] xhigh, please check the suggestions carefully.


if not actor_fwd_only:
if not actor_fwd_only and not versioned_staged:
if dist.get_rank() == 0:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Medium severity P2 · 补充 versioned_staged 免暂停/免 flush 分支的 CPU 回归测试

本 PR 的核心行为主张(RFC §11.2:base 同步后,adapter-only 发布不再全局 pause generation、不再 flush KV cache——这正是旧 Session 能在发布期间继续生成的前提)目前没有任何常规 CI 测试执行:

  • 全仓 grep 证实唯一驱动该路径的测试是 test_device_direct_uses_sync_identity_for_new_and_replayed_publications,它直接调用 _publish_lora_adapter_versioned(),绕过了 update_weights_for_rollout 里的 versioned_staged 门控;
  • GPU 验收 harness 确实用引擎日志扫描断言了 /pause_generation、/flush_cache 等端点未被调用(run.py:347-359),但它是 RELAX_LORA_GPU_ACCEPTANCE=1 + 3 GPU 的 opt-in 实验,CI 不会运行。

也就是说,若该门控回归(例如条件被改回 if not actor_fwd_only:),所有 CPU CI 照常通过,只有手动跑 3-GPU 验收才会发现。建议在 tests/distributed/checkpoint_service/test_lora_publication.py 增加一个沿用现有 _FakeEngines 模式的测试:stub 出 update_weights_for_rollout 所需的最小 backend 状态,断言 (a) base_sync_done=True 时 adapter-only 同步不触发 pause/flush/continue,(b) 首次同步(bootstrap)仍然 pause+flush。现有 test_device_direct_uses_sync_identity... 已经 stub 了 backend.lock/_new_lora_publisher,扩展成本很低。

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

已验证修复(7fdc029):新增 test_device_direct_only_pauses_and_flushes_during_bootstrap 通过真实 update_weights_for_rollout 入口断言端点时间线——bootstrap 为 pause → flush → snapshot → continue,稳态版本化发布仅 snapshot(无 pause/flush/continue),正是建议的 CPU 回归测试(沿用现有 _FakeEngines 模式)。另有容量跳过与致命错误路径的入口级测试。覆盖缺口解决。

yxy added 3 commits October 3, 2026 01:18
# 🐛 Bug Fix

- Handle PR redai-studio#382 F4: after bootstrap, skip capacity-refused publications
  and keep the published policy until a later weight sync can reclaim space.
- Log the skipped sync identity while preserving lock release and rank
  synchronization. Bootstrap, protocol, and ambiguous engine failures remain fatal.

---

# ✅ Tests

- Exercise the training update entrypoint with a real registry and publisher:
  retain the old Session binding, send no candidate RPC, and publish after release.
- Cover source and peer rank control flow plus bootstrap and protocol failures.
- Publication suite: 39 passed. All-file pre-commit checks passed.
- GPU and multi-node integration were not run: no cluster validation was requested.
# 🐛 Bug Fix

- Address PR redai-studio#382 F5 by sending continue_generation only for updates that
  pause generation; staged adapter updates leave rollout control untouched.
- Preserve bootstrap pause, cache flush, publication, and resume ordering.

---

# ✅ Tests

- Exercise update_weights_for_rollout in CPU CI for bootstrap and subsequent
  publication, including failure cleanup. The new assertions caught the
  unconditional continue_generation call before the fix.
- Publication suite: 41 passed. All-file pre-commit checks passed.
- GPU and multi-node integration were not run: no cluster validation was requested.
# 🐛 Bug Fix

- Address PR redai-studio#382 F3 with idempotent batch tombstone removal after Session
  binding tasks settle, releases are confirmed, and local records are removed.
- Retain Group ownership until registry acknowledgement so cleanup can retry
  safely after failure or a lost reply; make duplicate group releases harmless.
- Reject forgetting any batch that still contains an active version binding.

---

# ✅ Tests

- Cover repeated release/drop cycles, active binding rejection, lost forget
  acknowledgements, delayed binding, cancellation, and late chat rejection.
- Registry and Agentic suites: 104 passed with the unrelated prepare-gate
  test deselected after it stalled during teardown in this environment.
- All-file pre-commit checks passed. GPU and multi-node integration were not
  run because no cluster validation was requested.

@rai-studio-bot rai-studio-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

最终结论:通过

五个审查发现(P1×1、P2×3、P3×1)已在 bcb285e→03a369b 的四个提交中全部修复并经逐项验证:importorskip 收集守卫、两引擎 fleet 启动期校验(含新测试)、forget_sessions 墓碑批量清理(调用顺序与幂等性正确、配套排线测试)、容量拒绝降级为良性跳过(bootstrap/协议错误保持致命并有测试钉住)、以及真实 update_weights_for_rollout 入口的免暂停/免 flush 时间线回归测试。增量复核未发现新问题;本地可运行套件通过(registry 48、utils 31)。深度审查(独立设计对照 + 测试套件审计)结论详见看板。建议合并前由维护者批准运行 CI workflow 以确认 03a369b 的全套测试(当前 fork PR 的 workflows 处于待批准状态)。

看板:#382 (comment)

Powered by Nyanpasu with glm-5.3[1m] xhigh, please check the suggestions carefully.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants