From b5c12b50e6a8ac1518f7b6e9c4d49a56b31fdc86 Mon Sep 17 00:00:00 2001 From: Sandeep Sukhani Date: Tue, 8 Feb 2022 20:34:07 +0530 Subject: [PATCH] Fix apply retention issue (#5342) * call mark phase started/finished only when applying retention * correctly pass userID to IntervalMayHaveExpiredChunks call on retention and dletion expiry check --- pkg/storage/stores/shipper/compactor/compactor.go | 8 ++++---- pkg/storage/stores/shipper/compactor/compactor_test.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/storage/stores/shipper/compactor/compactor.go b/pkg/storage/stores/shipper/compactor/compactor.go index 43d01f264bcf6..0aee339550376 100644 --- a/pkg/storage/stores/shipper/compactor/compactor.go +++ b/pkg/storage/stores/shipper/compactor/compactor.go @@ -408,7 +408,7 @@ func (c *Compactor) CompactTable(ctx context.Context, tableName string, applyRet interval := retention.ExtractIntervalFromTableName(tableName) intervalMayHaveExpiredChunks := false - if c.cfg.RetentionEnabled && applyRetention { + if applyRetention { intervalMayHaveExpiredChunks = c.expirationChecker.IntervalMayHaveExpiredChunks(interval, "") } @@ -424,7 +424,7 @@ func (c *Compactor) RunCompaction(ctx context.Context, applyRetention bool) erro status := statusSuccess start := time.Now() - if c.cfg.RetentionEnabled { + if applyRetention { c.expirationChecker.MarkPhaseStarted() } @@ -439,7 +439,7 @@ func (c *Compactor) RunCompaction(ctx context.Context, applyRetention bool) erro } } - if c.cfg.RetentionEnabled { + if applyRetention { if status == statusSuccess { c.expirationChecker.MarkPhaseFinished() } else { @@ -550,7 +550,7 @@ func (e *expirationChecker) MarkPhaseFinished() { } func (e *expirationChecker) IntervalMayHaveExpiredChunks(interval model.Interval, userID string) bool { - return e.retentionExpiryChecker.IntervalMayHaveExpiredChunks(interval, "") || e.deletionExpiryChecker.IntervalMayHaveExpiredChunks(interval, "") + return e.retentionExpiryChecker.IntervalMayHaveExpiredChunks(interval, userID) || e.deletionExpiryChecker.IntervalMayHaveExpiredChunks(interval, userID) } func (e *expirationChecker) DropFromIndex(ref retention.ChunkEntry, tableEndTime model.Time, now model.Time) bool { diff --git a/pkg/storage/stores/shipper/compactor/compactor_test.go b/pkg/storage/stores/shipper/compactor/compactor_test.go index c6c3238e1375c..820a3055d4f64 100644 --- a/pkg/storage/stores/shipper/compactor/compactor_test.go +++ b/pkg/storage/stores/shipper/compactor/compactor_test.go @@ -107,7 +107,7 @@ func TestCompactor_RunCompaction(t *testing.T) { cm := storage.NewClientMetrics() defer cm.Unregister() compactor := setupTestCompactor(t, tempDir, cm) - err := compactor.RunCompaction(context.Background(), true) + err := compactor.RunCompaction(context.Background(), false) require.NoError(t, err) for name := range tables {