From 5759b5660d93565fa05e26679b14e573daecbf10 Mon Sep 17 00:00:00 2001 From: xueyizheng Date: Thu, 28 May 2026 16:42:15 +0800 Subject: [PATCH 1/4] fix: align LogRun lock value with retry path so retry-items can read runID --- .../domain/service/expt_manage_execution_impl.go | 2 +- .../service/expt_manage_execution_impl_test.go | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/backend/modules/evaluation/domain/service/expt_manage_execution_impl.go b/backend/modules/evaluation/domain/service/expt_manage_execution_impl.go index 12fc7a591a..09eb5c93ba 100644 --- a/backend/modules/evaluation/domain/service/expt_manage_execution_impl.go +++ b/backend/modules/evaluation/domain/service/expt_manage_execution_impl.go @@ -1039,7 +1039,7 @@ func (e *ExptMangerImpl) unlockCompletingRun(ctx context.Context, exptID, exptRu func (e *ExptMangerImpl) LogRun(ctx context.Context, exptID, exptRunID int64, mode entity.ExptRunMode, spaceID int64, itemIDs []int64, session *entity.Session) error { duration := time.Duration(e.configer.GetExptExecConf(ctx, spaceID).GetZombieIntervalSecond()) * time.Second - locked, err := e.mutex.LockBackoff(ctx, e.makeExptMutexLockKey(exptID), duration, time.Second) + locked, _, err := e.mutex.BackoffLockWithValue(ctx, e.makeExptMutexLockKey(exptID), strconv.FormatInt(exptRunID, 10), duration, time.Second) if err != nil { return err } diff --git a/backend/modules/evaluation/domain/service/expt_manage_execution_impl_test.go b/backend/modules/evaluation/domain/service/expt_manage_execution_impl_test.go index b2cb7c85ae..e44358c097 100755 --- a/backend/modules/evaluation/domain/service/expt_manage_execution_impl_test.go +++ b/backend/modules/evaluation/domain/service/expt_manage_execution_impl_test.go @@ -554,8 +554,8 @@ func TestExptMangerImpl_LogRun(t *testing.T) { setup: func() { mgr.mutex.(*lockMocks.MockILocker). EXPECT(). - LockBackoff(ctx, gomock.Any(), gomock.Any(), time.Second). - Return(true, nil) + BackoffLockWithValue(ctx, gomock.Any(), "456", gomock.Any(), time.Second). + Return(true, "", nil) mgr.mtr.(*metricsMocks.MockExptMetric). EXPECT(). @@ -595,8 +595,8 @@ func TestExptMangerImpl_LogRun(t *testing.T) { setup: func() { mgr.mutex.(*lockMocks.MockILocker). EXPECT(). - LockBackoff(ctx, gomock.Any(), gomock.Any(), time.Second). - Return(false, nil) + BackoffLockWithValue(ctx, gomock.Any(), "456", gomock.Any(), time.Second). + Return(false, "", nil) }, wantErr: true, }, @@ -609,8 +609,8 @@ func TestExptMangerImpl_LogRun(t *testing.T) { setup: func() { mgr.mutex.(*lockMocks.MockILocker). EXPECT(). - LockBackoff(ctx, gomock.Any(), gomock.Any(), time.Second). - Return(true, nil) + BackoffLockWithValue(ctx, gomock.Any(), "456", gomock.Any(), time.Second). + Return(true, "", nil) mgr.mtr.(*metricsMocks.MockExptMetric). EXPECT(). @@ -632,8 +632,8 @@ func TestExptMangerImpl_LogRun(t *testing.T) { setup: func() { mgr.mutex.(*lockMocks.MockILocker). EXPECT(). - LockBackoff(ctx, gomock.Any(), gomock.Any(), time.Second). - Return(true, nil) + BackoffLockWithValue(ctx, gomock.Any(), "456", gomock.Any(), time.Second). + Return(true, "", nil) mgr.mtr.(*metricsMocks.MockExptMetric). EXPECT(). From a51918f804c3cede76affc9809ba13255cbb8e55 Mon Sep 17 00:00:00 2001 From: xueyizheng Date: Fri, 29 May 2026 17:22:45 +0800 Subject: [PATCH 2/4] fix: call RetryItems on retried branch so scheduler picks up appended items MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Without this, retrying a single item on a Running experiment quietly fails: the item id is appended to expt_run_log.item_ids but no ExptScheduleEvent is published, so the scheduler never seeds expt_item_run_logs / resets turn results. The retry "succeeds" at the API layer while the item stays in its old (failed) state. Calling RetryItems with the same runID is safe and idempotent — the event carries only the user's requested itemIDs, and resetEvalItems uses a fresh schedule scope per (exptID, exptRunID). --- backend/modules/evaluation/application/experiment_app.go | 8 +++----- .../modules/evaluation/application/experiment_app_test.go | 3 ++- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/backend/modules/evaluation/application/experiment_app.go b/backend/modules/evaluation/application/experiment_app.go index f350f1fca9..9734d025ea 100644 --- a/backend/modules/evaluation/application/experiment_app.go +++ b/backend/modules/evaluation/application/experiment_app.go @@ -1254,16 +1254,14 @@ func (e *experimentApplication) RetryExperiment(ctx context.Context, req *expt.R switch runMode { case entity.EvaluationModeRetryItems: - rid, retried, err := e.manager.LogRetryItemsRun(ctx, req.GetExptID(), runMode, req.GetWorkspaceID(), req.GetItemIds(), session) + rid, _, err := e.manager.LogRetryItemsRun(ctx, req.GetExptID(), runMode, req.GetWorkspaceID(), req.GetItemIds(), session) if err != nil { return nil, err } runID = rid - if !retried { - if err := e.manager.RetryItems(ctx, req.GetExptID(), runID, req.GetWorkspaceID(), gptr.Indirect(got.EvalConf.ItemRetryNum), req.GetItemIds(), session, req.GetExt()); err != nil { - return nil, err - } + if err := e.manager.RetryItems(ctx, req.GetExptID(), runID, req.GetWorkspaceID(), gptr.Indirect(got.EvalConf.ItemRetryNum), req.GetItemIds(), session, req.GetExt()); err != nil { + return nil, err } default: if runID, err = e.idgen.GenID(ctx); err != nil { diff --git a/backend/modules/evaluation/application/experiment_app_test.go b/backend/modules/evaluation/application/experiment_app_test.go index d70d9df3b0..6df8e2e75d 100644 --- a/backend/modules/evaluation/application/experiment_app_test.go +++ b/backend/modules/evaluation/application/experiment_app_test.go @@ -6895,7 +6895,8 @@ func TestExperimentApplication_RetryExperiment_Branches(t *testing.T) { mockManager.EXPECT().Get(gomock.Any(), validExptID, validWorkspaceID, gomock.Any()).Return(baseExpt, nil) mockAuth.EXPECT().AuthorizationWithoutSPI(gomock.Any(), gomock.Any()).Return(nil) mockManager.EXPECT().LogRetryItemsRun(gomock.Any(), validExptID, entity.EvaluationModeRetryItems, validWorkspaceID, []int64{1}, gomock.Any()).Return(validRunID, true, nil) - // RetryItems should NOT be called since retried=true + // RetryItems IS called even when retried=true, so the scheduler picks up appended items + mockManager.EXPECT().RetryItems(gomock.Any(), validExptID, validRunID, validWorkspaceID, itemRetryNum, []int64{1}, gomock.Any(), gomock.Any()).Return(nil) resp, err := app.RetryExperiment(context.Background(), &exptpb.RetryExperimentRequest{ WorkspaceID: gptr.Of(validWorkspaceID), From 510ea02b487bb32878588875e0236698e4612cf7 Mon Sep 17 00:00:00 2001 From: xueyizheng Date: Mon, 1 Jun 2026 14:40:52 +0800 Subject: [PATCH 3/4] fix: scope retry-items idem key by item set so repeated retries can re-reset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ExptRetryItemsExec.ExptStart's idem key was (exptID, exptRunID) only. Once the first retry on a run wrote the key, every subsequent retry on the same run short-circuited at the idem.Exist check, skipping resetEvalItems entirely. The user-facing effect was that an item appended to a Running experiment got its row into expt_run_log.item_ids but its expt_run_id / expt_item_result_run_log / turn_result.status were never updated — UI stayed on the stale state. Switch the retry-items idem key to (exptID, exptRunID, sha1(sorted itemIDs)). MQ redelivery protection is preserved because identical event payloads produce identical hashes; the only relaxation is letting genuinely-new item sets through. Other schedule modes (Submit / FailRetry / Append / RetryAll) keep using makeStartIdemKey, so their behavior is unchanged. --- .../service/expt_run_scheduler_mode_impl.go | 21 ++++++++++++- .../expt_run_scheduler_mode_impl_test.go | 31 +++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/backend/modules/evaluation/domain/service/expt_run_scheduler_mode_impl.go b/backend/modules/evaluation/domain/service/expt_run_scheduler_mode_impl.go index 855f769e50..2fdf7fe447 100644 --- a/backend/modules/evaluation/domain/service/expt_run_scheduler_mode_impl.go +++ b/backend/modules/evaluation/domain/service/expt_run_scheduler_mode_impl.go @@ -5,7 +5,10 @@ package service import ( "context" + "crypto/sha1" "fmt" + "sort" + "strconv" "time" "github.com/bytedance/gg/gptr" @@ -1305,6 +1308,22 @@ func makeStartIdemKey(event *entity.ExptScheduleEvent) string { return fmt.Sprintf("expt_start:%v%v", event.ExptID, event.ExptRunID) } +// makeRetryItemsStartIdemKey 为 RetryItems 模式生成 idem key。 +// RetryItems 允许同一个 expt_run_id 上反复追加不同 item，因此 key 必须把 +// ExecEvalSetItemIDs 纳入指纹，否则前一次 retry 写下的 key 会让后续 retry +// 在 ExptStart 短路返回，跳过 resetEvalItems。MQ 重投递场景下 itemIDs 完全 +// 一致 → 指纹相同 → 仍然能正确去重。 +func makeRetryItemsStartIdemKey(event *entity.ExptScheduleEvent) string { + ids := append([]int64(nil), event.ExecEvalSetItemIDs...) + sort.Slice(ids, func(i, j int) bool { return ids[i] < ids[j] }) + h := sha1.New() + for _, id := range ids { + _, _ = h.Write(strconv.AppendInt(nil, id, 10)) + _, _ = h.Write([]byte{','}) + } + return fmt.Sprintf("expt_start:%v%v:%x", event.ExptID, event.ExptRunID, h.Sum(nil)) +} + func makeEndIdemKey(event *entity.ExptScheduleEvent) string { return fmt.Sprintf("expt_end:%v%v", event.ExptID, event.ExptRunID) } @@ -1600,7 +1619,7 @@ func (e *ExptRetryItemsExec) Mode() entity.ExptRunMode { } func (e *ExptRetryItemsExec) ExptStart(ctx context.Context, event *entity.ExptScheduleEvent, expt *entity.Experiment) error { - idemKey := makeStartIdemKey(event) + idemKey := makeRetryItemsStartIdemKey(event) exist, err := e.idem.Exist(ctx, idemKey) if err != nil { return err diff --git a/backend/modules/evaluation/domain/service/expt_run_scheduler_mode_impl_test.go b/backend/modules/evaluation/domain/service/expt_run_scheduler_mode_impl_test.go index 34962922a4..d9116dbf9e 100644 --- a/backend/modules/evaluation/domain/service/expt_run_scheduler_mode_impl_test.go +++ b/backend/modules/evaluation/domain/service/expt_run_scheduler_mode_impl_test.go @@ -5682,3 +5682,34 @@ func TestExptTrialRunExec_ExptStart_OriginalPath(t *testing.T) { }) } } + + +func TestMakeRetryItemsStartIdemKey(t *testing.T) { + base := &entity.ExptScheduleEvent{ExptID: 100, ExptRunID: 200} + + // 同 (exptID, runID, itemIDs) → 同 key（MQ 重投递保护不变） + a := *base + a.ExecEvalSetItemIDs = []int64{1, 2, 3} + b := *base + b.ExecEvalSetItemIDs = []int64{1, 2, 3} + assert.Equal(t, makeRetryItemsStartIdemKey(&a), makeRetryItemsStartIdemKey(&b)) + + // itemIDs 顺序变了 → 同 key（依赖排序） + c := *base + c.ExecEvalSetItemIDs = []int64{3, 1, 2} + assert.Equal(t, makeRetryItemsStartIdemKey(&a), makeRetryItemsStartIdemKey(&c)) + + // 同 (exptID, runID) 不同 itemIDs → 不同 key（修复点） + d := *base + d.ExecEvalSetItemIDs = []int64{1, 2, 4} + assert.NotEqual(t, makeRetryItemsStartIdemKey(&a), makeRetryItemsStartIdemKey(&d)) + + // 不同 (exptID, runID) → 不同 key + e := entity.ExptScheduleEvent{ExptID: 100, ExptRunID: 999, ExecEvalSetItemIDs: []int64{1, 2, 3}} + assert.NotEqual(t, makeRetryItemsStartIdemKey(&a), makeRetryItemsStartIdemKey(&e)) + + // 空 itemIDs 不 panic + empty := *base + empty.ExecEvalSetItemIDs = nil + assert.NotEmpty(t, makeRetryItemsStartIdemKey(&empty)) +} From b8b09e34251204270b3b39401cf193d484db9bf5 Mon Sep 17 00:00:00 2001 From: xueyizheng Date: Tue, 2 Jun 2026 14:37:16 +0800 Subject: [PATCH 4/4] style: gofmt expt_run_scheduler_mode_impl_test.go Co-Authored-By: Claude Opus 4.7 (1M context) --- .../domain/service/expt_run_scheduler_mode_impl_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/backend/modules/evaluation/domain/service/expt_run_scheduler_mode_impl_test.go b/backend/modules/evaluation/domain/service/expt_run_scheduler_mode_impl_test.go index d9116dbf9e..26d7c0b52b 100644 --- a/backend/modules/evaluation/domain/service/expt_run_scheduler_mode_impl_test.go +++ b/backend/modules/evaluation/domain/service/expt_run_scheduler_mode_impl_test.go @@ -5683,7 +5683,6 @@ func TestExptTrialRunExec_ExptStart_OriginalPath(t *testing.T) { } } - func TestMakeRetryItemsStartIdemKey(t *testing.T) { base := &entity.ExptScheduleEvent{ExptID: 100, ExptRunID: 200}