Skip to content

Commit 2807755

Browse files
committed
Add sync manager protection to make sure only 1 go routine is triggered for a repo
1 parent 5f9b783 commit 2807755

2 files changed

Lines changed: 49 additions & 0 deletions

File tree

pkg/cache/sync/sync.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package sync
1616

1717
import (
1818
"context"
19+
"sync"
1920
"time"
2021

2122
configapi "github.com/nephio-project/porch/api/porchconfig/v1alpha1"
@@ -52,6 +53,8 @@ type SyncManager struct {
5253
lastCronExpr string
5354
lastSyncError error
5455
coreClient client.WithWatch
56+
mutex sync.Mutex
57+
running bool
5558
}
5659

5760
// NewSyncManager creates a new sync manager
@@ -65,6 +68,15 @@ func NewSyncManager(handler SyncHandler, coreClient client.WithWatch) *SyncManag
6568

6669
// Start begins the sync process with periodic and one-time scheduling
6770
func (m *SyncManager) Start(ctx context.Context, defaultSyncFrequency time.Duration) {
71+
m.mutex.Lock()
72+
defer m.mutex.Unlock()
73+
74+
if m.running {
75+
klog.Warningf("repositorySync %+v: already running, ignoring start request", m.handler.Key())
76+
return
77+
}
78+
m.running = true
79+
6880
// Create cancellable context for goroutines
6981
syncCtx, cancel := context.WithCancel(ctx)
7082
m.cancel = cancel
@@ -75,9 +87,13 @@ func (m *SyncManager) Start(ctx context.Context, defaultSyncFrequency time.Durat
7587

7688
// Stop stops the sync manager
7789
func (m *SyncManager) Stop() {
90+
m.mutex.Lock()
91+
defer m.mutex.Unlock()
92+
7893
if m.cancel != nil {
7994
m.cancel()
8095
}
96+
m.running = false
8197
}
8298

8399
// GetLastSyncError returns the last sync error

pkg/cache/sync/sync_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,39 @@ func TestSyncManager_Start(t *testing.T) {
348348
}
349349
}
350350

351+
func TestSyncManager_Start_PreventMultipleStarts(t *testing.T) {
352+
handler := &mockSyncHandler{
353+
repoKey: repository.RepositoryKey{Name: "test-repo", Namespace: "test-ns"},
354+
}
355+
manager := NewSyncManager(handler, nil)
356+
357+
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
358+
defer cancel()
359+
360+
// First start should succeed
361+
manager.Start(ctx, 1*time.Second)
362+
assert.True(t, manager.running, "Manager should be running after first start")
363+
364+
// Parallel starts should be ignored and not increase syncCount
365+
manager.Start(ctx, 1*time.Second)
366+
manager.Start(ctx, 1*time.Second)
367+
manager.Start(ctx, 1*time.Second)
368+
369+
// Wait to ensure no additional goroutines were started
370+
time.Sleep(100 * time.Millisecond)
371+
372+
// Sync count should not have increased
373+
assert.Equal(t, handler.syncCount, 1, "Parallel starts should not create additional sync activity")
374+
375+
// Stop and verify we can start again
376+
manager.Stop()
377+
assert.False(t, manager.running, "Manager should not be running after stop")
378+
379+
// Should be able to start again after stop
380+
manager.Start(ctx, 1*time.Second)
381+
assert.True(t, manager.running, "Manager should be running after restart")
382+
}
383+
351384
func TestSyncManager_HasValidSyncSpec(t *testing.T) {
352385
tests := []struct {
353386
name string

0 commit comments

Comments
 (0)