Skip to content

Commit

Permalink
WriteRequest: change function args to LabelAdapter (#5302)
Browse files Browse the repository at this point in the history 
This makes things consistent: all these functions' arguments now come
from package `mimirpb`.

Primary motivation for this change was to remove spurious overhead under
`-tags stringlabels` from tests like `BenchmarkDistributor_Push`, where
`ToWriteRequest` was converting `Labels` to `[]LabelAdapter` on every
iteration.

The one place that used `ToWriteRequest` outside of tests is ruler, so
it now has to make the `Labels` to `[]LabelAdapter` conversion explicitly.
  • Loading branch information
@bboreham
bboreham authored Jun 21, 2023
1 parent fd6542a commit 4d4e346
Show file tree
Hide file tree
Showing 5 changed files with 155 additions and 161 deletions.
83 changes: 39 additions & 44 deletions pkg/distributor/distributor_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1520,17 +1520,17 @@ func TestDistributor_ExemplarValidation(t *testing.T) {
}
}

func mkLabels(n int, extra ...string) labels.Labels {
builder := labels.NewScratchBuilder(n + len(extra)/2)
builder.Add(model.MetricNameLabel, "foo")
func mkLabels(n int, extra ...string) []mimirpb.LabelAdapter {
ret := make([]mimirpb.LabelAdapter, 1+n+len(extra)/2)
ret[0] = mimirpb.LabelAdapter{Name: model.MetricNameLabel, Value: "foo"}
for i := 0; i < n; i++ {
builder.Add(fmt.Sprintf("name_%d", i), fmt.Sprintf("value_%d", i))
ret[i+1] = mimirpb.LabelAdapter{Name: fmt.Sprintf("name_%d", i), Value: fmt.Sprintf("value_%d", i)}
}
for i := 0; i < len(extra); i += 2 {
builder.Add(extra[i], extra[i+1])
ret[i+n+1] = mimirpb.LabelAdapter{Name: extra[i], Value: extra[i+1]}
}
builder.Sort()
return builder.Labels()
slices.SortFunc(ret, func(a, b mimirpb.LabelAdapter) bool { return a.Name < b.Name })
return ret
}

func BenchmarkDistributor_Push(b *testing.B) {
Expand All @@ -1541,13 +1541,13 @@ func BenchmarkDistributor_Push(b *testing.B) {

tests := map[string]struct {
prepareConfig func(limits *validation.Limits)
prepareSeries func() ([]labels.Labels, []mimirpb.Sample)
prepareSeries func() ([][]mimirpb.LabelAdapter, []mimirpb.Sample)
expectedErr string
}{
"all samples successfully pushed": {
prepareConfig: func(limits *validation.Limits) {},
prepareSeries: func() ([]labels.Labels, []mimirpb.Sample) {
metrics := make([]labels.Labels, numSeriesPerRequest)
prepareSeries: func() ([][]mimirpb.LabelAdapter, []mimirpb.Sample) {
metrics := make([][]mimirpb.LabelAdapter, numSeriesPerRequest)
samples := make([]mimirpb.Sample, numSeriesPerRequest)

for i := 0; i < numSeriesPerRequest; i++ {
Expand All @@ -1567,8 +1567,8 @@ func BenchmarkDistributor_Push(b *testing.B) {
limits.IngestionRate = 1
limits.IngestionBurstSize = 1
},
prepareSeries: func() ([]labels.Labels, []mimirpb.Sample) {
metrics := make([]labels.Labels, numSeriesPerRequest)
prepareSeries: func() ([][]mimirpb.LabelAdapter, []mimirpb.Sample) {
metrics := make([][]mimirpb.LabelAdapter, numSeriesPerRequest)
samples := make([]mimirpb.Sample, numSeriesPerRequest)

for i := 0; i < numSeriesPerRequest; i++ {
Expand All @@ -1587,8 +1587,8 @@ func BenchmarkDistributor_Push(b *testing.B) {
prepareConfig: func(limits *validation.Limits) {
limits.MaxLabelNamesPerSeries = 30
},
prepareSeries: func() ([]labels.Labels, []mimirpb.Sample) {
metrics := make([]labels.Labels, numSeriesPerRequest)
prepareSeries: func() ([][]mimirpb.LabelAdapter, []mimirpb.Sample) {
metrics := make([][]mimirpb.LabelAdapter, numSeriesPerRequest)
samples := make([]mimirpb.Sample, numSeriesPerRequest)

for i := 0; i < numSeriesPerRequest; i++ {
Expand All @@ -1607,8 +1607,8 @@ func BenchmarkDistributor_Push(b *testing.B) {
prepareConfig: func(limits *validation.Limits) {
limits.MaxLabelNameLength = 1024
},
prepareSeries: func() ([]labels.Labels, []mimirpb.Sample) {
metrics := make([]labels.Labels, numSeriesPerRequest)
prepareSeries: func() ([][]mimirpb.LabelAdapter, []mimirpb.Sample) {
metrics := make([][]mimirpb.LabelAdapter, numSeriesPerRequest)
samples := make([]mimirpb.Sample, numSeriesPerRequest)

for i := 0; i < numSeriesPerRequest; i++ {
Expand All @@ -1628,8 +1628,8 @@ func BenchmarkDistributor_Push(b *testing.B) {
prepareConfig: func(limits *validation.Limits) {
limits.MaxLabelValueLength = 1024
},
prepareSeries: func() ([]labels.Labels, []mimirpb.Sample) {
metrics := make([]labels.Labels, numSeriesPerRequest)
prepareSeries: func() ([][]mimirpb.LabelAdapter, []mimirpb.Sample) {
metrics := make([][]mimirpb.LabelAdapter, numSeriesPerRequest)
samples := make([]mimirpb.Sample, numSeriesPerRequest)

for i := 0; i < numSeriesPerRequest; i++ {
Expand All @@ -1649,8 +1649,8 @@ func BenchmarkDistributor_Push(b *testing.B) {
prepareConfig: func(limits *validation.Limits) {
limits.CreationGracePeriod = model.Duration(time.Minute)
},
prepareSeries: func() ([]labels.Labels, []mimirpb.Sample) {
metrics := make([]labels.Labels, numSeriesPerRequest)
prepareSeries: func() ([][]mimirpb.LabelAdapter, []mimirpb.Sample) {
metrics := make([][]mimirpb.LabelAdapter, numSeriesPerRequest)
samples := make([]mimirpb.Sample, numSeriesPerRequest)

for i := 0; i < numSeriesPerRequest; i++ {
Expand All @@ -1677,17 +1677,12 @@ func BenchmarkDistributor_Push(b *testing.B) {
},
}
},
prepareSeries: func() ([]labels.Labels, []mimirpb.Sample) {
metrics := make([]labels.Labels, numSeriesPerRequest)
prepareSeries: func() ([][]mimirpb.LabelAdapter, []mimirpb.Sample) {
metrics := make([][]mimirpb.LabelAdapter, numSeriesPerRequest)
samples := make([]mimirpb.Sample, numSeriesPerRequest)

for i := 0; i < numSeriesPerRequest; i++ {
lbls := labels.NewBuilder(labels.FromStrings(model.MetricNameLabel, "foo"))
for i := 0; i < 10; i++ {
lbls.Set(fmt.Sprintf("name_%d", i), fmt.Sprintf("value_%d", i))
}

metrics[i] = lbls.Labels()
metrics[i] = mkLabels(10)
samples[i] = mimirpb.Sample{
Value: float64(i),
TimestampMs: time.Now().UnixNano() / int64(time.Millisecond),
Expand Down Expand Up @@ -2898,7 +2893,7 @@ func mockWriteRequest(lbls labels.Labels, value float64, timestampMs int64) *mim
},
}

return mimirpb.ToWriteRequest([]labels.Labels{lbls}, samples, nil, nil, mimirpb.API)
return mimirpb.ToWriteRequest([][]mimirpb.LabelAdapter{mimirpb.FromLabelsToLabelAdapters(lbls)}, samples, nil, nil, mimirpb.API)
}

type prepConfig struct {
Expand Down Expand Up @@ -3916,7 +3911,7 @@ func TestDistributorValidation(t *testing.T) {

for i, tc := range []struct {
metadata []*mimirpb.MetricMetadata
labels []labels.Labels
labels [][]mimirpb.LabelAdapter
samples []mimirpb.Sample
exemplars []*mimirpb.Exemplar
expectedStatusCode int32
Expand All @@ -3925,7 +3920,7 @@ func TestDistributorValidation(t *testing.T) {
// Test validation passes.
{
metadata: []*mimirpb.MetricMetadata{{MetricFamilyName: "testmetric", Help: "a test metric.", Unit: "", Type: mimirpb.COUNTER}},
labels: []labels.Labels{labels.FromStrings(labels.MetricName, "testmetric", "foo", "bar")},
labels: [][]mimirpb.LabelAdapter{{{Name: labels.MetricName, Value: "testmetric"}, {Name: "foo", Value: "bar"}}},
samples: []mimirpb.Sample{{
TimestampMs: int64(now),
Value: 1,
Expand All @@ -3939,11 +3934,11 @@ func TestDistributorValidation(t *testing.T) {

// Test validation passes when labels are unsorted.
{
labels: []labels.Labels{mimirpb.FromLabelAdaptersToLabels(
[]mimirpb.LabelAdapter{
labels: [][]mimirpb.LabelAdapter{
{
{Name: "foo", Value: "bar"},
{Name: labels.MetricName, Value: "testmetric"},
})},
}},
samples: []mimirpb.Sample{{
TimestampMs: int64(now),
Value: 1,
Expand All @@ -3952,7 +3947,7 @@ func TestDistributorValidation(t *testing.T) {

// Test validation fails for samples from the future.
{
labels: []labels.Labels{labels.FromStrings(labels.MetricName, "testmetric", "foo", "bar")},
labels: [][]mimirpb.LabelAdapter{{{Name: labels.MetricName, Value: "testmetric"}, {Name: "foo", Value: "bar"}}},
samples: []mimirpb.Sample{{
TimestampMs: int64(future),
Value: 4,
Expand All @@ -3963,7 +3958,7 @@ func TestDistributorValidation(t *testing.T) {

// Test maximum labels names per series.
{
labels: []labels.Labels{labels.FromStrings(labels.MetricName, "testmetric", "foo", "bar", "foo2", "bar2")},
labels: [][]mimirpb.LabelAdapter{{{Name: labels.MetricName, Value: "testmetric"}, {Name: "foo", Value: "bar"}, {Name: "foo2", Value: "bar2"}}},
samples: []mimirpb.Sample{{
TimestampMs: int64(now),
Value: 2,
Expand All @@ -3973,9 +3968,9 @@ func TestDistributorValidation(t *testing.T) {
},
// Test multiple validation fails return the first one.
{
labels: []labels.Labels{
labels.FromStrings(labels.MetricName, "testmetric", "foo", "bar", "foo2", "bar2"),
labels.FromStrings(labels.MetricName, "testmetric", "foo", "bar"),
labels: [][]mimirpb.LabelAdapter{
{{Name: labels.MetricName, Value: "testmetric"}, {Name: "foo", Value: "bar"}, {Name: "foo2", Value: "bar2"}},
{{Name: labels.MetricName, Value: "testmetric"}, {Name: "foo", Value: "bar"}},
},
samples: []mimirpb.Sample{
{TimestampMs: int64(now), Value: 2},
Expand All @@ -3987,7 +3982,7 @@ func TestDistributorValidation(t *testing.T) {
// Test metadata validation fails
{
metadata: []*mimirpb.MetricMetadata{{MetricFamilyName: "", Help: "a test metric.", Unit: "", Type: mimirpb.COUNTER}},
labels: []labels.Labels{labels.FromStrings(labels.MetricName, "testmetric", "foo", "bar")},
labels: [][]mimirpb.LabelAdapter{{{Name: labels.MetricName, Value: "testmetric"}, {Name: "foo", Value: "bar"}}},
samples: []mimirpb.Sample{{
TimestampMs: int64(now),
Value: 1,
Expand All @@ -3998,7 +3993,7 @@ func TestDistributorValidation(t *testing.T) {
// Test empty exemplar labels fails.
{
metadata: []*mimirpb.MetricMetadata{{MetricFamilyName: "testmetric", Help: "a test metric.", Unit: "", Type: mimirpb.COUNTER}},
labels: []labels.Labels{labels.FromStrings(labels.MetricName, "testmetric", "foo", "bar")},
labels: [][]mimirpb.LabelAdapter{{{Name: labels.MetricName, Value: "testmetric"}, {Name: "foo", Value: "bar"}}},
samples: []mimirpb.Sample{{
TimestampMs: int64(now),
Value: 1,
Expand Down Expand Up @@ -4430,11 +4425,11 @@ func TestDistributor_MetricsWithRequestModifications(t *testing.T) {

req := mimirpb.NewWriteRequest(nil, mimirpb.API)
for sampleIdx := 0; sampleIdx < 20; sampleIdx++ {
lbs := mimirpb.FromLabelAdaptersToLabels(uniqueMetricsGen(sampleIdx))
lbs := uniqueMetricsGen(sampleIdx)
if sampleIdx%2 == 0 {
req.AddHistogramSeries([]labels.Labels{lbs}, []mimirpb.Histogram{validHistogram}, nil)
req.AddHistogramSeries([][]mimirpb.LabelAdapter{lbs}, []mimirpb.Histogram{validHistogram}, nil)
} else {
req.AddHistogramSeries([]labels.Labels{lbs}, []mimirpb.Histogram{invalidHistogram}, nil)
req.AddHistogramSeries([][]mimirpb.LabelAdapter{lbs}, []mimirpb.Histogram{invalidHistogram}, nil)
}
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/ingester/client/buffering_client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,10 +206,10 @@ func TestWriteRequestBufferingClient_PushConcurrent(t *testing.T) {
}

func createRequest(metricName string, seriesPerRequest int) *mimirpb.WriteRequest {
metrics := make([]labels.Labels, 0, seriesPerRequest)
metrics := make([][]mimirpb.LabelAdapter, 0, seriesPerRequest)
samples := make([]mimirpb.Sample, 0, seriesPerRequest)
for i := 0; i < seriesPerRequest; i++ {
metrics = append(metrics, labels.FromStrings(labels.MetricName, metricName, "cardinality", strconv.Itoa(i)))
metrics = append(metrics, []mimirpb.LabelAdapter{{Name: labels.MetricName, Value: metricName}, {Name: "cardinality", Value: strconv.Itoa(i)}})
samples = append(samples, mimirpb.Sample{Value: float64(i), TimestampMs: time.Now().UnixMilli()})
}

Expand Down
Loading

0 comments on commit 4d4e346

Please sign in to comment.