compactor: verify that TSDB uploads use index format v2 (#13815)

Verify that the TSDB block index file uses v2 for the file format, not
v1. The v1 format was only used for Prometheus 2.0 and 2.1 as far as I
can tell. It hasn't been the default format for new blocks since
Prometheus 2.2 and has never been the default for Mimir.

Part of #13808

Author: 56quarters

CHANGELOG.md

@@ -27,6 +27,7 @@
 * [CHANGE] Limits: removed the experimental `cost_attribution_labels` configuration option. Use `cost_attribution_labels_structured` instead. #13286
 * [CHANGE] Ingester: Renamed `cortex_ingest_storage_writer_buffered_produce_bytes` metric to `cortex_ingest_storage_writer_buffered_produce_bytes_distribution` (Prometheus summary), and added `cortex_ingest_storage_writer_buffered_produce_bytes` metric that exports the buffer size as a Prometheus Gauge. #13414
 * [CHANGE] Querier and query-frontend: Removed support for per-step stats when MQE is enabled. #13582
+* [CHANGE] Compactor: Require that uploaded TSDB blocks use v2 of the index file format. #13815
 * [CHANGE] Query-frontend: Removed support for calculating 'cache-adjusted samples processed' query statistic. The `-query-frontend.cache-samples-processed-stats` CLI flag has been deprecated and will be removed in a future release. Setting it has now no effect. #13582
 * [CHANGE] Querier: Renamed experimental flag `-querier.prefer-availability-zone` to `-querier.prefer-availability-zones` and changed it to accept a comma-separated list of availability zones. All zones in the list are given equal priority when querying ingesters and store-gateways. #13756 #13758
 * [CHANGE] Ingester: Stabilize experimental flag `-ingest-storage.write-logs-fsync-before-kafka-commit-concurrency` to fsync write logs before the offset is committed to Kafka. Remove `-ingest-storage.write-logs-fsync-before-kafka-commit-enabled` since this is always enabled now. #13591

pkg/storage/tsdb/block/index.go

@@ -33,6 +33,10 @@ import (
 	util_log "github.com/grafana/mimir/pkg/util/log"
 )
 
+var (
+	SupportedIndexFormats = []int{index.FormatV2}
+)
+
 // VerifyBlock does a full run over a block index and chunk data and verifies that they fulfill the order invariants.
 func VerifyBlock(ctx context.Context, logger log.Logger, blockDir string, minTime, maxTime int64, checkChunks bool) error {
 	stats, err := GatherBlockHealthStats(ctx, logger, blockDir, minTime, maxTime, checkChunks)
@@ -44,6 +48,9 @@ func VerifyBlock(ctx context.Context, logger log.Logger, blockDir string, minTim
 }
 
 type HealthStats struct {
+	// IndexFormat is the format version used by the TSDB index file.
+	IndexFormat int
+
 	// TotalSeries represents total number of series in block.
 	TotalSeries int64
 	// OutOfOrderSeries represents number of series that have out of order chunks.
@@ -68,6 +75,17 @@ type HealthStats struct {
 	OutOfOrderLabels int
 }
 
+// UnsupportedIndexFormat returns an error if the stats indicate this TSDB block uses
+// an index format that isn't supported by the read path (store-gateways use a custom
+// implementation of TSDB index parsing code, to avoid mmap, which only supports v2).
+func (i HealthStats) UnsupportedIndexFormat() error {
+	if !slices.Contains(SupportedIndexFormats, i.IndexFormat) {
+		return fmt.Errorf("index uses format %d which is not supported (%v are supported)", i.IndexFormat, SupportedIndexFormats)
+	}
+
+	return nil
+}
+
 // OutOfOrderLabelsErr returns an error if the HealthStats object indicates
 // postings without of order labels.  This is corrected by Prometheus Issue
 // #5372 and affects Prometheus versions 2.8.0 and below.
@@ -129,6 +147,10 @@ func (i HealthStats) AnyErr() error {
 		errMsg = append(errMsg, err.Error())
 	}
 
+	if err := i.UnsupportedIndexFormat(); err != nil {
+		errMsg = append(errMsg, err.Error())
+	}
+
 	if err := i.Issue347OutsideChunksErr(); err != nil {
 		errMsg = append(errMsg, err.Error())
 	}
@@ -162,6 +184,8 @@ func GatherBlockHealthStats(ctx context.Context, logger log.Logger, blockDir str
 	}
 	defer runutil.CloseWithErrCapture(&err, r, "gather index issue file reader")
 
+	stats.IndexFormat = r.Version()
+
 	n, v := index.AllPostingsKey()
 	p, err := r.Postings(ctx, n, v)
 	if err != nil {

pkg/storage/tsdb/block/index_test.go

@@ -20,6 +20,77 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+func TestHealthStats_AnyErr(t *testing.T) {
+	testCases := []struct {
+		name          string
+		stats         HealthStats
+		expectSuccess bool
+	}{
+		{
+			name: "critical error",
+			stats: HealthStats{
+				IndexFormat:           index.FormatV2,
+				OutsideChunks:         1,
+				CompleteOutsideChunks: 1,
+			},
+			expectSuccess: false,
+		},
+		{
+			name: "invalid index version",
+			stats: HealthStats{
+				IndexFormat: index.FormatV1,
+			},
+			expectSuccess: false,
+		},
+		{
+			name: "issue 347 outside chunks",
+			stats: HealthStats{
+				IndexFormat:           index.FormatV2,
+				OutsideChunks:         1,
+				Issue347OutsideChunks: 1,
+			},
+			expectSuccess: false,
+		},
+		{
+			name: "out of order labels",
+			stats: HealthStats{
+				IndexFormat:      index.FormatV2,
+				OutOfOrderLabels: 1,
+			},
+			expectSuccess: false,
+		},
+
+		{
+			name: "out of order chunks",
+			stats: HealthStats{
+				IndexFormat:      index.FormatV2,
+				TotalSeries:      100,
+				OutOfOrderSeries: 50,
+				OutOfOrderChunks: 10,
+			},
+			expectSuccess: false,
+		},
+		{
+			name: "success",
+			stats: HealthStats{
+				IndexFormat: index.FormatV2,
+			},
+			expectSuccess: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := tc.stats.AnyErr()
+			if tc.expectSuccess {
+				require.NoError(t, err)
+			} else {
+				require.Error(t, err)
+			}
+		})
+	}
+}
+
 func TestRewrite(t *testing.T) {
 	const excludeTime int64 = 600