From 6dad3ad43deda3ab357040f6c07ab0b1c5eb17c8 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 2 Oct 2026 12:28:51 -0700 Subject: [PATCH] refactor: only update sync status columns when syncing push mirror (#39517) `UpdatePushMirror` used `AllCols()`, so a sync could overwrite columns changed concurrently (e.g. `interval`) with stale values. It now updates only `last_update` and `last_error`, and is renamed to `UpdatePushMirrorSyncStatus` to match. Co-authored-by: Giteabot Co-authored-by: silverwind Co-authored-by: Claude (Opus 5) --- models/repo/pushmirror.go | 8 ++++---- services/mirror/mirror_push.go | 4 ++-- tests/integration/mirror_push_test.go | 2 ++ 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/models/repo/pushmirror.go b/models/repo/pushmirror.go index 8c1dd4970b3..8aea602e6a8 100644 --- a/models/repo/pushmirror.go +++ b/models/repo/pushmirror.go @@ -78,13 +78,13 @@ func (m *PushMirror) GetRemoteName() string { return m.RemoteName } -// UpdatePushMirror updates the push-mirror -func UpdatePushMirror(ctx context.Context, m *PushMirror) error { - _, err := db.GetEngine(ctx).ID(m.ID).AllCols().Update(m) +// UpdatePushMirrorSyncStatus updates the sync status (last update time and last error) of the push-mirror +func UpdatePushMirrorSyncStatus(ctx context.Context, m *PushMirror) error { + _, err := db.GetEngine(ctx).ID(m.ID).Cols("last_update", "last_error").Update(m) return err } -// UpdatePushMirrorInterval updates the push-mirror +// UpdatePushMirrorInterval updates the sync interval of the push-mirror func UpdatePushMirrorInterval(ctx context.Context, m *PushMirror) error { _, err := db.GetEngine(ctx).ID(m.ID).Cols("interval").Update(m) return err diff --git a/services/mirror/mirror_push.go b/services/mirror/mirror_push.go index 3ce4d72d500..f3431468f60 100644 --- a/services/mirror/mirror_push.go +++ b/services/mirror/mirror_push.go @@ -109,8 +109,8 @@ func SyncPushMirror(ctx context.Context, mirrorID int64) bool { m.LastUpdateUnix = timeutil.TimeStampNow() - if err := repo_model.UpdatePushMirror(ctx, m); err != nil { - log.Error("UpdatePushMirror [%d]: %v", m.ID, err) + if err := repo_model.UpdatePushMirrorSyncStatus(ctx, m); err != nil { + log.Error("UpdatePushMirrorSyncStatus [%d]: %v", m.ID, err) return false } diff --git a/tests/integration/mirror_push_test.go b/tests/integration/mirror_push_test.go index 7defe9b94b9..6372ab856fe 100644 --- a/tests/integration/mirror_push_test.go +++ b/tests/integration/mirror_push_test.go @@ -55,6 +55,7 @@ func testMirrorPush(t *testing.T, u *url.URL) { ok := mirror_service.SyncPushMirror(t.Context(), mirrors[0].ID) assert.True(t, ok) + assert.NotZero(t, unittest.AssertExistsAndLoadBean(t, &repo_model.PushMirror{ID: mirrors[0].ID}).LastUpdateUnix) srcGitRepo, err := git.OpenRepository(t.Context(), srcRepo) assert.NoError(t, err) @@ -74,6 +75,7 @@ func testMirrorPush(t *testing.T, u *url.URL) { defer test.MockVariableValue(&setting.Migrations.AllowedHostList, "")() assert.False(t, mirror_service.SyncPushMirror(t.Context(), mirrors[0].ID)) + assert.NotEmpty(t, unittest.AssertExistsAndLoadBean(t, &repo_model.PushMirror{ID: mirrors[0].ID}).LastError) // Cleanup assert.True(t, doRemovePushMirror(t, session, user.Name, srcRepo.Name, mirrors[0].ID))