Skip to content

Commit

Permalink
Throttler: reduce regexp/string allocations by pre-computing pascal c…
Browse files Browse the repository at this point in the history
…ase (#17817)

Signed-off-by: Shlomi Noach <[email protected]>
  • Loading branch information
shlomi-noach authored Feb 25, 2025
1 parent afdb26b commit 9ca2135
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 5 deletions.
11 changes: 11 additions & 0 deletions go/vt/vttablet/tabletserver/throttle/base/metric_name.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import (
"fmt"
"slices"
"strings"

"vitess.io/vitess/go/textutil"
)

// MetricName is a formalized name for a metric, such as "lag" or "threads_running". A metric name
Expand Down Expand Up @@ -77,6 +79,14 @@ func (metric MetricName) DefaultScope() Scope {
return SelfScope
}

// Pascal case representation of this name, e.g. "ThreadsRunning"
func (metric MetricName) Pascal() string {
if pascal, ok := pascalMetricNames[metric]; ok {
return pascal
}
return textutil.PascalCase(metric.String())
}

func (metric MetricName) String() string {
return string(metric)
}
Expand Down Expand Up @@ -113,6 +123,7 @@ var (
// - no textual parsing is needed in the critical path
// - we can easily check if a metric name is valid
aggregatedMetricNames = make(map[string]AggregatedMetricName)
pascalMetricNames = make(map[MetricName]string)
)

// DisaggregateMetricName splits a metric name into its scope name and metric name
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ func TestKnownMetricNamesPascalCase(t *testing.T) {
t.Run(metricName.String(), func(t *testing.T) {
expect, ok := expectCases[metricName]
require.True(t, ok)
assert.Equal(t, expect, metricName.Pascal())
assert.Equal(t, expect, textutil.PascalCase(metricName.String()))
})
}
Expand Down
12 changes: 12 additions & 0 deletions go/vt/vttablet/tabletserver/throttle/base/metric_scope.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,18 @@ const (
SelfScope Scope = "self"
)

var (
pascalScopeNames = map[Scope]string{
UndefinedScope: "",
ShardScope: "Shard",
SelfScope: "Self",
}
)

func (s Scope) Pascal() string {
return pascalScopeNames[s]
}

func (s Scope) String() string {
return string(s)
}
Expand Down
37 changes: 37 additions & 0 deletions go/vt/vttablet/tabletserver/throttle/base/metric_scope_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
Copyright 2024 The Vitess Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package base

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestScopePascalCase(t *testing.T) {
expectCases := map[Scope]string{
UndefinedScope: "",
ShardScope: "Shard",
SelfScope: "Self",
}
for scope, expect := range expectCases {
t.Run(scope.String(), func(t *testing.T) {
pascal := scope.Pascal()
assert.Equal(t, expect, pascal)
})
}
}
2 changes: 2 additions & 0 deletions go/vt/vttablet/tabletserver/throttle/base/self_metric.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"fmt"
"strconv"

"vitess.io/vitess/go/textutil"
"vitess.io/vitess/go/vt/topo"
"vitess.io/vitess/go/vt/vttablet/tabletserver/connpool"
"vitess.io/vitess/go/vt/vttablet/tmclient"
Expand Down Expand Up @@ -48,6 +49,7 @@ var (
func registerSelfMetric(selfMetric SelfMetric) SelfMetric {
RegisteredSelfMetrics[selfMetric.Name()] = selfMetric
KnownMetricNames = append(KnownMetricNames, selfMetric.Name())
pascalMetricNames[selfMetric.Name()] = textutil.PascalCase(selfMetric.Name().String())
aggregatedMetricNames[selfMetric.Name().String()] = AggregatedMetricName{
Scope: selfMetric.DefaultScope(),
Metric: selfMetric.Name(),
Expand Down
9 changes: 4 additions & 5 deletions go/vt/vttablet/tabletserver/throttle/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ import (
"time"

"vitess.io/vitess/go/stats"
"vitess.io/vitess/go/textutil"
"vitess.io/vitess/go/vt/vttablet/tabletserver/throttle/base"
"vitess.io/vitess/go/vt/vttablet/tabletserver/throttle/throttlerapp"

Expand Down Expand Up @@ -175,9 +174,9 @@ func (check *ThrottlerCheck) Check(ctx context.Context, appName string, scope ba
// Out of abundance of caution, we will protect against such a scenario.
return
}
stats.GetOrNewCounter(fmt.Sprintf("ThrottlerCheck%s%sTotal", textutil.PascalCase(metricScope.String()), textutil.PascalCase(metricName.String())), "").Add(1)
stats.GetOrNewCounter(fmt.Sprintf("ThrottlerCheck%s%sTotal", metricScope.Pascal(), metricName.Pascal()), "").Add(1)
if !metricCheckResult.IsOK() {
stats.GetOrNewCounter(fmt.Sprintf("ThrottlerCheck%s%sError", textutil.PascalCase(metricScope.String()), textutil.PascalCase(metricName.String())), "").Add(1)
stats.GetOrNewCounter(fmt.Sprintf("ThrottlerCheck%s%sError", metricScope.Pascal(), metricName.Pascal()), "").Add(1)
}
}(metricCheckResult)
}
Expand Down Expand Up @@ -232,7 +231,7 @@ func (check *ThrottlerCheck) localCheck(ctx context.Context, aggregatedMetricNam
check.throttler.markMetricHealthy(aggregatedMetricName)
}
if timeSinceHealthy, found := check.throttler.timeSinceMetricHealthy(aggregatedMetricName); found {
go stats.GetOrNewGauge(fmt.Sprintf("ThrottlerCheck%sSecondsSinceHealthy", textutil.PascalCase(scope.String())), fmt.Sprintf("seconds since last healthy check for %v", scope)).Set(int64(timeSinceHealthy.Seconds()))
go stats.GetOrNewGauge(fmt.Sprintf("ThrottlerCheck%sSecondsSinceHealthy", scope.Pascal()), fmt.Sprintf("seconds since last healthy check for %v", scope)).Set(int64(timeSinceHealthy.Seconds()))
}

return checkResult
Expand All @@ -244,7 +243,7 @@ func (check *ThrottlerCheck) reportAggregated(aggregatedMetricName string, metri
return
}
if value, err := metricResult.Get(); err == nil {
stats.GetOrNewGaugeFloat64(fmt.Sprintf("ThrottlerAggregated%s%s", textutil.PascalCase(scope.String()), textutil.PascalCase(metricName.String())), fmt.Sprintf("aggregated value for %v", scope)).Set(value)
stats.GetOrNewGaugeFloat64(fmt.Sprintf("ThrottlerAggregated%s%s", scope.Pascal(), metricName.Pascal()), fmt.Sprintf("aggregated value for %v", scope)).Set(value)
}
}

Expand Down

0 comments on commit 9ca2135

Please sign in to comment.