Implementing Bucket index sync status

Author: alanprot

pkg/compactor/blocks_cleaner.go

@@ -206,6 +206,11 @@ func (c *BlocksCleaner) deleteUserMarkedForDeletion(ctx context.Context, userID
 	if err := bucketindex.DeleteIndex(ctx, c.bucketClient, userID, c.cfgProvider); err != nil {
 		return err
 	}
+
+	// Delete the bucket sync status
+	if err := bucketindex.DeleteIndexSyncStatus(ctx, c.bucketClient, userID); err != nil {
+		return err
+	}
 	c.tenantBucketIndexLastUpdate.DeleteLabelValues(userID)
 
 	var deletedBlocks, failed int
@@ -327,9 +332,11 @@ func (c *BlocksCleaner) cleanUser(ctx context.Context, userID string, firstRun b
 		level.Warn(userLogger).Log("msg", "found a corrupted bucket index, recreating it")
 	} else if errors.Is(err, bucket.ErrCustomerManagedKeyAccessDenied) {
 		// Give up cleaning if we get access denied
-		level.Warn(userLogger).Log("msg", err.Error())
+		level.Warn(userLogger).Log("msg", "customer manager key access denied", "err", err)
+		bucketindex.WriteSyncStatus(ctx, c.bucketClient, userID, bucketindex.CustomerManagedKeyError, userLogger)
 		return nil
 	} else if err != nil && !errors.Is(err, bucketindex.ErrIndexNotFound) {
+		bucketindex.WriteSyncStatus(ctx, c.bucketClient, userID, bucketindex.GenericError, userLogger)
 		return err
 	}
 
@@ -348,6 +355,7 @@ func (c *BlocksCleaner) cleanUser(ctx context.Context, userID string, firstRun b
 	w := bucketindex.NewUpdater(c.bucketClient, userID, c.cfgProvider, c.logger)
 	idx, partials, totalBlocksBlocksMarkedForNoCompaction, err := w.UpdateIndex(ctx, idx)
 	if err != nil {
+		bucketindex.WriteSyncStatus(ctx, c.bucketClient, userID, bucketindex.GenericError, userLogger)
 		return err
 	}
 
@@ -398,7 +406,7 @@ func (c *BlocksCleaner) cleanUser(ctx context.Context, userID string, firstRun b
 	c.tenantBlocksMarkedForNoCompaction.WithLabelValues(userID).Set(float64(totalBlocksBlocksMarkedForNoCompaction))
 	c.tenantBucketIndexLastUpdate.WithLabelValues(userID).SetToCurrentTime()
 	c.tenantPartialBlocks.WithLabelValues(userID).Set(float64(len(partials)))
-
+	bucketindex.WriteSyncStatus(ctx, c.bucketClient, userID, bucketindex.Ok, userLogger)
 	return nil
 }
 

pkg/compactor/blocks_cleaner_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"crypto/rand"
 	"fmt"
+	"os"
 	"path"
 	"strings"
 	"testing"
@@ -57,7 +58,7 @@ func TestBlocksCleaner(t *testing.T) {
 func TestBlockCleaner_KeyPermissionDenied(t *testing.T) {
 	const userID = "user-1"
 
-	bucketClient, _ := cortex_testutil.PrepareFilesystemBucket(t)
+	bucketClient, dir := cortex_testutil.PrepareFilesystemBucket(t)
 	bucketClient = bucketindex.BucketWithGlobalMarkers(bucketClient)
 
 	// Create blocks.
@@ -83,6 +84,12 @@ func TestBlockCleaner_KeyPermissionDenied(t *testing.T) {
 	cleaner := NewBlocksCleaner(cfg, bucketClient, scanner, cfgProvider, logger, nil)
 	err := cleaner.cleanUser(ctx, userID, true)
 	require.NoError(t, err)
+	stat, err := os.Stat(path.Join(dir, userID, bucketindex.SyncStatusFile))
+	require.NoError(t, err)
+	require.True(t, stat.Size() > 0)
+	s, err := bucketindex.ReadSyncStatus(ctx, bucketClient, userID, logger)
+	require.NoError(t, err)
+	require.Equal(t, bucketindex.CustomerManagedKeyError, s)
 }
 
 func testBlocksCleanerWithOptions(t *testing.T, options testBlocksCleanerOptions) {
@@ -232,6 +239,9 @@ func testBlocksCleanerWithOptions(t *testing.T, options testBlocksCleanerOptions
 		require.NoError(t, err)
 		assert.ElementsMatch(t, tc.expectedBlocks, idx.Blocks.GetULIDs())
 		assert.ElementsMatch(t, tc.expectedMarks, idx.BlockDeletionMarks.GetULIDs())
+		s, err := bucketindex.ReadSyncStatus(ctx, bucketClient, tc.userID, logger)
+		require.NoError(t, err)
+		require.Equal(t, bucketindex.Ok, s)
 	}
 
 	assert.NoError(t, prom_testutil.GatherAndCompare(reg, strings.NewReader(`
@@ -385,6 +395,9 @@ func TestBlocksCleaner_ShouldRebuildBucketIndexOnCorruptedOne(t *testing.T) {
 	require.NoError(t, err)
 	assert.ElementsMatch(t, []ulid.ULID{block1, block3}, idx.Blocks.GetULIDs())
 	assert.ElementsMatch(t, []ulid.ULID{block3}, idx.BlockDeletionMarks.GetULIDs())
+	s, err := bucketindex.ReadSyncStatus(ctx, bucketClient, userID, logger)
+	require.NoError(t, err)
+	require.Equal(t, bucketindex.Ok, s)
 }
 
 func TestBlocksCleaner_ShouldRemoveMetricsForTenantsNotBelongingAnymoreToTheShard(t *testing.T) {

pkg/compactor/compactor_test.go

@@ -473,6 +473,7 @@ func TestCompactor_ShouldIncrementCompactionErrorIfFailedToCompactASingleTenant(
 	bucketClient.MockUpload(userID+"/01FN6CDF3PNEWWRY5MPGJPE3EX/visit-mark.json", nil)
 	bucketClient.MockGet(userID+"/bucket-index.json.gz", "", nil)
 	bucketClient.MockUpload(userID+"/bucket-index.json.gz", nil)
+	bucketClient.MockUpload(userID+"/bucket-index-sync-status.json", nil)
 
 	c, _, tsdbPlannerMock, _, registry := prepare(t, prepareConfig(), bucketClient, nil)
 	tsdbPlannerMock.On("Plan", mock.Anything, mock.Anything).Return([]*metadata.Meta{}, errors.New("Failed to plan"))
@@ -538,6 +539,8 @@ func TestCompactor_ShouldIterateOverUsersAndRunCompaction(t *testing.T) {
 	bucketClient.MockIter("user-2/markers/", nil, nil)
 	bucketClient.MockUpload("user-1/bucket-index.json.gz", nil)
 	bucketClient.MockUpload("user-2/bucket-index.json.gz", nil)
+	bucketClient.MockUpload("user-1/bucket-index-sync-status.json", nil)
+	bucketClient.MockUpload("user-2/bucket-index-sync-status.json", nil)
 
 	c, _, tsdbPlanner, logs, registry := prepare(t, prepareConfig(), bucketClient, nil)
 
@@ -674,6 +677,7 @@ func TestCompactor_ShouldNotCompactBlocksMarkedForDeletion(t *testing.T) {
 	bucketClient.MockDelete("user-1/01DTW0ZCPDDNV4BV83Q2SV4QAZ", nil)
 	bucketClient.MockGet("user-1/bucket-index.json.gz", "", nil)
 	bucketClient.MockUpload("user-1/bucket-index.json.gz", nil)
+	bucketClient.MockUpload("user-1/bucket-index-sync-status.json", nil)
 
 	c, _, tsdbPlanner, logs, registry := prepare(t, cfg, bucketClient, nil)
 
@@ -799,6 +803,8 @@ func TestCompactor_ShouldNotCompactBlocksMarkedForSkipCompact(t *testing.T) {
 	bucketClient.MockIter("user-2/markers/", nil, nil)
 	bucketClient.MockUpload("user-1/bucket-index.json.gz", nil)
 	bucketClient.MockUpload("user-2/bucket-index.json.gz", nil)
+	bucketClient.MockUpload("user-1/bucket-index-sync-status.json", nil)
+	bucketClient.MockUpload("user-2/bucket-index-sync-status.json", nil)
 
 	c, _, tsdbPlanner, _, registry := prepare(t, prepareConfig(), bucketClient, nil)
 
@@ -850,6 +856,7 @@ func TestCompactor_ShouldNotCompactBlocksForUsersMarkedForDeletion(t *testing.T)
 	bucketClient.MockDelete("user-1/01DTVP434PA9VFXSW2JKB3392D/meta.json", nil)
 	bucketClient.MockDelete("user-1/01DTVP434PA9VFXSW2JKB3392D/index", nil)
 	bucketClient.MockDelete("user-1/bucket-index.json.gz", nil)
+	bucketClient.MockDelete("user-1/bucket-index-sync-status.json", nil)
 
 	c, _, tsdbPlanner, logs, registry := prepare(t, cfg, bucketClient, nil)
 
@@ -1026,6 +1033,8 @@ func TestCompactor_ShouldCompactAllUsersOnShardingEnabledButOnlyOneInstanceRunni
 	bucketClient.MockGet("user-2/bucket-index.json.gz", "", nil)
 	bucketClient.MockUpload("user-1/bucket-index.json.gz", nil)
 	bucketClient.MockUpload("user-2/bucket-index.json.gz", nil)
+	bucketClient.MockUpload("user-1/bucket-index-sync-status.json", nil)
+	bucketClient.MockUpload("user-2/bucket-index-sync-status.json", nil)
 
 	ringStore, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger(), nil)
 	t.Cleanup(func() { assert.NoError(t, closer.Close()) })
@@ -1107,6 +1116,7 @@ func TestCompactor_ShouldCompactOnlyUsersOwnedByTheInstanceOnShardingEnabledAndM
 		bucketClient.MockUpload(userID+"/01DTVP434PA9VFXSW2JKB3392D/visit-mark.json", nil)
 		bucketClient.MockGet(userID+"/bucket-index.json.gz", "", nil)
 		bucketClient.MockUpload(userID+"/bucket-index.json.gz", nil)
+		bucketClient.MockUpload(userID+"/bucket-index-sync-status.json", nil)
 	}
 
 	// Create a shared KV Store
@@ -1230,6 +1240,7 @@ func TestCompactor_ShouldCompactOnlyShardsOwnedByTheInstanceOnShardingEnabledWit
 		bucketClient.MockExists(path.Join(userID, cortex_tsdb.TenantDeletionMarkPath), false, nil)
 		bucketClient.MockGet(userID+"/bucket-index.json.gz", "", nil)
 		bucketClient.MockUpload(userID+"/bucket-index.json.gz", nil)
+		bucketClient.MockUpload(userID+"/bucket-index-sync-status.json", nil)
 	}
 
 	// Create a shared KV Store

pkg/querier/blocks_finder_bucket_index.go

@@ -58,7 +58,7 @@ func (f *BucketIndexBlocksFinder) GetBlocks(ctx context.Context, userID string,
 	}
 
 	// Get the bucket index for this user.
-	idx, err := f.loader.GetIndex(ctx, userID)
+	idx, ss, err := f.loader.GetIndex(ctx, userID)
 	if errors.Is(err, bucketindex.ErrIndexNotFound) {
 		// This is a legit edge case, happening when a new tenant has not shipped blocks to the storage yet
 		// so the bucket index hasn't been created yet.
@@ -69,6 +69,10 @@ func (f *BucketIndexBlocksFinder) GetBlocks(ctx context.Context, userID string,
 		return nil, nil, validation.AccessDeniedError(err.Error())
 	}
 
+	if ss == bucketindex.CustomerManagedKeyError {
+		return nil, nil, validation.AccessDeniedError(bucket.ErrCustomerManagedKeyAccessDenied.Error())
+	}
+
 	if err != nil {
 		return nil, nil, err
 	}

pkg/querier/blocks_finder_bucket_index_test.go

@@ -13,6 +13,8 @@ import (
 	"github.com/stretchr/testify/require"
 	"github.com/thanos-io/objstore"
 
+	"github.com/cortexproject/cortex/pkg/storage/bucket"
+
 	"github.com/cortexproject/cortex/pkg/util/validation"
 
 	"github.com/cortexproject/cortex/pkg/storage/tsdb/bucketindex"
@@ -221,6 +223,28 @@ func TestBucketIndexBlocksFinder_GetBlocks_BucketIndexIsTooOld(t *testing.T) {
 	require.Equal(t, errBucketIndexTooOld, err)
 }
 
+func TestBucketIndexBlocksFinder_GetBlocks_BucketIndexIsTooOldWithCustomerKeyError(t *testing.T) {
+	t.Parallel()
+
+	const userID = "user-1"
+
+	ctx := context.Background()
+	bkt, _ := cortex_testutil.PrepareFilesystemBucket(t)
+	finder := prepareBucketIndexBlocksFinder(t, bkt)
+
+	require.NoError(t, bucketindex.WriteIndex(ctx, bkt, userID, nil, &bucketindex.Index{
+		Version:            bucketindex.IndexVersion1,
+		Blocks:             bucketindex.Blocks{},
+		BlockDeletionMarks: bucketindex.BlockDeletionMarks{},
+		UpdatedAt:          time.Now().Add(-2 * time.Hour).Unix(),
+	}))
+
+	bucketindex.WriteSyncStatus(ctx, bkt, userID, bucketindex.CustomerManagedKeyError, log.NewNopLogger())
+
+	_, _, err := finder.GetBlocks(ctx, userID, 10, 20)
+	require.Equal(t, validation.AccessDeniedError(bucket.ErrCustomerManagedKeyAccessDenied.Error()), err)
+}
+
 func prepareBucketIndexBlocksFinder(t testing.TB, bkt objstore.Bucket) *BucketIndexBlocksFinder {
 	ctx := context.Background()
 	cfg := BucketIndexBlocksFinderConfig{

pkg/storage/tsdb/bucketindex/loader.go

@@ -91,7 +91,7 @@ func NewLoader(cfg LoaderConfig, bucketClient objstore.Bucket, cfgProvider bucke
 
 // GetIndex returns the bucket index for the given user. It returns the in-memory cached
 // index if available, or load it from the bucket otherwise.
-func (l *Loader) GetIndex(ctx context.Context, userID string) (*Index, error) {
+func (l *Loader) GetIndex(ctx context.Context, userID string) (*Index, SyncStatus, error) {
 	l.indexesMx.RLock()
 	if entry := l.indexes[userID]; entry != nil {
 		idx := entry.index
@@ -101,7 +101,7 @@ func (l *Loader) GetIndex(ctx context.Context, userID string) (*Index, error) {
 		// We don't check if the index is stale because it's the responsibility
 		// of the background job to keep it updated.
 		entry.requestedAt.Store(time.Now().Unix())
-		return idx, err
+		return idx, entry.syncStatus, err
 	}
 	l.indexesMx.RUnlock()
 
@@ -111,7 +111,7 @@ func (l *Loader) GetIndex(ctx context.Context, userID string) (*Index, error) {
 	if err != nil {
 		// Cache the error, to avoid hammering the object store in case of persistent issues
 		// (eg. corrupted bucket index or not existing).
-		l.cacheIndex(userID, nil, err)
+		l.cacheIndex(userID, nil, Unknown, err)
 
 		if errors.Is(err, ErrIndexNotFound) {
 			level.Warn(l.logger).Log("msg", "bucket index not found", "user", userID)
@@ -124,25 +124,31 @@ func (l *Loader) GetIndex(ctx context.Context, userID string) (*Index, error) {
 			level.Error(l.logger).Log("msg", "unable to load bucket index", "user", userID, "err", err)
 		}
 
-		return nil, err
+		return nil, Unknown, err
+	}
+
+	ss, err := ReadSyncStatus(ctx, l.bkt, userID, l.logger)
+
+	if err != nil {
+		level.Warn(l.logger).Log("msg", "unable to read bucket index status", "user", userID, "err", err)
 	}
 
 	// Cache the index.
-	l.cacheIndex(userID, idx, nil)
+	l.cacheIndex(userID, idx, ss, nil)
 
 	elapsedTime := time.Since(startTime)
 	l.loadDuration.Observe(elapsedTime.Seconds())
 	level.Info(l.logger).Log("msg", "loaded bucket index", "user", userID, "duration", elapsedTime)
-	return idx, nil
+	return idx, ss, nil
 }
 
-func (l *Loader) cacheIndex(userID string, idx *Index, err error) {
+func (l *Loader) cacheIndex(userID string, idx *Index, ss SyncStatus, err error) {
 	l.indexesMx.Lock()
 	defer l.indexesMx.Unlock()
 
 	// Not an issue if, due to concurrency, another index was already cached
 	// and we overwrite it: last will win.
-	l.indexes[userID] = newCachedIndex(idx, err)
+	l.indexes[userID] = newCachedIndex(idx, ss, err)
 }
 
 // checkCachedIndexes checks all cached indexes and, for each of them, does two things:
@@ -240,8 +246,9 @@ func (l *Loader) countLoadedIndexesMetric() float64 {
 type cachedIndex struct {
 	// We cache either the index or the error occurred while fetching it. They're
 	// mutually exclusive.
-	index *Index
-	err   error
+	index      *Index
+	syncStatus SyncStatus
+	err        error
 
 	// Unix timestamp (seconds) of when the index has been updated from the storage the last time.
 	updatedAt atomic.Int64
@@ -250,10 +257,11 @@ type cachedIndex struct {
 	requestedAt atomic.Int64
 }
 
-func newCachedIndex(idx *Index, err error) *cachedIndex {
+func newCachedIndex(idx *Index, ss SyncStatus, err error) *cachedIndex {
 	entry := &cachedIndex{
-		index: idx,
-		err:   err,
+		index:      idx,
+		err:        err,
+		syncStatus: ss,
 	}
 
 	now := time.Now()