From 665955da482e81eac56b07aef9d345d8c7597e6d Mon Sep 17 00:00:00 2001
From: Fabian Reinartz <fab.reinartz@gmail.com>
Date: Wed, 11 Oct 2017 09:33:35 +0200
Subject: [PATCH] Clarify postings index semantics, handle staleness

The postings list index may point to series that no longer
exist during garbage collection. This clarifies that this is valid
behavior.
It would be possible, though more complex, to always keep them in sync.
However, series existance means nothing in itself as the queried time
range defines whether there's actual data. Thus our definition is sane
overall as long as drift is kept small.
---
 head.go         |  9 +--------
 index.go        |  3 +++
 querier.go      |  4 ++++
 querier_test.go | 10 ++++------
 4 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/head.go b/head.go
index 8f5beb4fb6..1d0f688395 100644
--- a/head.go
+++ b/head.go
@@ -833,24 +833,17 @@ func (h *headIndexReader) SortedPostings(p Postings) Postings {
 	if err := p.Err(); err != nil {
 		return errPostings{err: errors.Wrap(err, "expand postings")}
 	}
-	var err error
 
 	sort.Slice(ep, func(i, j int) bool {
-		if err != nil {
-			return false
-		}
 		a := h.head.series.getByID(ep[i])
 		b := h.head.series.getByID(ep[j])
 
 		if a == nil || b == nil {
-			err = errors.Errorf("series not found")
+			level.Debug(h.head.logger).Log("msg", "looked up series not found")
 			return false
 		}
 		return labels.Compare(a.lset, b.lset) < 0
 	})
-	if err != nil {
-		return errPostings{err: err}
-	}
 	return newListPostings(ep)
 }
 
diff --git a/index.go b/index.go
index 89f7764331..c0680aa338 100644
--- a/index.go
+++ b/index.go
@@ -525,6 +525,8 @@ type IndexReader interface {
 
 	// Postings returns the postings list iterator for the label pair.
 	// The Postings here contain the offsets to the series inside the index.
+	// Found IDs are not strictly required to point to a valid Series, e.g. during
+	// background garbage collections.
 	Postings(name, value string) (Postings, error)
 
 	// SortedPostings returns a postings list that is reordered to be sorted
@@ -533,6 +535,7 @@ type IndexReader interface {
 
 	// Series populates the given labels and chunk metas for the series identified
 	// by the reference.
+	// Returns ErrNotFound if the ref does not resolve to a known series.
 	Series(ref uint64, lset *labels.Labels, chks *[]ChunkMeta) error
 
 	// LabelIndices returns the label pairs for which indices exist.
diff --git a/querier.go b/querier.go
index 808a1057aa..12e05e79df 100644
--- a/querier.go
+++ b/querier.go
@@ -450,6 +450,10 @@ Outer:
 	for s.p.Next() {
 		ref := s.p.At()
 		if err := s.index.Series(ref, &lset, &chunks); err != nil {
+			// Postings may be stale. Skip if no underlying series exists.
+			if errors.Cause(err) == ErrNotFound {
+				continue
+			}
 			s.err = err
 			return false
 		}
diff --git a/querier_test.go b/querier_test.go
index e46f3b9475..b6f95bf7ce 100644
--- a/querier_test.go
+++ b/querier_test.go
@@ -702,9 +702,8 @@ func TestBaseChunkSeries(t *testing.T) {
 					ref: 108,
 				},
 			},
-			postings: []uint64{12, 10, 108},
-
-			expIdxs: []int{0, 1, 3},
+			postings: []uint64{12, 13, 10, 108}, // 13 doesn't exist and should just be skipped over.
+			expIdxs:  []int{0, 1, 3},
 		},
 		{
 			series: []refdSeries{
@@ -718,12 +717,11 @@ func TestBaseChunkSeries(t *testing.T) {
 				{
 					lset:   labels.New([]labels.Label{{"b", "c"}}...),
 					chunks: []ChunkMeta{{Ref: 8282}},
-					ref:    1,
+					ref:    3,
 				},
 			},
 			postings: []uint64{},
-
-			expIdxs: []int{},
+			expIdxs:  []int{},
 		},
 	}