From 2fbe81ca220b3c6e25160b701d1fbbfabbce265a Mon Sep 17 00:00:00 2001 From: Bartlomiej Plotka Date: Fri, 19 Mar 2021 21:35:41 +0100 Subject: [PATCH] Copy labels. ``` GOROOT=/home/bwplotka/.gvm/gos/go1.15 #gosetup GOPATH=/home/bwplotka/Repos/thanosgopath #gosetup /home/bwplotka/.gvm/gos/go1.15/bin/go test -c -o /tmp/___BenchmarkHandlerReceiveHTTP_in_github_com_thanos_io_thanos_pkg_receive github.com/thanos-io/thanos/pkg/receive #gosetup /tmp/___BenchmarkHandlerReceiveHTTP_in_github_com_thanos_io_thanos_pkg_receive -test.v -test.bench ^\QBenchmarkHandlerReceiveHTTP\E$ -test.run ^$ -test.benchmem -test.benchtime=30s goos: linux goarch: amd64 pkg: github.com/thanos-io/thanos/pkg/receive BenchmarkHandlerReceiveHTTP BenchmarkHandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them BenchmarkHandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/OK BenchmarkHandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/OK-12 25887 1537262 ns/op 1380023 B/op 6092 allocs/op BenchmarkHandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/conflict_errors BenchmarkHandlerReceiveHTTP/typical_labels_under_1KB,_500_of_them/conflict_errors-12 4237 7547968 ns/op 4522583 B/op 26118 allocs/op BenchmarkHandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them BenchmarkHandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/OK BenchmarkHandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/OK-12 2205 16513380 ns/op 15071092 B/op 60420 allocs/op BenchmarkHandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/conflict_errors BenchmarkHandlerReceiveHTTP/typical_labels_under_1KB,_5000_of_them/conflict_errors-12 525 67278233 ns/op 46396645 B/op 260141 allocs/op BenchmarkHandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them BenchmarkHandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/OK BenchmarkHandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/OK-12 285 148049189 ns/op 226596168 B/op 132 allocs/op BenchmarkHandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/conflict_errors BenchmarkHandlerReceiveHTTP/extremely_large_label_value_10MB,_10_of_them/conflict_errors-12 20 1731361499 ns/op 698722550 B/op 401 allocs/op PASS Process finished with exit code 0 ``` Signed-off-by: Bartlomiej Plotka --- pkg/receive/handler.go | 4 ++++ pkg/receive/handler_test.go | 26 ++++++++++++++++++++++++-- pkg/receive/writer.go | 11 ++++------- pkg/store/labelpb/label.go | 13 +++++++++++++ 4 files changed, 45 insertions(+), 9 deletions(-) diff --git a/pkg/receive/handler.go b/pkg/receive/handler.go index ca20855778..693bd74336 100644 --- a/pkg/receive/handler.go +++ b/pkg/receive/handler.go @@ -277,6 +277,7 @@ func (h *Handler) receiveHTTP(w http.ResponseWriter, r *http.Request) { span, ctx := tracing.StartSpan(r.Context(), "receive_http") defer span.Finish() + // TODO(bwplotka): Optimize readAll https://github.com/thanos-io/thanos/pull/3334/files. compressed, err := ioutil.ReadAll(r.Body) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) @@ -290,6 +291,9 @@ func (h *Handler) receiveHTTP(w http.ResponseWriter, r *http.Request) { return } + // NOTE: Due to zero copy ZLabels, Labels used from WriteRequests keeps memory + // from the whole request. Ensure that we always copy those when we want to + // store them for longer time. var wreq prompb.WriteRequest if err := proto.Unmarshal(reqBuf, &wreq); err != nil { http.Error(w, err.Error(), http.StatusBadRequest) diff --git a/pkg/receive/handler_test.go b/pkg/receive/handler_test.go index f41814672f..5d64cd4c39 100644 --- a/pkg/receive/handler_test.go +++ b/pkg/receive/handler_test.go @@ -13,6 +13,9 @@ import ( "net/http" "net/http/httptest" "os" + "path/filepath" + "runtime" + "runtime/pprof" "strings" "sync" "testing" @@ -1066,7 +1069,7 @@ func benchmarkHandlerMultiTSDBReceiveRemoteWrite(b testutil.TB) { writeRequest []byte }{ { - name: "typical labels under 1KB, 500 of them.", + name: "typical labels under 1KB, 500 of them", writeRequest: serializeSeriesWithOneSample(b, func() [][]labelpb.ZLabel { series := make([][]labelpb.ZLabel, 500) for s := 0; s < len(series); s++ { @@ -1081,7 +1084,7 @@ func benchmarkHandlerMultiTSDBReceiveRemoteWrite(b testutil.TB) { }()), }, { - name: "typical labels under 1KB, 5000 of them.", + name: "typical labels under 1KB, 5000 of them", writeRequest: serializeSeriesWithOneSample(b, func() [][]labelpb.ZLabel { series := make([][]labelpb.ZLabel, 5000) for s := 0; s < len(series); s++ { @@ -1173,4 +1176,23 @@ func benchmarkHandlerMultiTSDBReceiveRemoteWrite(b testutil.TB) { }) }) } + + runtime.GC() + // Take snapshot. + // TODO(bwplotka): Remove it + testutil.Ok(b, Heap("../../")) + +} + +func Heap(dir string) (err error) { + if err := os.MkdirAll(dir, os.ModePerm); err != nil { + return err + } + + f, err := os.Create(filepath.Join(dir, "mem.pprof")) + if err != nil { + return err + } + defer runutil.CloseWithErrCapture(&err, f, "close") + return pprof.WriteHeapProfile(f) } diff --git a/pkg/receive/writer.go b/pkg/receive/writer.go index 8fe1cfe68c..6bfaaa893c 100644 --- a/pkg/receive/writer.go +++ b/pkg/receive/writer.go @@ -13,6 +13,7 @@ import ( "github.com/prometheus/prometheus/pkg/labels" "github.com/prometheus/prometheus/storage" "github.com/prometheus/prometheus/tsdb" + "github.com/thanos-io/thanos/pkg/store/labelpb" "github.com/thanos-io/thanos/pkg/errutil" "github.com/thanos-io/thanos/pkg/store/storepb/prompb" @@ -61,13 +62,9 @@ func (r *Writer) Write(ctx context.Context, tenantID string, wreq *prompb.WriteR var errs errutil.MultiError for _, t := range wreq.Timeseries { - lset := make(labels.Labels, len(t.Labels)) - for j := range t.Labels { - lset[j] = labels.Label{ - Name: t.Labels[j].Name, - Value: t.Labels[j].Value, - } - } + // Copy labels so we allocate memory only for labels, nothing else. + // TODO(bwplotka): Use improvement https://github.com/prometheus/prometheus/pull/8600 + lset := labelpb.CopyZLabelsToPromLabels(t.Labels) // Append as many valid samples as possible, but keep track of the errors. for _, s := range t.Samples { diff --git a/pkg/store/labelpb/label.go b/pkg/store/labelpb/label.go index 0614eb8294..eacf6e64e7 100644 --- a/pkg/store/labelpb/label.go +++ b/pkg/store/labelpb/label.go @@ -34,10 +34,23 @@ func ZLabelsFromPromLabels(lset labels.Labels) []ZLabel { // ZLabelsToPromLabels convert slice of labelpb.ZLabel to Prometheus labels in type unsafe manner. // It reuses the same memory. Caller should abort using passed []ZLabel. +// NOTE: Use with care. ZLabels holds memory from the whole protobuf unmarshal. func ZLabelsToPromLabels(lset []ZLabel) labels.Labels { return *(*labels.Labels)(unsafe.Pointer(&lset)) } +// CopyZLabelsToPromLabels convert slice of labelpb.ZLabel to Prometheus labels by copying all underlying bytes. +func CopyZLabelsToPromLabels(lset []ZLabel) labels.Labels { + ret := make(labels.Labels, len(lset)) + for j := range lset { + ret[j] = labels.Label{ + Name: string(*(*[]byte)(unsafe.Pointer(&lset[j].Name))), + Value: string(*(*[]byte)(unsafe.Pointer(&lset[j].Value))), + } + } + return *(*labels.Labels)(unsafe.Pointer(&lset)) +} + // LabelsFromPromLabels converts Prometheus labels to slice of labelpb.ZLabel in type unsafe manner. // It reuses the same memory. Caller should abort using passed labels.Labels. func LabelsFromPromLabels(lset labels.Labels) []Label {