diff --git a/internal/datastore/common/gc.go b/internal/datastore/common/gc.go index 729739273e..fdf3430c24 100644 --- a/internal/datastore/common/gc.go +++ b/internal/datastore/common/gc.go @@ -196,7 +196,7 @@ func RunGarbageCollection(gc GarbageCollector, window, timeout time.Duration) er } defer func() { - err := gc.UnlockAfterGCRun(ctx) + err := gc.UnlockAfterGCRun(context.Background()) if err != nil { log.Error(). Err(err). diff --git a/internal/datastore/common/gc_test.go b/internal/datastore/common/gc_test.go index 60a1db203e..796efabf03 100644 --- a/internal/datastore/common/gc_test.go +++ b/internal/datastore/common/gc_test.go @@ -23,6 +23,8 @@ type fakeGC struct { deleter gcDeleter metrics gcMetrics lock sync.RWMutex + wasLocked bool + wasUnlocked bool } type gcMetrics struct { @@ -39,11 +41,27 @@ func newFakeGC(deleter gcDeleter) fakeGC { } } -func (*fakeGC) LockForGCRun(ctx context.Context) (bool, error) { +func (f *fakeGC) LockForGCRun(ctx context.Context) (bool, error) { + if f.wasLocked { + return false, fmt.Errorf("already locked") + } + f.wasLocked = true return true, nil } -func (*fakeGC) UnlockAfterGCRun(ctx context.Context) error { +func (f *fakeGC) UnlockAfterGCRun(ctx context.Context) error { + if ctx.Err() != nil { + return ctx.Err() + } + + if !f.wasLocked { + return fmt.Errorf("not locked") + } + + if f.wasUnlocked { + return fmt.Errorf("already unlocked") + } + f.wasUnlocked = true return nil } @@ -227,3 +245,23 @@ func TestGCFailureBackoffReset(t *testing.T) { // the GC enough time to run. require.Greater(t, gc.GetMetrics().markedCompleteCount, 20, "Next interval was not reset with backoff") } + +func TestGCUnlockOnTimeout(t *testing.T) { + gc := newFakeGC(alwaysErrorDeleter{}) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + go func() { + interval := 10 * time.Millisecond + window := 10 * time.Second + timeout := 1 * time.Millisecond + + require.Error(t, StartGarbageCollector(ctx, &gc, interval, window, timeout)) + }() + + time.Sleep(30 * time.Millisecond) + require.False(t, gc.HasGCRun(), "GC should not have run") + require.True(t, gc.wasLocked, "GC should have been locked") + require.True(t, gc.wasUnlocked, "GC should have been unlocked") +}