From e215fa599b4aa5007367f2f428211623c922ef82 Mon Sep 17 00:00:00 2001
From: Ben Ye <benye@amazon.com>
Date: Mon, 22 Jan 2024 10:25:32 -0800
Subject: [PATCH] Fix lazy postings with zero length (#7083)

* fix lazy postings with zero length

Signed-off-by: Ben Ye <benye@amazon.com>

* changelog

Signed-off-by: Ben Ye <benye@amazon.com>

* unit tests

Signed-off-by: Ben Ye <benye@amazon.com>

* fix doc

Signed-off-by: Ben Ye <benye@amazon.com>

---------

Signed-off-by: Ben Ye <benye@amazon.com>
---
 CHANGELOG.md               |  2 ++
 pkg/store/bucket.go        | 17 ++++++++++-------
 pkg/store/bucket_test.go   | 35 +++++++++++++++++++++++++++++++++++
 pkg/store/lazy_postings.go |  3 +++
 4 files changed, 50 insertions(+), 7 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 76137ab4de..264a6351ea 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,8 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re
 
 ### Fixed
 
+- [#7083](https://github.com/thanos-io/thanos/pull/7083) Store Gateway: Fix lazy expanded postings with 0 length failed to be cached.
+
 ### Added
 
 ### Changed
diff --git a/pkg/store/bucket.go b/pkg/store/bucket.go
index fd4fb7392c..05c4995c7a 100644
--- a/pkg/store/bucket.go
+++ b/pkg/store/bucket.go
@@ -1163,13 +1163,16 @@ func (b *blockSeriesClient) nextBatch(tenant string) error {
 	if len(postingsBatch) == 0 {
 		b.hasMorePostings = false
 		if b.lazyPostings.lazyExpanded() {
-			v, err := b.indexr.IndexVersion()
-			if err != nil {
-				return errors.Wrap(err, "get index version")
-			}
-			if v >= 2 {
-				for i := range b.expandedPostings {
-					b.expandedPostings[i] = b.expandedPostings[i] / 16
+			// No need to fetch index version again if lazy posting has 0 length.
+			if len(b.lazyPostings.postings) > 0 {
+				v, err := b.indexr.IndexVersion()
+				if err != nil {
+					return errors.Wrap(err, "get index version")
+				}
+				if v >= 2 {
+					for i := range b.expandedPostings {
+						b.expandedPostings[i] = b.expandedPostings[i] / 16
+					}
 				}
 			}
 			b.indexr.storeExpandedPostingsToCache(b.blockMatchers, index.NewListPostings(b.expandedPostings), len(b.expandedPostings), tenant)
diff --git a/pkg/store/bucket_test.go b/pkg/store/bucket_test.go
index 67223a9467..b8306fa110 100644
--- a/pkg/store/bucket_test.go
+++ b/pkg/store/bucket_test.go
@@ -1282,6 +1282,41 @@ func TestExpandedPostingsEmptyPostings(t *testing.T) {
 	testutil.Equals(t, 1, indexr.stats.cachedPostingsCompressions)
 }
 
+func TestLazyExpandedPostingsEmptyPostings(t *testing.T) {
+	tmpDir := t.TempDir()
+
+	bkt, err := filesystem.NewBucket(filepath.Join(tmpDir, "bkt"))
+	testutil.Ok(t, err)
+	defer func() { testutil.Ok(t, bkt.Close()) }()
+
+	id := uploadTestBlock(t, tmpDir, bkt, 100)
+
+	r, err := indexheader.NewBinaryReader(context.Background(), log.NewNopLogger(), bkt, tmpDir, id, DefaultPostingOffsetInMemorySampling, indexheader.NewBinaryReaderMetrics(nil))
+	testutil.Ok(t, err)
+	b := &bucketBlock{
+		logger:                 log.NewNopLogger(),
+		metrics:                newBucketStoreMetrics(nil),
+		indexHeaderReader:      r,
+		indexCache:             noopCache{},
+		bkt:                    bkt,
+		meta:                   &metadata.Meta{BlockMeta: tsdb.BlockMeta{ULID: id}},
+		partitioner:            NewGapBasedPartitioner(PartitionerMaxGapSize),
+		estimatedMaxSeriesSize: 20,
+	}
+
+	indexr := newBucketIndexReader(b)
+	// matcher1 and matcher2 will match nothing after intersection.
+	matcher1 := labels.MustNewMatcher(labels.MatchEqual, "j", "foo")
+	matcher2 := labels.MustNewMatcher(labels.MatchRegexp, "n", "1_.*")
+	matcher3 := labels.MustNewMatcher(labels.MatchRegexp, "i", ".+")
+	ctx := context.Background()
+	dummyCounter := promauto.With(prometheus.NewRegistry()).NewCounter(prometheus.CounterOpts{Name: "test"})
+	ps, err := indexr.ExpandedPostings(ctx, newSortedMatchers([]*labels.Matcher{matcher1, matcher2, matcher3}), NewBytesLimiterFactory(0)(nil), true, dummyCounter, tenancy.DefaultTenant)
+	testutil.Ok(t, err)
+	// We expect emptyLazyPostings rather than lazy postings with 0 length but with matchers.
+	testutil.Equals(t, ps, emptyLazyPostings)
+}
+
 func TestBucketSeries(t *testing.T) {
 	tb := testutil.NewTB(t)
 	storetestutil.RunSeriesInterestingCases(tb, 200e3, 200e3, func(t testutil.TB, samplesPerSeries, series int) {
diff --git a/pkg/store/lazy_postings.go b/pkg/store/lazy_postings.go
index 146e7b0eec..4608d75a3c 100644
--- a/pkg/store/lazy_postings.go
+++ b/pkg/store/lazy_postings.go
@@ -184,6 +184,9 @@ func fetchLazyExpandedPostings(
 	if err != nil {
 		return nil, err
 	}
+	if len(ps) == 0 {
+		return emptyLazyPostings, nil
+	}
 	return &lazyExpandedPostings{postings: ps, matchers: matchers}, nil
 }