Fix flaky chunk tests by being explicit about now (#656)

* Fix flaky chunk tests by being explicit about `now`

`dummyChunkFor` was consulting the system clock itself. This meant that tests
like `TestChunkStore_Get` would intermittently fail, because _sometimes_ the
created chunks would have timestamps outside the window chosen within
`TestChunkStore_Get` (which itself consults the system clock).

This fixes the tests by making `now` a parameter of `dummyChunkFor`, which
makes the created chunks deterministic.

It also adds a `now` parameter to `dummyChunk`, which isn't strictly
necessary, but I believe is less error prone.

* Review feedback

- Be even more deterministic by sharing `now` values
- Be even more deterministic by making benchmark chunk factory accept a `now` value

Author: jml

pkg/chunk/aws_storage_client_test.go

@@ -531,7 +531,7 @@ func testStorageClientChunks(t *testing.T, client StorageClient) {
 	for i := 0; i < 50; i++ {
 		chunks := []Chunk{}
 		for j := 0; j < batchSize; j++ {
-			chunk := dummyChunkFor(model.Metric{
+			chunk := dummyChunkFor(model.Now(), model.Metric{
 				model.MetricNameLabel: "foo",
 				"index":               model.LabelValue(strconv.Itoa(i*batchSize + j)),
 			})

pkg/chunk/by_key_test.go

@@ -102,7 +102,8 @@ func TestNWayIntersect(t *testing.T) {
 }
 
 func BenchmarkByKeyLess(b *testing.B) {
-	a := ByKey{dummyChunk(), dummyChunk()}
+	now := model.Now()
+	a := ByKey{dummyChunk(now), dummyChunk(now)}
 
 	b.ResetTimer()
 
@@ -118,10 +119,7 @@ func BenchmarkByKeySort10000(b *testing.B) { benchmarkByKeySort(b, 10000) }
 func benchmarkByKeySort(b *testing.B, batchSize int) {
 	chunks := []Chunk{}
 	for i := 0; i < batchSize; i++ {
-		chunk := dummyChunk()
-		// Tweak the dummy data slightly so the chunks are more likely to be different
-		// this makes the checksum wrong but we don't look at it
-		chunk.From += model.Time(rand.Intn(batchSize))
+		chunk := dummyChunk(model.Now() + model.Time(rand.Intn(batchSize)))
 		chunks = append(chunks, chunk)
 	}
 

pkg/chunk/chunk_store_test.go

@@ -86,11 +86,11 @@ func TestChunkStore_Get(t *testing.T) {
 		"toms": "code",
 	}
 
-	fooChunk1 := dummyChunkFor(fooMetric1)
-	fooChunk2 := dummyChunkFor(fooMetric2)
+	fooChunk1 := dummyChunkFor(now, fooMetric1)
+	fooChunk2 := dummyChunkFor(now, fooMetric2)
 
-	barChunk1 := dummyChunkFor(barMetric1)
-	barChunk2 := dummyChunkFor(barMetric2)
+	barChunk1 := dummyChunkFor(now, barMetric1)
+	barChunk2 := dummyChunkFor(now, barMetric2)
 
 	fooSampleStream1, err := createSampleStreamFrom(fooChunk1)
 	require.NoError(t, err)
@@ -222,6 +222,8 @@ func TestChunkStore_Get(t *testing.T) {
 
 				sort.Sort(ByFingerprint(matrix1))
 				if !reflect.DeepEqual(tc.expect, matrix1) {
+					t.Fatalf("jml\nstart = %#v\nnow = %#v\nfooChunk1 = %#v\nfooChunk2 = %#v\nbarChunk1 = %#v\nbarChunk2 = %#v\n",
+						now.Add(-time.Hour), now, fooChunk1, fooChunk2, barChunk1, barChunk2)
 					t.Fatalf("%s: wrong chunks - %s", tc.query, test.Diff(tc.expect, matrix1))
 				}
 
@@ -251,13 +253,13 @@ func TestChunkStore_getMetricNameChunks(t *testing.T) {
 	ctx := user.InjectOrgID(context.Background(), userID)
 	now := model.Now()
 	metricName := "foo"
-	chunk1 := dummyChunkFor(model.Metric{
+	chunk1 := dummyChunkFor(now, model.Metric{
 		model.MetricNameLabel: "foo",
 		"bar":  "baz",
 		"toms": "code",
 		"flip": "flop",
 	})
-	chunk2 := dummyChunkFor(model.Metric{
+	chunk2 := dummyChunkFor(now, model.Metric{
 		model.MetricNameLabel: "foo",
 		"bar":  "beep",
 		"toms": "code",

pkg/chunk/chunk_test.go

@@ -16,16 +16,15 @@ import (
 
 const userID = "userID"
 
-func dummyChunk() Chunk {
-	return dummyChunkFor(model.Metric{
+func dummyChunk(now model.Time) Chunk {
+	return dummyChunkFor(now, model.Metric{
 		model.MetricNameLabel: "foo",
 		"bar":  "baz",
 		"toms": "code",
 	})
 }
 
-func dummyChunkFor(metric model.Metric) Chunk {
-	now := model.Now()
+func dummyChunkFor(now model.Time, metric model.Metric) Chunk {
 	cs, _ := chunk.New().Add(model.SamplePair{Timestamp: now, Value: 0})
 	chunk := NewChunk(
 		userID,
@@ -44,39 +43,40 @@ func dummyChunkFor(metric model.Metric) Chunk {
 }
 
 func TestChunkCodec(t *testing.T) {
+	dummy := dummyChunk(model.Now())
 	decodeContext := NewDecodeContext()
 	for i, c := range []struct {
 		chunk Chunk
 		err   error
 		f     func(*Chunk, []byte)
 	}{
 		// Basic round trip
-		{chunk: dummyChunk()},
+		{chunk: dummy},
 
 		// Checksum should fail
 		{
-			chunk: dummyChunk(),
+			chunk: dummy,
 			err:   ErrInvalidChecksum,
 			f:     func(_ *Chunk, buf []byte) { buf[4]++ },
 		},
 
 		// Checksum should fail
 		{
-			chunk: dummyChunk(),
+			chunk: dummy,
 			err:   ErrInvalidChecksum,
 			f:     func(c *Chunk, _ []byte) { c.Checksum = 123 },
 		},
 
 		// Metadata test should fail
 		{
-			chunk: dummyChunk(),
+			chunk: dummy,
 			err:   ErrWrongMetadata,
 			f:     func(c *Chunk, _ []byte) { c.Fingerprint++ },
 		},
 
 		// Metadata test should fail
 		{
-			chunk: dummyChunk(),
+			chunk: dummy,
 			err:   ErrWrongMetadata,
 			f:     func(c *Chunk, _ []byte) { c.UserID = "foo" },
 		},
@@ -139,10 +139,11 @@ func TestChunksToMatrix(t *testing.T) {
 		"bar":  "baz",
 		"toms": "code",
 	}
-	chunk1 := dummyChunkFor(metric)
+	now := model.Now()
+	chunk1 := dummyChunkFor(now, metric)
 	chunk1Samples, err := chunk1.Samples(chunk1.From, chunk1.Through)
 	require.NoError(t, err)
-	chunk2 := dummyChunkFor(metric)
+	chunk2 := dummyChunkFor(now, metric)
 	chunk2Samples, err := chunk2.Samples(chunk2.From, chunk2.Through)
 	require.NoError(t, err)
 
@@ -157,7 +158,7 @@ func TestChunksToMatrix(t *testing.T) {
 		"bar":  "baz",
 		"toms": "code",
 	}
-	chunk3 := dummyChunkFor(otherMetric)
+	chunk3 := dummyChunkFor(now, otherMetric)
 	chunk3Samples, err := chunk3.Samples(chunk3.From, chunk3.Through)
 	require.NoError(t, err)
 
@@ -194,9 +195,9 @@ func TestChunksToMatrix(t *testing.T) {
 	}
 }
 
-func benchmarkChunk() Chunk {
+func benchmarkChunk(now model.Time) Chunk {
 	// This is a real example from Kubernetes' embedded cAdvisor metrics, lightly obfuscated
-	return dummyChunkFor(model.Metric{
+	return dummyChunkFor(now, model.Metric{
 		model.MetricNameLabel:              "container_cpu_usage_seconds_total",
 		"beta_kubernetes_io_arch":          "amd64",
 		"beta_kubernetes_io_instance_type": "c3.somesize",
@@ -218,7 +219,7 @@ func benchmarkChunk() Chunk {
 }
 
 func BenchmarkEncode(b *testing.B) {
-	chunk := dummyChunk()
+	chunk := dummyChunk(model.Now())
 
 	b.ResetTimer()
 
@@ -232,7 +233,7 @@ func BenchmarkDecode100(b *testing.B)   { benchmarkDecode(b, 100) }
 func BenchmarkDecode10000(b *testing.B) { benchmarkDecode(b, 10000) }
 
 func benchmarkDecode(b *testing.B, batchSize int) {
-	chunk := benchmarkChunk()
+	chunk := benchmarkChunk(model.Now())
 	buf, err := chunk.Encode()
 	require.NoError(b, err)
 

pkg/ingester/ingester_lifecycle.go

@@ -155,6 +155,7 @@ loop:
 	for {
 		select {
 		case <-autoJoinAfter:
+			level.Debug(util.Logger).Log("msg", "JoinAfter expired")
 			// Will only fire once, after auto join timeout.  If we haven't entered "JOINING" state,
 			// then pick some tokens and enter ACTIVE state.
 			if i.state == ring.PENDING {

pkg/ingester/ingester_lifecycle_test.go

@@ -142,7 +142,7 @@ func TestIngesterTransfer(t *testing.T) {
 				Labels: util.ToLabelPairs(m),
 				Samples: []client.Sample{
 					{
-						Value:       456.,
+						Value:       456,
 						TimestampMs: 123000,
 					},
 				},