diff --git a/pkg/storage/tsdb/users_scanner.go b/pkg/storage/tsdb/users_scanner.go index 21ded4701f..0ade0f7a10 100644 --- a/pkg/storage/tsdb/users_scanner.go +++ b/pkg/storage/tsdb/users_scanner.go @@ -7,10 +7,7 @@ package tsdb import ( "context" - "path" - "sort" "strings" - "time" "github.com/go-kit/log" "github.com/go-kit/log/level" @@ -51,8 +48,6 @@ func (s *UsersScanner) ScanUsers(ctx context.Context) (users, markedForDeletion // Check users for being owned by instance, and split users into non-deleted and deleted. // We do these checks after listing all users, to improve cacheability of Iter (result is only cached at the end of Iter call). - now := time.Now().Unix() - var mtimes []int64 for ix := 0; ix < len(users); { userID := users[ix] @@ -74,27 +69,9 @@ func (s *UsersScanner) ScanUsers(ctx context.Context) (users, markedForDeletion continue } - // TODO: can't reference bucketindex.IndexCompressedFilename because of a circular dependency. - indexFile := path.Join(userID, "bucket-index.json.gz") - attr, err := s.bucketClient.Attributes(ctx, indexFile) - if err != nil { - level.Warn(s.logger).Log("msg", "unable to check user bucket index", "user", userID, "err", err) - mtimes = append(mtimes, now) - } else { - mtimes = append(mtimes, attr.LastModified.Unix()) - } ix++ } - // sort users increasing by bucket index's LastModified time - sort.SliceStable(users, func(i, j int) bool { - if mtimes[i] < mtimes[j] { - mtimes[i], mtimes[j] = mtimes[j], mtimes[i] - return true - } - return false - }) - return users, markedForDeletion, nil } diff --git a/pkg/storage/tsdb/users_scanner_test.go b/pkg/storage/tsdb/users_scanner_test.go index 1296be9924..0af741d777 100644 --- a/pkg/storage/tsdb/users_scanner_test.go +++ b/pkg/storage/tsdb/users_scanner_test.go @@ -10,12 +10,10 @@ import ( "errors" "path" "testing" - "time" "github.com/go-kit/log" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/thanos-io/objstore" "github.com/grafana/mimir/pkg/storage/bucket" ) @@ -30,10 +28,6 @@ func TestUsersScanner_ScanUsers_ShouldReturnedOwnedUsersOnly(t *testing.T) { return userID == "user-1" || userID == "user-3", nil } - bucketClient.MockAttributes(path.Join("user-1", "bucket-index.json.gz"), - objstore.ObjectAttributes{LastModified: time.Now().Add(-20 * time.Minute)}, nil, - ) - s := NewUsersScanner(bucketClient, isOwned, log.NewNopLogger()) actual, deleted, err := s.ScanUsers(context.Background()) require.NoError(t, err) @@ -49,10 +43,6 @@ func TestUsersScanner_ScanUsers_ShouldReturnUsersForWhichOwnerCheckOrTenantDelet bucketClient.MockExists(path.Join("user-1", TenantDeletionMarkPath), false, nil) bucketClient.MockExists(path.Join("user-2", TenantDeletionMarkPath), false, errors.New("fail")) - attrs := objstore.ObjectAttributes{LastModified: time.Now().Add(-20 * time.Minute)} - bucketClient.MockAttributes(path.Join("user-1", "bucket-index.json.gz"), attrs, nil) - bucketClient.MockAttributes(path.Join("user-2", "bucket-index.json.gz"), attrs, nil) - isOwned := func(string) (bool, error) { return false, errors.New("failed to check if user is owned") } @@ -70,70 +60,8 @@ func TestUsersScanner_ScanUsers_ShouldNotReturnPrefixedUsedByMimirInternals(t *t bucketClient.MockExists(path.Join("user-1", TenantDeletionMarkPath), false, nil) bucketClient.MockExists(path.Join("user-2", TenantDeletionMarkPath), false, nil) - attrs := objstore.ObjectAttributes{LastModified: time.Now().Add(-20 * time.Minute)} - bucketClient.MockAttributes(path.Join("user-1", "bucket-index.json.gz"), attrs, nil) - bucketClient.MockAttributes(path.Join("user-2", "bucket-index.json.gz"), attrs, nil) - s := NewUsersScanner(bucketClient, AllUsers, log.NewNopLogger()) actual, _, err := s.ScanUsers(context.Background()) require.NoError(t, err) assert.Equal(t, []string{"user-1", "user-2"}, actual) } - -func TestUsersScanner_ScanUsers_ShouldReturnConsistentOrder(t *testing.T) { - expected := []string{"user-2", "user-3", "user-4", "user-1"} - - users := []struct { - ID string - indexAttrs objstore.ObjectAttributes - }{ - {"user-1", objstore.ObjectAttributes{LastModified: time.Now().Add(-5 * time.Minute)}}, - {"user-2", objstore.ObjectAttributes{LastModified: time.Now().Add(-20 * time.Minute)}}, - {"user-3", objstore.ObjectAttributes{LastModified: time.Now().Add(-15 * time.Minute)}}, - {"user-4", objstore.ObjectAttributes{LastModified: time.Now().Add(-10 * time.Minute)}}, - } - - bucketClient := &bucket.ClientMock{} - bucketClient.MockIter("", []string{"user-1", "user-2", "user-3", "user-4"}, nil) - for i := 0; i < len(users); i++ { - bucketClient.MockExists(path.Join(users[i].ID, TenantDeletionMarkPath), false, nil) - bucketClient.MockAttributes(path.Join(users[i].ID, "bucket-index.json.gz"), users[i].indexAttrs, nil) - } - - isOwned := func(_ string) (bool, error) { - return true, nil - } - - s := NewUsersScanner(bucketClient, isOwned, log.NewNopLogger()) - actual, deleted, err := s.ScanUsers(context.Background()) - require.NoError(t, err) - assert.Equal(t, expected, actual) - assert.Empty(t, deleted) -} - -func TestUsersScanner_ScanUsers_ShouldReturnTenantWithoutBucketIndexLast(t *testing.T) { - bucketClient := &bucket.ClientMock{} - bucketClient.MockIter("", []string{"user-1", "user-2"}, nil) - bucketClient.MockExists(path.Join("user-1", TenantDeletionMarkPath), false, nil) - bucketClient.MockExists(path.Join("user-2", TenantDeletionMarkPath), false, nil) - - bucketClient.MockAttributes( - path.Join("user-1", "bucket-index.json.gz"), - objstore.ObjectAttributes{}, errors.New("fail"), - ) - - bucketClient.MockAttributes( - path.Join("user-2", "bucket-index.json.gz"), - objstore.ObjectAttributes{LastModified: time.Now().Add(-20 * time.Minute)}, nil, - ) - - isOwned := func(_ string) (bool, error) { - return true, nil - } - - s := NewUsersScanner(bucketClient, isOwned, log.NewNopLogger()) - actual, deleted, err := s.ScanUsers(context.Background()) - require.NoError(t, err) - assert.Equal(t, []string{"user-2", "user-1"}, actual) - assert.Empty(t, deleted) -}