From 05103d91dad1aec999e2e63975c56c496bc47e41 Mon Sep 17 00:00:00 2001
From: Nikolay <nik@victoriametrics.com>
Date: Tue, 10 Oct 2023 13:44:02 +0200
Subject: [PATCH] app/vmselect: reduce lock contention for heavy aggregation
requests (#5119)
reduce lock contention for heavy aggregation requests
previously lock contetion may happen on machine with big number of CPU due to enabled string interning. sync.Map was a choke point for all aggregation requests.
Now instead of interning, new string is created. It may increase CPU and memory usage for some cases.
https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5087
---
app/vmselect/promql/aggr.go | 3 +--
app/vmselect/promql/aggr_incremental.go | 3 +--
app/vmselect/promql/binary_op.go | 6 ++----
app/vmselect/promql/eval.go | 7 +++----
app/vmselect/promql/exec.go | 3 +--
app/vmselect/promql/rollup_result_cache.go | 5 ++---
app/vmselect/promql/transform.go | 8 ++++----
docs/CHANGELOG.md | 1 +
8 files changed, 15 insertions(+), 21 deletions(-)
diff --git a/app/vmselect/promql/aggr.go b/app/vmselect/promql/aggr.go
index 3ba78615a9..c3310ee22f 100644
--- a/app/vmselect/promql/aggr.go
+++ b/app/vmselect/promql/aggr.go
@@ -8,7 +8,6 @@ import (
"strconv"
"strings"
- "github.com/VictoriaMetrics/VictoriaMetrics/lib/bytesutil"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/logger"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/storage"
"github.com/VictoriaMetrics/metrics"
@@ -130,7 +129,7 @@ func aggrPrepareSeries(argOrig []*timeseries, modifier *metricsql.ModifierExpr,
for i, ts := range arg {
removeGroupTags(&ts.MetricName, modifier)
bb.B = marshalMetricNameSorted(bb.B[:0], &ts.MetricName)
- k := bytesutil.InternBytes(bb.B)
+ k := string(bb.B)
if keepOriginal {
ts = argOrig[i]
}
diff --git a/app/vmselect/promql/aggr_incremental.go b/app/vmselect/promql/aggr_incremental.go
index f3d5bd4167..c87ba7bb35 100644
--- a/app/vmselect/promql/aggr_incremental.go
+++ b/app/vmselect/promql/aggr_incremental.go
@@ -6,7 +6,6 @@ import (
"unsafe"
"github.com/VictoriaMetrics/VictoriaMetrics/app/vmselect/netstorage"
- "github.com/VictoriaMetrics/VictoriaMetrics/lib/bytesutil"
"github.com/VictoriaMetrics/metricsql"
)
@@ -105,7 +104,7 @@ func (iafc *incrementalAggrFuncContext) updateTimeseries(tsOrig *timeseries, wor
removeGroupTags(&ts.MetricName, &iafc.ae.Modifier)
bb := bbPool.Get()
bb.B = marshalMetricNameSorted(bb.B[:0], &ts.MetricName)
- k := bytesutil.InternBytes(bb.B)
+ k := string(bb.B)
iac := m[k]
if iac == nil {
if iafc.ae.Limit > 0 && len(m) >= iafc.ae.Limit {
diff --git a/app/vmselect/promql/binary_op.go b/app/vmselect/promql/binary_op.go
index 3c354946a8..d020825569 100644
--- a/app/vmselect/promql/binary_op.go
+++ b/app/vmselect/promql/binary_op.go
@@ -5,7 +5,6 @@ import (
"math"
"strings"
- "github.com/VictoriaMetrics/VictoriaMetrics/lib/bytesutil"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/logger"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/storage"
"github.com/VictoriaMetrics/metricsql"
@@ -261,8 +260,7 @@ func groupJoin(singleTimeseriesSide string, be *metricsql.BinaryOpExpr, rvsLeft,
bb.B = marshalMetricTagsSorted(bb.B[:0], &tsCopy.MetricName)
pair, ok := m[string(bb.B)]
if !ok {
- k := bytesutil.InternBytes(bb.B)
- m[k] = &tsPair{
+ m[string(bb.B)] = &tsPair{
left: &tsCopy,
right: tsRight,
}
@@ -524,7 +522,7 @@ func createTimeseriesMapByTagSet(be *metricsql.BinaryOpExpr, left, right []*time
logger.Panicf("BUG: unexpected binary op modifier %q", groupOp)
}
bb.B = marshalMetricTagsSorted(bb.B[:0], mn)
- k := bytesutil.InternBytes(bb.B)
+ k := string(bb.B)
m[k] = append(m[k], ts)
}
storage.PutMetricName(mn)
diff --git a/app/vmselect/promql/eval.go b/app/vmselect/promql/eval.go
index a9a85f6247..b7593f9639 100644
--- a/app/vmselect/promql/eval.go
+++ b/app/vmselect/promql/eval.go
@@ -548,8 +548,8 @@ func getCommonLabelFilters(tss []*timeseries) []metricsql.LabelFilter {
for _, tag := range ts.MetricName.Tags {
vc, ok := m[string(tag.Key)]
if !ok {
- k := bytesutil.InternBytes(tag.Key)
- v := bytesutil.InternBytes(tag.Value)
+ k := string(tag.Key)
+ v := string(tag.Value)
m[k] = &valuesCounter{
values: map[string]struct{}{
v: {},
@@ -566,8 +566,7 @@ func getCommonLabelFilters(tss []*timeseries) []metricsql.LabelFilter {
}
vc.count++
if _, ok := vc.values[string(tag.Value)]; !ok {
- v := bytesutil.InternBytes(tag.Value)
- vc.values[v] = struct{}{}
+ vc.values[string(tag.Value)] = struct{}{}
}
}
}
diff --git a/app/vmselect/promql/exec.go b/app/vmselect/promql/exec.go
index acbaa59d5b..280969a77a 100644
--- a/app/vmselect/promql/exec.go
+++ b/app/vmselect/promql/exec.go
@@ -12,7 +12,6 @@ import (
"github.com/VictoriaMetrics/VictoriaMetrics/app/vmselect/netstorage"
"github.com/VictoriaMetrics/VictoriaMetrics/app/vmselect/querystats"
- "github.com/VictoriaMetrics/VictoriaMetrics/lib/bytesutil"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/decimal"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/querytracer"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/storage"
@@ -122,7 +121,7 @@ func timeseriesToResult(tss []*timeseries, maySort bool) ([]netstorage.Result, e
bb := bbPool.Get()
for i, ts := range tss {
bb.B = marshalMetricNameSorted(bb.B[:0], &ts.MetricName)
- k := bytesutil.InternBytes(bb.B)
+ k := string(bb.B)
if _, ok := m[k]; ok {
return nil, fmt.Errorf(`duplicate output timeseries: %s`, stringMetricName(&ts.MetricName))
}
diff --git a/app/vmselect/promql/rollup_result_cache.go b/app/vmselect/promql/rollup_result_cache.go
index 376867db0f..b5ecaf1472 100644
--- a/app/vmselect/promql/rollup_result_cache.go
+++ b/app/vmselect/promql/rollup_result_cache.go
@@ -486,8 +486,7 @@ func mergeTimeseries(a, b []*timeseries, bStart int64, ec *EvalConfig) []*timese
defer bbPool.Put(bb)
for _, ts := range a {
bb.B = marshalMetricNameSorted(bb.B[:0], &ts.MetricName)
- k := bytesutil.InternBytes(bb.B)
- m[k] = ts
+ m[string(bb.B)] = ts
}
rvs := make([]*timeseries, 0, len(a))
@@ -499,7 +498,7 @@ func mergeTimeseries(a, b []*timeseries, bStart int64, ec *EvalConfig) []*timese
tmp.MetricName.MoveFrom(&tsB.MetricName)
bb.B = marshalMetricNameSorted(bb.B[:0], &tmp.MetricName)
- k := bytesutil.InternBytes(bb.B)
+ k := string(bb.B)
tsA := m[k]
if tsA == nil {
tStart := ec.Start
diff --git a/app/vmselect/promql/transform.go b/app/vmselect/promql/transform.go
index 94da2a5727..709fd190a2 100644
--- a/app/vmselect/promql/transform.go
+++ b/app/vmselect/promql/transform.go
@@ -419,7 +419,7 @@ func transformBucketsLimit(tfa *transformFuncArg) ([]*timeseries, error) {
mn.CopyFrom(&ts.MetricName)
mn.RemoveTag("le")
b = marshalMetricNameSorted(b[:0], &mn)
- k := bytesutil.InternBytes(b)
+ k := bytesutil.ToUnsafeString(b)
m[k] = append(m[k], x{
le: le,
ts: ts,
@@ -522,7 +522,7 @@ func vmrangeBucketsToLE(tss []*timeseries) []*timeseries {
ts.MetricName.RemoveTag("le")
ts.MetricName.RemoveTag("vmrange")
bb.B = marshalMetricNameSorted(bb.B[:0], &ts.MetricName)
- k := bytesutil.InternBytes(bb.B)
+ k := string(bb.B)
m[k] = append(m[k], x{
startStr: startStr,
endStr: endStr,
@@ -1022,7 +1022,7 @@ func groupLeTimeseries(tss []*timeseries) map[string][]leTimeseries {
ts.MetricName.ResetMetricGroup()
ts.MetricName.RemoveTag("le")
bb.B = marshalMetricTagsSorted(bb.B[:0], &ts.MetricName)
- k := bytesutil.InternBytes(bb.B)
+ k := string(bb.B)
m[k] = append(m[k], leTimeseries{
le: le,
ts: ts,
@@ -1656,7 +1656,7 @@ func transformUnion(tfa *transformFuncArg) ([]*timeseries, error) {
for _, arg := range args {
for _, ts := range arg {
bb.B = marshalMetricNameSorted(bb.B[:0], &ts.MetricName)
- k := bytesutil.InternBytes(bb.B)
+ k := string(bb.B)
if m[k] {
continue
}
diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
index 90cc54e5e6..568028cfeb 100644
--- a/docs/CHANGELOG.md
+++ b/docs/CHANGELOG.md
@@ -14,6 +14,7 @@ The following `tip` changes can be tested by building VictoriaMetrics components
* SECURITY: upgrade Go builder from Go1.21.1 to Go1.21.3. See [the list of issues addressed in Go1.21.2](https://github.com/golang/go/issues?q=milestone%3AGo1.21.2+label%3ACherryPickApproved) and [the list of issues addressed in Go1.21.3](https://github.com/golang/go/issues?q=milestone%3AGo1.21.3+label%3ACherryPickApproved).
* BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert.html): strip sensitive information such as auth headers or passwords from datasource, remote-read, remote-write or notifier URLs in log messages or UI. This behavior is by default and is controlled via `-datasource.showURL`, `-remoteRead.showURL`, `remoteWrite.showURL` or `-notifier.showURL` cmd-line flags. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5044).
+* BUGFIX: [vmselect](https://docs.victoriametrics.com/Cluster-VictoriaMetrics.html): improve performance and memory usage during query processing on machines with big number of CPU cores. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5087) for details.
* BUGFIX: [vmbackup](https://docs.victoriametrics.com/vmbackup.html): properly copy `appliedRetention.txt` files inside `<-storageDataPath>/{data}` folders during [incremental backups](https://docs.victoriametrics.com/vmbackup.html#incremental-backups). Previously the new `appliedRetention.txt` could be skipped during incremental backups, which could lead to increased load on storage after restoring from backup. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5005).
* BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): suppress `context canceled` error messages in logs when `vmagent` is reloading service discovery config. This error could appear starting from [v1.93.5](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.93.5). See [this PR](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/5048).
* BUGFIX: [MetricsQL](https://docs.victoriametrics.com/MetricsQL.html): allow passing [median_over_time](https://docs.victoriametrics.com/MetricsQL.html#median_over_time) to [aggr_over_time](https://docs.victoriametrics.com/MetricsQL.html#aggr_over_time). See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5034).