cockroachdb / cockroachdb/cockroach

admission: root cause & fix the slight negative perf impact of cpuTimeTokenFiller

Open
#161,945 1 comment 0 reactions 0 assignees View on GitHub
A-admission-control C-enhancement T-kv
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

An initial version of https://github.com/cockroachdb/cockroach/pull/161179 had a small but but statistically significant regression in SQL level benchmark results. That PR hooked in CPU time token AC, but slot-based AC is still enabled by default, so CPU time token AC is not in the hot path. The likely proximate cause was thus the `cpuTimeTokenFiller` goroutine, which runs every 1ms. Some benchmarking was done of `cpuTimeTokenFiller`, and no glaring issues were found. Still, making the filler background process start only when CPU time token AC was enabled resolved the regression:

```
// The filler ticking appears to have a slight negative impact on perf.
// For now, we accept this, since CPU time token AC will be off by
// default, and only enabled in Serverless. To track fixing the perf
// issue, we have the following ticket:
// https://github.com/cockroachdb/cockroach/issues/161945
if !knobs.DisableCPUTimeTokenFillerGoroutine {
var fillerStarted bool
cpuTimeTokenACEnabled.SetOnChange(&settings.SV, func(ctx context.Context) {
if !fillerStarted && cpuTimeTokenACEnabled.Get(&settings.SV) {
filler.start(ambientCtx.AnnotateCtx(context.Background()))
fillerStarted = true
}
})
}
```

This ticket tracks root causing & fixing the slight negative perf impact of `cpuTimeTokenFiller`. Since the impact is slight, we may not do this until we modify CPU time token AC to support SH & Dedicated, instead of just Serverless.

Make sure to check out https://github.com/cockroachdb/cockroach/pull/161179 for an initial analysis of the regression.

Jira issue: CRDB-59188

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.