Repository navigation
Conversation
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.
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,各线程内已附验证回复。
审查发现待处理
已解决或已取代提交范围 · 接收 30 · 建议移出 0 · 待确认 0接收 30 个文件 · 建议移出 0 个文件 · 待确认 0 个文件。移出与待确认部分暂停深审,不代表审查通过。
精简审查与验证依据
生产代码的必要性与替代方案
测试的必要性与替代方案
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
left a comment
There was a problem hiding this comment.
结论:需要修改(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 墓碑集合无界增长)见行内评论。
深度审查(独立设计对照与测试检测力/必要性审计)仍在进行,结论随后在看板更新。
| from types import SimpleNamespace | ||
|
|
||
| import torch | ||
| from sglang.srt.lora.utils import verify_lora_tensor_checksums |
There was a problem hiding this comment.
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 上整包缺失):
| 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 |
| 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 |
There was a problem hiding this comment.
已验证修复(55f4c1b,head 03a369b):与建议一致的 importorskip 四个 sglang.srt.* 子模块守卫已就位。本地在无 sglang 环境(torch stub)下 --collect-only 复核:该文件现在正确跳过收集,不再报 ModuleNotFoundError。CI 的 Tests (Python 3.10/3.11/3.12) 收集阻塞解除。
| # 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. |
There was a problem hiding this comment.
版本化发布路径把目标 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:
| # 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 ( |
| # 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 ( |
There was a problem hiding this comment.
已验证修复(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) |
There was a problem hiding this comment.
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 路径已收敛)。
首版容量规模下影响很小,不阻塞;仅作为长跑部署的维护性记录。
There was a problem hiding this comment.
已验证修复(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
left a comment
There was a problem hiding this comment.
结论:仍需修改(新增 1 个 P2,P1 未解决)
深度审查的设计对照已完成:独立最小设计与本实现核心同构(版本化命名、单写 Registry、双引擎就绪才切 default、首代绑定、max_retries=1),作者还正确补上了独立设计遗漏的两处重发漏洞(router 层重试与 distributed POST 回退)。新增 1 个 P2:容量拒绝在旧 Session 仍存活时会使训练 run 致命终止而非跳过该次发布(触发条件即本特性的标称场景,详见行内评论);此前的 P1(CI 收集失败)在当前 head 仍未解决。测试套件的独立审计仍在进行,结论将在看板汇总。
| 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(): |
There was a problem hiding this comment.
这是深度设计对照得出的行为问题:当 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 即可在下一次权重同步时终止整个训练作业。
两个方向二选一(需要维护者定夺语义):
- 良性跳过(推荐讨论):在
_publish_lora_adapter_versioned捕获CAPACITY_ERROR,记 log/metric 后返回 SKIPPED 结果——本步不发布、继续用 B 生成,下一步以新 version_id 重试(weight_version 已递增,注册表从未见过被拒版本,状态干净)。fully-async 模式本身接受策略 staleness,这与_check_services_health对服务不可用的降级模式一致;同时应把「发布被旧 Session 阻塞」暴露为可观测指标。 - 显式声明 fail-closed:若维持现语义(拒绝发布即失败),至少在
--enable-versioned-lora-publication的 help 与 PR 描述中写明「旧 Session 阻塞回收时训练会失败」,让使用者可预期。
不建议保持现状:PR 描述 §6 只说明了「C 会在 Begin/NCCL 之前得到 CAPACITY_ERROR」,未说明这对训练循环意味着致命失败。
There was a problem hiding this comment.
已验证修复(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
left a comment
There was a problem hiding this comment.
最终结论:需要修改(深度审查已完成)
常规与深度审查均已完成。生产实现(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 墓碑无界增长),详见各行内讨论与看板。
|
|
||
| if not actor_fwd_only: | ||
| if not actor_fwd_only and not versioned_staged: | ||
| if dist.get_rank() == 0: |
There was a problem hiding this comment.
本 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,扩展成本很低。
There was a problem hiding this comment.
已验证修复(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 模式)。另有容量跳过与致命错误路径的入口级测试。覆盖缺口解决。
# 🐛 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
left a comment
There was a problem hiding this comment.
最终结论:通过
五个审查发现(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 处于待批准状态)。



What
实现 No.7:不可变 LoRA 版本的在线发布、Session 绑定与安全回收。
本 PR 在现有 Relax + SGLang LoRA adapter 路径上增加版本化 lifecycle,使在线权重更新期间:
RFC:Relates to #369
Why
现有 LoRA adapter 更新使用固定名称覆盖,无法表达:
这种 Agentic / fully-async 场景下需要的 policy consistency。
如果 Session 的多个工具轮次、retry 或 abort/resume 跨越一次权重更新,直接使用“当前最新 adapter”会使同一个 Session 在不同轮次实际执行不同 policy。
本 PR 将三个阶段明确分开:
并分别定义对应的 ownership:
因此 Relax 不需要复制一套后端 request lifecycle,同时又能保证旧 Session 和已经被 SGLang 接纳的请求不会被提前卸载。
How
1. 不可变版本身份
每次 publication 使用:
version_id表示一次 publication event,digest 表示该版本的实际内容,两者不再混为同一个 identity。Registry 使用:
维护
(deployment_epoch, version_id) -> digest。语义为:
VERSION_CONFLICT;因此:
中的 v3 会正常经过容量准入、加载、READY 和 commit,而不会因为 digest 与 v1 相同被错误当成旧版本 replay。
已完成版本的迟到 replay 只返回
NO_OP,不会重新加载,也不会把 default 回滚到历史版本。2. staged publication
训练侧首先通过现有 adapter export/gather 路径得到一份冻结 snapshot。
snapshot 固定:
后续摘要计算和 DCS/NCCL 传输全部使用同一份 snapshot,不再读取持续变化的 live 参数。
publication 顺序为:
claim_publication()保证一个 attempt 只有一个 transport owner,避免不确定状态下重复驱动 NCCL collective。READY receipt 会校验:
只有 E0/E1 均属于同一个 publication attempt 并确认 READY 后,Registry 才允许 commit。
3. 唯一 default 切换点
LoRAVersionRegistry.mark_published()是唯一的 default linearization point:因此:
时,default 仍然是 A。
只有两个目标都完成 READY 并成功 commit 后,新 Session 才会看到 B。
4. Session 首次生成绑定
Session 创建时不立即选择版本。
第一次真正执行 generation 时:
Registry 在同一个 actor turn 内读取 default 并记录 Session binding,因此 bind 与 publication commit 之间存在确定顺序:
绑定保存在
_SessionRecord中,而不是某个单独 IR。后续:
均继续使用相同的 immutable
lora_path,不再重新读取最新 default。首次绑定使用 Session 自己持有的共享 task,并通过
asyncio.shield()等待;取消某一个 IR waiter 不会取消整个 binding,也不会让后续请求重新选择版本。5. 禁止底层透明重发
对于 versioned generation:
Router retry 也被关闭。
原因是 HTTP timeout / Ray reply loss 并不能证明后端没有接受请求。
例如:
这时不能再由 local HTTP client 自动发送第二次。
是否重新执行 generation 只能由 Agentic IR lifecycle 根据自身状态明确决定。
6. 安全回收
新版本 B commit 后,A 只进入:
仍然允许已绑定 A 的旧 Session 继续生成。
只有当没有 Session binding 后:
每个 SGLang engine 随后执行:
只有两个目标均确认卸载完成后:
逻辑容量才真正释放。
因此:
时,C 会在任何 Begin / NCCL 操作之前得到
CAPACITY_ERROR。7. SGLang lifecycle 修复
本 PR 继续复用 SGLang 原生 request refcount 和 LoRA registry 作为后端 request lifetime authority,只修复会破坏该 contract 的具体问题,包括:
wait_for_unload;因此 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: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:
唯一 skip 为 opt-in GPU experiment。
全仓:
通过。
覆盖场景包括:
lora_pathpropagation;主要回归命令:
GPU acceptance
此前限定 GPU acceptance 已完成真实:
验证内容包括:
历史确定性联合实验完成:
publication 总耗时 5.854 s,其中包含人为保持的 3 s READY window。
publication 开始到 E0 READY 的传输/准备阶段为 2.752 s,该阶段 E0/E1 各完成 9 个正常请求。
最长无完成间隔:
预设上限为:
请求延迟:
512-token A 请求在 reclaim 期间正常完成,之后才允许物理卸载;两个 engine 各卸载一次。
双向 KV 实验中,首次跨版本目标请求保持 cold,再次同版本请求命中自身缓存,数值均与统一 cold baseline 匹配。
Scope
当前首版验证范围:
n=1;RadixTreeMiddleware;本 PR 不声称已经完成:
其中部分执行路径从代码结构上可能可以扩展,但本 PR 只声明实际验证过的范围。
Type of Change
Checklist
pre-commit run --all-files