From b7017c251badaacaab840646a959635d00665e2d Mon Sep 17 00:00:00 2001 From: Seefs <40468931+seefs001@users.noreply.github.com> Date: Tue, 1 Sep 2026 20:54:17 +0800 Subject: [PATCH] fix(model): do not treat no-op system task state writes as lock loss (#7135) --- model/system_task.go | 18 +++++++++++++++++- model/system_task_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/model/system_task.go b/model/system_task.go index c811409b48..ffbe2dc1c9 100644 --- a/model/system_task.go +++ b/model/system_task.go @@ -322,7 +322,23 @@ func UpdateSystemTaskState(taskID string, lockedBy string, state any) error { if result.Error != nil { return result.Error } - if result.RowsAffected == 0 { + if result.RowsAffected > 0 { + return nil + } + // MySQL counts changed rows, not matched rows. A no-op persist of the same + // state in the same second therefore returns RowsAffected == 0 even while + // the lease is still held. Confirm the lock before treating this as loss. + // Reuse `now` from the UPDATE so a clock tick cannot reintroduce false + // lock-loss; a lease that expires during the write is caught by the next heartbeat. + var held int64 + err = DB.Model(&SystemTask{}). + Where("task_id = ? AND status = ? AND locked_by = ?", taskID, SystemTaskStatusRunning, lockedBy). + Where("EXISTS (SELECT 1 FROM system_task_locks WHERE system_task_locks.task_id = system_tasks.task_id AND system_task_locks.locked_by = ? AND system_task_locks.locked_until >= ?)", lockedBy, now). + Count(&held).Error + if err != nil { + return err + } + if held == 0 { return ErrSystemTaskLockLost } return nil diff --git a/model/system_task_test.go b/model/system_task_test.go index ac5678f74b..d0a4e5db6a 100644 --- a/model/system_task_test.go +++ b/model/system_task_test.go @@ -350,3 +350,34 @@ func TestSystemTaskUpdatesRequireUnexpiredLock(t *testing.T) { assert.Equal(t, SystemTaskStatusRunning, reloaded.Status) assert.Empty(t, reloaded.State) } + +func TestUpdateSystemTaskStateIdenticalPayloadDoesNotLoseLock(t *testing.T) { + // SQLite reports matched rows for unchanged UPDATEs, so this case passed + // even before the fix. The MySQL regression is covered by + // TestUpdateSystemTaskStateIdenticalPayloadDoesNotLoseLockConfiguredDatabases. + truncateTables(t) + runUpdateSystemTaskStateIdenticalPayloadKeepsLock(t, SystemTaskTypeLogCleanup) +} + +func runUpdateSystemTaskStateIdenticalPayloadKeepsLock(t *testing.T, taskType string) { + t.Helper() + // Two persists in the same second so MySQL's unchanged-row UPDATE returns 0. + + task, err := CreateSystemTask(taskType, nil, nil) + require.NoError(t, err) + + runnerID := "runner-a" + _, claimed, err := ClaimSystemTask(task.ID, taskType, runnerID, common.GetTimestamp()+60) + require.NoError(t, err) + require.True(t, claimed) + + state := testSystemTaskState{Total: 10, Processed: 10, Progress: 100, Remaining: 0} + require.NoError(t, UpdateSystemTaskState(task.TaskID, runnerID, state)) + require.NoError(t, UpdateSystemTaskState(task.TaskID, runnerID, state), "identical state persist must not be treated as lock loss") + + require.NoError(t, FinishSystemTask(task.TaskID, runnerID, SystemTaskStatusSucceeded, map[string]int64{"deleted_count": 10}, "")) + finished, err := GetSystemTaskByTaskID(task.TaskID) + require.NoError(t, err) + require.NotNil(t, finished) + assert.Equal(t, SystemTaskStatusSucceeded, finished.Status) +}