receive: Improved Performance for err path. (#4085)

``` benchstat -delta-test=none ../_dev/thanos/2021/receive2/impr5-go1.16.3.txt ../_dev/thanos/2021/receive2/errimpr1-go1.16.3.txt name old time/op new time/op delta HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/OK-12 1.22ms ± 0% 1.05ms ± 0% -13.97% HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/conflict_errors-12 5.84ms ± 0% 1.26ms ± 0% -78.46% HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/OK-12 12.8ms ± 0% 11.3ms ± 0% -11.54% HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/conflict_errors-12 59.4ms ± 0% 12.8ms ± 0% -78.39% HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/OK-12 109ms ± 0% 97ms ± 0% -11.16% HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/conflict_errors-12 1.44s ± 0% 0.10s ± 0% -93.25% name old alloc/op new alloc/op delta HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/OK-12 1.20MB ± 0% 1.20MB ± 0% -0.69% HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/conflict_errors-12 4.34MB ± 0% 1.36MB ± 0% -68.54% HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/OK-12 13.3MB ± 0% 13.2MB ± 0% -0.67% HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/conflict_errors-12 44.6MB ± 0% 14.9MB ± 0% -66.67% HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/OK-12 120MB ± 0% 120MB ± 0% -0.00% HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/conflict_errors-12 592MB ± 0% 120MB ± 0% -79.77% name old allocs/op new allocs/op delta HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/OK-12 5.10k ± 0% 4.59k ± 0% -9.83% HandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/conflict_errors-12 25.1k ± 0% 7.6k ± 0% -69.67% HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/OK-12 50.4k ± 0% 45.4k ± 0% -9.93% HandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/conflict_errors-12 250k ± 0% 75k ± 0% -69.96% HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/OK-12 89.0 ± 0% 78.0 ± 0% -12.36% HandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/conflict_errors-12 368 ± 0% 201 ± 0% -45.38% ``` Signed-off-by: Bartlomiej Plotka <[email protected]>
thanos-io · Apr 21, 2021 · ad8dc2a · ad8dc2a
1 parent 9ae2a15
commit ad8dc2a
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 12 deletions.
diff --git a/pkg/receive/handler_test.go b/pkg/receive/handler_test.go
@@ -1293,7 +1293,7 @@ func Heap(dir string) (err error) {
 		return err
 	}
 
-	f, err := os.Create(filepath.Join(dir, "impr5-go1.16.3.pprof"))
+	f, err := os.Create(filepath.Join(dir, "errimpr1-go1.16.3.pprof"))
 	if err != nil {
 		return err
 	}

diff --git a/pkg/receive/hashring.go b/pkg/receive/hashring.go
@@ -67,7 +67,6 @@ func (s simpleHashring) GetN(tenant string, ts *prompb.TimeSeries, n uint64) (st
 		return "", &insufficientNodesError{have: uint64(len(s)), want: n + 1}
 	}
 
-	// TODO(bwplotka): This might be not needed, double check.
 	sort.Slice(ts.Labels, func(i, j int) bool { return ts.Labels[i].Name < ts.Labels[j].Name })
 
 	return s[(labelpb.HashWithPrefix(tenant, ts.Labels)+n)%uint64(len(s))], nil

diff --git a/pkg/receive/writer.go b/pkg/receive/writer.go
@@ -81,33 +81,32 @@ func (r *Writer) Write(ctx context.Context, tenantID string, wreq *prompb.WriteR
 			switch err {
 			case storage.ErrOutOfOrderSample:
 				numOutOfOrder++
-				level.Debug(r.logger).Log("msg", "Out of order sample", "lset", lset.String(), "sample", s.String())
+				level.Debug(r.logger).Log("msg", "Out of order sample", "lset", lset, "sample", s)
 			case storage.ErrDuplicateSampleForTimestamp:
 				numDuplicates++
-				level.Debug(r.logger).Log("msg", "Duplicate sample for timestamp", "lset", lset.String(), "sample", s.String())
+				level.Debug(r.logger).Log("msg", "Duplicate sample for timestamp", "lset", lset, "sample", s)
 			case storage.ErrOutOfBounds:
 				numOutOfBounds++
-				level.Debug(r.logger).Log("msg", "Out of bounds metric", "lset", lset.String(), "sample", s.String())
+				level.Debug(r.logger).Log("msg", "Out of bounds metric", "lset", lset, "sample", s)
 			}
 		}
 	}
 
 	if numOutOfOrder > 0 {
-		level.Warn(r.logger).Log("msg", "Error on ingesting out-of-order samples", "num_dropped", numOutOfOrder)
-		errs.Add(errors.Wrapf(storage.ErrOutOfOrderSample, "failed to non-fast add %d samples", numOutOfOrder))
+		level.Warn(r.logger).Log("msg", "Error on ingesting out-of-order samples", "numDropped", numOutOfOrder)
+		errs.Add(errors.Wrapf(storage.ErrOutOfOrderSample, "add %d samples", numOutOfOrder))
 	}
 	if numDuplicates > 0 {
-		level.Warn(r.logger).Log("msg", "Error on ingesting samples with different value but same timestamp", "num_dropped", numDuplicates)
-		errs.Add(errors.Wrapf(storage.ErrDuplicateSampleForTimestamp, "failed to non-fast add %d samples", numDuplicates))
+		level.Warn(r.logger).Log("msg", "Error on ingesting samples with different value but same timestamp", "numDropped", numDuplicates)
+		errs.Add(errors.Wrapf(storage.ErrDuplicateSampleForTimestamp, "add %d samples", numDuplicates))
 	}
 	if numOutOfBounds > 0 {
-		level.Warn(r.logger).Log("msg", "Error on ingesting samples that are too old or are too far into the future", "num_dropped", numOutOfBounds)
-		errs.Add(errors.Wrapf(storage.ErrOutOfBounds, "failed to non-fast add %d samples", numOutOfBounds))
+		level.Warn(r.logger).Log("msg", "Error on ingesting samples that are too old or are too far into the future", "numDropped", numOutOfBounds)
+		errs.Add(errors.Wrapf(storage.ErrOutOfBounds, "add %d samples", numOutOfBounds))
 	}
 
 	if err := app.Commit(); err != nil {
 		errs.Add(errors.Wrap(err, "commit samples"))
 	}
-
 	return errs.Err()
 }