Repository navigation
Conversation
1dab903 to
e471cb4
Compare
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (79.50%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. @@ Coverage Diff @@
## main #709 +/- ##
==========================================
- Coverage 78.91% 78.90% -0.01%
==========================================
Files 709 710 +1
Lines 88456 88538 +82
==========================================
+ Hits 69802 69863 +61
- Misses 14627 14640 +13
- Partials 4027 4035 +8
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
xueyizheng
left a comment
xueyizheng
left a comment
There was a problem hiding this comment.
共享 Agent Workspace CR，审查 head e471cb478f174b818af2015cff65e5aaca898021。
执行记录和 EvalTarget span 归实验空间的方向正确。已贴三条 P2 行级意见：Windows 来源清理参数遗漏、旧 invocation 升级恢复盲区、历史运行次数/重试明细漏读来源空间。
全链扫描中还需要保留以下区分：
| 字段或出口 | 正确归属 |
|---|---|
| Experiment.SpaceID、事件 SpaceID、ExptTurnResultRunLog.SpaceID | 实验 E；turn run log 的创建点原本已用 event.SpaceID |
| Target/Version.SpaceID、Experiment.TargetSpaceID | 共享对象来源 A |
| 新 SandboxAgent record、EvalTarget span、执行指标 | 实验 E；debug 与其他 target 类型保持原口径 |
| async context.Event.SpaceID、结果回调 workspace | 实验事件 E；回调必须与该次 record 的实际归属一致 |
| sandbox_workspace_id、scheduler_space_id、Destroy.WorkspaceID | 执行真实创建空间；旧记录应保留历史值 |
| Destroy.SourceWorkspaceID | 对象来源配置/凭据空间 A |
| ItemComplete.ExptWorkspaceID | E |
| ItemComplete.EvalTargetWorkspaceID | A |
| ItemComplete.DatasetWorkspaceID | 数据集自己的来源空间 |
另有两项兼容边界，建议在这次迁移中一并说明或补回归：
- 公共
GetEvalTargetRecord仍严格按请求 workspace 查记录。实验结果 BatchGet 的旧 A fallback 不会覆盖 UI/CLI 的 direct GET；历史页面可能仍取不到旧记录日志与 execution metadata。应使用有实验授权上下文的兼容读，或从实验结果传递日志地址和真实创建空间，保留空间鉴权。 - 新旧版本消费者混跑时，旧基线
93eb2ed4的 target result 恢复仍只读 A，新版本写 E 后旧消费者可能读到 nil；旧销毁又用 record.SpaceID=E 查 A 的 target version，可能跳过收尾。是否触发取决于发布/消息路由隔离，尚未在线验证。建议先发布读兼容、再切写，或证明新记录事件不会被旧消费者承接；回滚也要考虑已写入的 E 记录。
本次是静态执行链和持久化契约审查，未做线上历史迁移或滚动混版本实验。未把检索命中数当作全部代码均经完整语义验证。
| } | ||
| } | ||
| _ = l.service.destroySandboxExecute(cleanupCtx, strconv.FormatInt(gptr.Indirect(l.param.ExperimentID), 10), l.schedulerSpaceID(), l.spaceID, sandboxExecuteIDsOf(cleanupCtx, l.record), false) | ||
| _ = l.service.destroySandboxExecute(cleanupCtx, strconv.FormatInt(gptr.Indirect(l.param.ExperimentID), 10), l.schedulerSpaceID(), resolveLoadSpaceID(l.spaceID, l.target.SpaceID), sandboxExecuteIDsOf(cleanupCtx, l.record), false) |
There was a problem hiding this comment.
[P2] lifecycle.spaceID 迁移后，两处 Destroy 的来源配置空间未同步
这里把 sandboxInvocationLifecycle.spaceID 从 Agent 来源 A 改成实验 E，但 sandbox_invocation.go:135 的中断初始化清理和 :188 的 finish 检查失败清理，仍把 l.spaceID 当 sourceSpaceID 传给 destroySandboxExecute。
destroySandboxExecuteSync 会把它写入 SandboxDestroyRequest.SourceWorkspaceID。下游按这个字段取来源的取消收尾配置；A/E 配置不同且走上述分支时，会读取 E 的模板。正常成功销毁、abort、批量取消本次已分别改为真实 target 来源，只有这两处遗漏。
建议两处来源参数同样用 resolveLoadSpaceID(l.spaceID, l.target.SpaceID)，scheduler 空间继续保持创建执行时的归属。增加 A/E 取消模板不同的回归。本问题不表示全部 Windows 销毁失败。
| ctx = looptracer.GetTracer().Inject(ctx) | ||
|
|
||
| lifecycle := &sandboxInvocationLifecycle{service: e, spaceID: spaceID, target: target, param: param, input: inputData, traceID: span.GetTraceID()} | ||
| lifecycle := &sandboxInvocationLifecycle{service: e, spaceID: recordSpaceID, target: target, param: param, input: inputData, traceID: span.GetTraceID()} |
There was a problem hiding this comment.
[P2] 跨升级重投时，Windows durable invocation 查不到旧来源空间记录
这里将 lifecycle.spaceID 改成 E 后，sandbox_invocation.go:87 的 LatestSandboxInvocation 仍只查该空间，repo SQL 也严格过滤 space_id。
如果升级前同 run/item/turn 已有 A 空间的 ready/initializing invocation，但 TargetResultID 尚未挂到 turn，升级后事件重投会在 E 查空，绕过旧 ready 复用或 initializing fence，进入新 invocation 的初始化流程；下游旧 ID 为 0 的分支会新建 VM、重跑初始化。
建议在授权实验范围内兼容旧 A 的 Latest 查找，并在复用后使用旧 record 的真实空间执行后续 check、loadReported 与 CAS。增加同 run/item/turn 跨 A/E 的升级恢复回归。源码与调用方已核，尚未做滚动升级实测，按 P2 记录。
| // 前端 case 旁不显示 aiosession。按来源空间读 (0=同调用方空间)。 | ||
| targetRecords, err := e.evalTargetService.BatchGetRecordByIDs(ctx, resolveLoadSpaceID(e.SpaceID, e.exptDO.TargetSpaceID), targetResultIDs) | ||
|
|
||
| targetRecords, err := BatchGetExperimentTargetRecords(ctx, e.evalTargetService, e.SpaceID, e.exptDO.TargetSpaceID, targetResultIDs) |
There was a problem hiding this comment.
[P2] Get/BatchGet 兼容旧来源空间，但运行次数与重试日志聚合仍只查实验空间
这里通过 BatchGetExperimentTargetRecords 保留旧 A 记录的读取；同文件 :1438 的 CountItemRunsByRun 与 :1455 的 ListItemRetryRecordsByRun 仍只传 builder.SpaceID=E，两个 DAO 都按 space_id 等值过滤。
旧共享实验的 attempts 在 A，会继续显示 total_runs=0、retry_records 为空；同一 run 跨升级时也只统计 E 中的新 attempts，漏掉 A 中的失败尝试和日志。这是已有共享功能的历史兼容缺口，不是新实验的执行故障。
建议用授权实验的 E 和冻结 A 联合读取、按 record ID 去重。Count 保持原聚合口径：先合并每个 item/turn 的次数，再取 item 下最大的 turn 次数，不能直接相加两个空间各自的 MAX。覆盖旧 A、纯新 E、同 run 混存 A/E 三类数据。
Shared Sandbox Agent runs currently persist execution records in the Agent's source workspace. The experiment workspace cannot retrieve those records, so the item page falls back to a fabricated BOE log URL.
Persist new Sandbox Agent records and control traces in the experiment workspace while keeping target definitions and configuration in their source workspace. Resolve legacy records through the already-authorized experiment's frozen source reference. Update async resume, result/field reads, retry reuse, aggregates, cancellation and timeout cleanup. Preserve source-specific cleanup configuration and account-pool release.
Validation: evaluation service/application tests, focused ownership/compatibility tests, race checks and GitHub CI pass. Internal PPE tests verified successful two-round execution, expected startup failure with one retry, legacy result reads, log downloads, control-trace reads, aggregates, and cancellation with both sandboxes canceled.
Full independent backflow validation remains pending: the production backflow worker uses the production evaluation reader, which cannot consume the new records produced in PPE. The planned validation uses an API-only analysis instance in the same PPE and its single-record synchronous backflow endpoint. TCC access for that dependent PPE configuration is currently blocked. Keep reader/writer versions coordinated during rollout; this PR remains a draft.
Commercial companion MR and internal validation evidence: https://code.byted.org/flowdevops/cozeloop-commercial/merge_requests/975