Skip to content

Commit

Permalink
undo changes on usersScanner
Browse files Browse the repository at this point in the history
  • Loading branch information
dmwilson-grafana committed Jan 24, 2025
1 parent 53a55d9 commit 61687ea
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 95 deletions.
23 changes: 0 additions & 23 deletions pkg/storage/tsdb/users_scanner.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,7 @@ package tsdb

import (
"context"
"path"
"sort"
"strings"
"time"

"github.com/go-kit/log"
"github.com/go-kit/log/level"
Expand Down Expand Up @@ -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]

Expand All @@ -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
}

Expand Down
72 changes: 0 additions & 72 deletions pkg/storage/tsdb/users_scanner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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)
Expand All @@ -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")
}
Expand All @@ -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)
}

0 comments on commit 61687ea

Please sign in to comment.