Comments and reverted change to qfe

Signed-off-by: Sebastian Rabenhorst <[email protected]>
thanos-io · Jan 19, 2023 · 2456ee2 · 2456ee2
1 parent 16422da
commit 2456ee2
Show file tree

Hide file tree

Showing 4 changed files with 10 additions and 15 deletions.
diff --git a/internal/cortex/querier/series/series_set.go b/internal/cortex/querier/series/series_set.go
@@ -239,7 +239,7 @@ func (d DeletedSeriesIterator) Seek(t int64) chunkenc.ValueType {
 		return valueType
 	}
 
-	seekedTs := d.itr.AtT()
+	seekedTs, _ := d.itr.At()
 	if d.isDeleted(seekedTs) {
 		// point we have seeked into is deleted, Next() should find a new non-deleted sample which is after t and seekedTs
 		return d.Next()
@@ -268,7 +268,7 @@ func (d DeletedSeriesIterator) AtT() int64 {
 
 func (d DeletedSeriesIterator) Next() chunkenc.ValueType {
 	for valueType := d.itr.Next(); valueType != chunkenc.ValNone; valueType = d.itr.Next() {
-		ts := d.itr.AtT()
+		ts, _ := d.itr.At()
 
 		if d.isDeleted(ts) {
 			continue

diff --git a/internal/cortex/querier/series/series_set_test.go b/internal/cortex/querier/series/series_set_test.go
@@ -145,7 +145,7 @@ func TestDeletedIterator_WithSeek(t *testing.T) {
 
 		require.Equal(t, c.valueType, it.Seek(c.seek))
 		if c.valueType != chunkenc.ValNone {
-			ts := it.AtT()
+			ts, _ := it.At()
 			require.Equal(t, c.seekedTs, ts)
 		}
 	}

diff --git a/pkg/query/querier_test.go b/pkg/query/querier_test.go
@@ -843,6 +843,7 @@ func (s *mockedSeriesIterator) At() (t int64, v float64) {
 	return sample.t, sample.v
 }
 
+// TODO(rabenhorst): Needs to be implemented for native histogram support.
 func (s *mockedSeriesIterator) AtHistogram() (int64, *histogram.Histogram) {
 	panic("not implemented")
 }

diff --git a/test/e2e/native_histograms_test.go b/test/e2e/native_histograms_test.go
@@ -40,23 +40,16 @@ func TestQueryNativeHistograms(t *testing.T) {
 	testutil.Ok(t, synthesizeHistogram(ctx, rawRemoteWriteURL1))
 	testutil.Ok(t, synthesizeHistogram(ctx, rawRemoteWriteURL2))
 
-	queryAndAssertSeries(t, ctx, prom1.Endpoint("http"), func() string { return "fake_histogram" }, time.Now, promclient.QueryOptions{}, []model.Metric{
-		{
-			"__name__": "fake_histogram",
-			"foo":      "bar",
-		},
-	})
-	queryAndAssertSeries(t, ctx, prom2.Endpoint("http"), func() string { return "fake_histogram" }, time.Now, promclient.QueryOptions{}, []model.Metric{
-		{
-			"__name__": "fake_histogram",
-			"foo":      "bar",
-		},
-	})
+	// Make sure we can query histogram from both Prometheus instances.
+	queryAndAssert(t, ctx, prom1.Endpoint("http"), func() string { return "fake_histogram" }, time.Now, promclient.QueryOptions{}, expectedHistogramModelVector(nil))
+	queryAndAssert(t, ctx, prom2.Endpoint("http"), func() string { return "fake_histogram" }, time.Now, promclient.QueryOptions{}, expectedHistogramModelVector(nil))
 
+	// Query deduplicated histogram from Thanos Querier.
 	queryAndAssert(t, ctx, querier.Endpoint("http"), func() string { return "fake_histogram" }, time.Now, promclient.QueryOptions{Deduplicate: true}, expectedHistogramModelVector(map[string]string{
 		"prometheus": "prom-ha",
 	}))
 
+	// Query histogram using histogram_count function and deduplication from Thanos Querier.
 	queryAndAssert(t, ctx, querier.Endpoint("http"), func() string { return "histogram_count(fake_histogram)" }, time.Now, promclient.QueryOptions{Deduplicate: true}, model.Vector{
 		&model.Sample{
 			Value: 5,
@@ -67,6 +60,7 @@ func TestQueryNativeHistograms(t *testing.T) {
 		},
 	})
 
+	// Query histogram using group function to test pushdown.
 	queryAndAssert(t, ctx, querier.Endpoint("http"), func() string { return "group(fake_histogram)" }, time.Now, promclient.QueryOptions{Deduplicate: true}, model.Vector{
 		&model.Sample{
 			Value:  1,