Uploading no compact markers to the global marker index

Author: alanprot

pkg/storage/tsdb/bucketindex/markers.go

@@ -27,6 +27,12 @@ func BlockDeletionMarkFilepath(blockID ulid.ULID) string {
 	return fmt.Sprintf("%s/%s-%s", MarkersPathname, blockID.String(), metadata.DeletionMarkFilename)
 }
 
+// NoCompactMarkFilenameMarkFilepath returns the path, relative to the tenant's bucket location,
+// of a block no compact mark in the bucket markers location.
+func NoCompactMarkFilenameMarkFilepath(blockID ulid.ULID) string {
+	return fmt.Sprintf("%s/%s-%s", MarkersPathname, blockID.String(), metadata.NoCompactMarkFilename)
+}
+
 // IsBlockDeletionMarkFilename returns whether the input filename matches the expected pattern
 // of block deletion markers stored in the markers location.
 func IsBlockDeletionMarkFilename(name string) (ulid.ULID, bool) {

pkg/storage/tsdb/bucketindex/markers_bucket_client.go

@@ -29,7 +29,7 @@ func BucketWithGlobalMarkers(b objstore.Bucket) objstore.Bucket {
 
 // Upload implements objstore.Bucket.
 func (b *globalMarkersBucket) Upload(ctx context.Context, name string, r io.Reader) error {
-	blockID, ok := b.isBlockDeletionMark(name)
+	globalMarkPath, ok := b.isMark(name)
 	if !ok {
 		return b.parent.Upload(ctx, name, r)
 	}
@@ -46,7 +46,6 @@ func (b *globalMarkersBucket) Upload(ctx context.Context, name string, r io.Read
 	}
 
 	// Upload it to the global markers location too.
-	globalMarkPath := path.Clean(path.Join(path.Dir(name), "../", BlockDeletionMarkFilepath(blockID)))
 	return b.parent.Upload(ctx, globalMarkPath, bytes.NewBuffer(body))
 }
 
@@ -58,8 +57,7 @@ func (b *globalMarkersBucket) Delete(ctx context.Context, name string) error {
 	}
 
 	// Delete the marker in the global markers location too.
-	if blockID, ok := b.isBlockDeletionMark(name); ok {
-		globalMarkPath := path.Clean(path.Join(path.Dir(name), "../", BlockDeletionMarkFilepath(blockID)))
+	if globalMarkPath, ok := b.isMark(name); ok {
 		if err := b.parent.Delete(ctx, globalMarkPath); err != nil {
 			if !b.parent.IsObjNotFoundErr(err) {
 				return err
@@ -128,6 +126,29 @@ func (b *globalMarkersBucket) ReaderWithExpectedErrs(fn objstore.IsOpFailureExpe
 	return b
 }
 
+func (b *globalMarkersBucket) isMark(name string) (string, bool) {
+	marks := map[string]func(ulid.ULID) string{
+		metadata.DeletionMarkFilename:  BlockDeletionMarkFilepath,
+		metadata.NoCompactMarkFilename: NoCompactMarkFilenameMarkFilepath,
+	}
+
+	for mark, f := range marks {
+		if path.Base(name) == mark {
+			// Parse the block ID in the path. If there's not block ID, then it's not the per-block
+			// deletion mark.
+			id, ok := block.IsBlockDir(path.Dir(name))
+
+			if ok {
+				return path.Clean(path.Join(path.Dir(name), "../", f(id))), ok
+			}
+
+			return "", ok
+		}
+	}
+
+	return "", false
+}
+
 func (b *globalMarkersBucket) isBlockDeletionMark(name string) (ulid.ULID, bool) {
 	if path.Base(name) != metadata.DeletionMarkFilename {
 		return ulid.ULID{}, false

pkg/storage/tsdb/bucketindex/markers_bucket_client_test.go

@@ -11,43 +11,114 @@ import (
 	"github.com/prometheus/client_golang/prometheus/testutil"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"github.com/thanos-io/thanos/pkg/block/metadata"
 	"github.com/thanos-io/thanos/pkg/objstore"
 
 	"github.com/cortexproject/cortex/pkg/storage/bucket"
 	cortex_testutil "github.com/cortexproject/cortex/pkg/storage/tsdb/testutil"
 )
 
-func TestGlobalMarkersBucket_Delete_ShouldSucceedIfDeletionMarkDoesNotExistInTheBlockButExistInTheGlobalLocation(t *testing.T) {
-	bkt, _ := cortex_testutil.PrepareFilesystemBucket(t)
+func TestGlobalMarker_ShouldUploadGlobalLocation(t *testing.T) {
+	block1 := ulid.MustNew(1, nil)
 
-	ctx := context.Background()
-	bkt = BucketWithGlobalMarkers(bkt)
+	tests := []struct {
+		mark       string
+		globalpath string
+	}{
+		{
+			mark:       metadata.DeletionMarkFilename,
+			globalpath: "markers/" + block1.String() + "-deletion-mark.json",
+		},
+		{
+			mark:       metadata.NoCompactMarkFilename,
+			globalpath: "markers/" + block1.String() + "-no-compact-mark.json",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.mark, func(t *testing.T) {
+			originalPath := block1.String() + "/" + tc.mark
+			bkt, _ := cortex_testutil.PrepareFilesystemBucket(t)
+
+			ctx := context.Background()
+			bkt = BucketWithGlobalMarkers(bkt)
+
+			bkt.Upload(ctx, originalPath, strings.NewReader("{}"))
 
-	// Create a mocked block deletion mark in the global location.
-	blockID := ulid.MustNew(1, nil)
-	globalPath := BlockDeletionMarkFilepath(blockID)
-	require.NoError(t, bkt.Upload(ctx, globalPath, strings.NewReader("{}")))
+			// Ensure it exists on originalPath
+			ok, err := bkt.Exists(ctx, originalPath)
+			require.NoError(t, err)
+			require.True(t, ok)
 
-	// Ensure it exists before deleting it.
-	ok, err := bkt.Exists(ctx, globalPath)
-	require.NoError(t, err)
-	require.True(t, ok)
+			// Ensure it exists on globalPath
+			ok, err = bkt.Exists(ctx, tc.globalpath)
+			require.NoError(t, err)
+			require.True(t, ok)
 
-	require.NoError(t, bkt.Delete(ctx, globalPath))
+			bkt.Delete(ctx, originalPath)
 
-	// Ensure has been actually deleted.
-	ok, err = bkt.Exists(ctx, globalPath)
-	require.NoError(t, err)
-	require.False(t, ok)
+			// Ensure it deleted on originalPath
+			ok, err = bkt.Exists(ctx, originalPath)
+			require.NoError(t, err)
+			require.False(t, ok)
+
+			// Ensure it exists on globalPath
+			ok, err = bkt.Exists(ctx, tc.globalpath)
+			require.NoError(t, err)
+			require.False(t, ok)
+		})
+	}
 }
 
-func TestGlobalMarkersBucket_isBlockDeletionMark(t *testing.T) {
+func TestGlobalMarkersBucket_Delete_ShouldSucceedIfMarkDoesNotExistInTheBlockButExistInTheGlobalLocation(t *testing.T) {
+	tests := []struct {
+		name  string
+		pathF func(ulid.ULID) string
+	}{
+		{
+			name:  metadata.DeletionMarkFilename,
+			pathF: BlockDeletionMarkFilepath,
+		},
+		{
+			name:  metadata.NoCompactMarkFilename,
+			pathF: NoCompactMarkFilenameMarkFilepath,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			bkt, _ := cortex_testutil.PrepareFilesystemBucket(t)
+
+			ctx := context.Background()
+			bkt = BucketWithGlobalMarkers(bkt)
+
+			// Create a mocked block deletion mark in the global location.
+			blockID := ulid.MustNew(1, nil)
+			globalPath := tc.pathF(blockID)
+			require.NoError(t, bkt.Upload(ctx, globalPath, strings.NewReader("{}")))
+
+			// Ensure it exists before deleting it.
+			ok, err := bkt.Exists(ctx, globalPath)
+			require.NoError(t, err)
+			require.True(t, ok)
+
+			require.NoError(t, bkt.Delete(ctx, globalPath))
+
+			// Ensure has been actually deleted.
+			ok, err = bkt.Exists(ctx, globalPath)
+			require.NoError(t, err)
+			require.False(t, ok)
+		})
+	}
+}
+
+func TestGlobalMarkersBucket_isMark(t *testing.T) {
 	block1 := ulid.MustNew(1, nil)
 
 	tests := []struct {
-		name       string
-		expectedOk bool
-		expectedID ulid.ULID
+		name               string
+		expectedOk         bool
+		expectedGlobalPath string
 	}{
 		{
 			name:       "",
@@ -59,23 +130,31 @@ func TestGlobalMarkersBucket_isBlockDeletionMark(t *testing.T) {
 			name:       block1.String() + "/index",
 			expectedOk: false,
 		}, {
-			name:       block1.String() + "/deletion-mark.json",
-			expectedOk: true,
-			expectedID: block1,
+			name:               block1.String() + "/deletion-mark.json",
+			expectedOk:         true,
+			expectedGlobalPath: "markers/" + block1.String() + "-deletion-mark.json",
+		}, {
+			name:               "/path/to/" + block1.String() + "/deletion-mark.json",
+			expectedOk:         true,
+			expectedGlobalPath: "/path/to/markers/" + block1.String() + "-deletion-mark.json",
+		}, {
+			name:               block1.String() + "/no-compact-mark.json",
+			expectedOk:         true,
+			expectedGlobalPath: "markers/" + block1.String() + "-no-compact-mark.json",
 		}, {
-			name:       "/path/to/" + block1.String() + "/deletion-mark.json",
-			expectedOk: true,
-			expectedID: block1,
+			name:               "/path/to/" + block1.String() + "/no-compact-mark.json",
+			expectedOk:         true,
+			expectedGlobalPath: "/path/to/markers/" + block1.String() + "-no-compact-mark.json",
 		},
 	}
 
 	b := BucketWithGlobalMarkers(nil).(*globalMarkersBucket)
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			actualID, actualOk := b.isBlockDeletionMark(tc.name)
+			globalPath, actualOk := b.isMark(tc.name)
 			assert.Equal(t, tc.expectedOk, actualOk)
-			assert.Equal(t, tc.expectedID, actualID)
+			assert.Equal(t, tc.expectedGlobalPath, globalPath)
 		})
 	}
 }