Skip to content

Commit

Permalink
split: add string methods for debugging/logging
Browse files Browse the repository at this point in the history 
Previously, there was no way to peak the contents of the load based
splitter samples when inspecting nodes. This commit adds string methods
for the `UnweightedFinder`, `WeightedFinder` and `Decider`.

This commit also swaps the order of the should split check to avoid
computation. As a result the output of `cpu_decider_cartesian` changed
slightly as the no split key logging message is now ordered differently.

Informs: cockroachdb#103672
Informs: cockroachdb#103483

Release note: None
  • Loading branch information
@kvoli
kvoli committed May 24, 2023
1 parent a1b8517 commit b5cdee7
Show file tree
Hide file tree
Showing 9 changed files with 192 additions and 44 deletions.
7 changes: 4 additions & 3 deletions pkg/cmd/roachtest/tests/split.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,8 @@ func runLoadSplits(ctx context.Context, t test.Test, c cluster.Cluster, params s
c.Put(ctx, t.DeprecatedWorkload(), "./workload", c.Node(4))
startOpts := option.DefaultStartOptsNoBackups()
startOpts.RoachprodOpts.ExtraArgs = append(startOpts.RoachprodOpts.ExtraArgs,
"--vmodule=split_queue=2,store_rebalancer=2,allocator=2,replicate_queue=2",
"--vmodule=split_queue=2,store_rebalancer=2,allocator=2,replicate_queue=2,"+
"decider=3,replica_split_load=1",
)
c.Start(ctx, t.L(), startOpts, install.MakeClusterSettings(), c.All())

Expand All @@ -424,7 +425,7 @@ func runLoadSplits(ctx context.Context, t test.Test, c cluster.Cluster, params s
// Set the objective to QPS or CPU and update the load split threshold
// appropriately.
if params.qpsThreshold > 0 {
t.Status("setting split objective to QPS with threshold %d", params.qpsThreshold)
t.Status("setting split objective to QPS with threshold ", params.qpsThreshold)
if err := setLoadBasedRebalancingObjective(ctx, db, "qps"); err != nil {
return err
}
Expand All @@ -433,7 +434,7 @@ func runLoadSplits(ctx context.Context, t test.Test, c cluster.Cluster, params s
return err
}
} else if params.cpuThreshold > 0 {
t.Status("setting split objective to CPU with threshold %s", params.cpuThreshold)
t.Status("setting split objective to CPU with threshold ", params.cpuThreshold)
if err := setLoadBasedRebalancingObjective(ctx, db, "cpu"); err != nil {
return err
}
Expand Down
1 change: 1 addition & 0 deletions pkg/kv/kvserver/split/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ go_library(
"//pkg/util/log",
"//pkg/util/metric",
"//pkg/util/syncutil",
"@com_github_cockroachdb_redact//:redact",
],
)

Expand Down
52 changes: 40 additions & 12 deletions pkg/kv/kvserver/split/decider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ package split

import (
"context"
"fmt"
"math/rand"
"time"

Expand All @@ -23,13 +22,15 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/metric"
"github.com/cockroachdb/cockroach/pkg/util/syncutil"
"github.com/cockroachdb/redact"
)

const minSplitSuggestionInterval = time.Minute
const minNoSplitKeyLoggingMetricsInterval = time.Minute
const minPerSecondSampleDuration = time.Second

type LoadBasedSplitter interface {
redact.SafeFormatter
// Record informs the LoadBasedSplitter about where the span lies with regard
// to the keys in the samples.
Record(span roachpb.Span, weight float64)
Expand All @@ -46,11 +47,14 @@ type LoadBasedSplitter interface {
// number of samples that don't pass each split key requirement if not all
// samples are invalid due to insufficient counters, otherwise returns an
// empty string.
NoSplitKeyCauseLogMsg() string
NoSplitKeyCauseLogMsg() redact.RedactableString

// PopularKeyFrequency returns the percentage that the most popular key
// appears in the sampled candidate split keys.
PopularKeyFrequency() float64

// String formats the state of the load based splitter.
String() string
}

type LoadSplitConfig interface {
Expand Down Expand Up @@ -154,6 +158,7 @@ type Decider struct {
// Fields tracking split key suggestions.
splitFinder LoadBasedSplitter // populated when engaged or decided
lastSplitSuggestion time.Time // last stipulation to client to carry out split
suggestionsMade int // suggestions made since last reset

// Fields tracking logging / metrics around load-based splitter split key.
lastNoSplitKeyLoggingMetrics time.Time
Expand All @@ -175,6 +180,23 @@ func Init(
lbs.mu.objective = objective
}

type lockedDecider Decider

func (ld *lockedDecider) SafeFormat(w redact.SafePrinter, r rune) {
w.Printf(
"objective=%v count=%d suggestions=%d last=%.1f last-roll=%v last-suggest=%v",
ld.mu.objective, ld.mu.count, ld.mu.suggestionsMade, ld.mu.lastStatVal,
ld.mu.lastStatRollover, ld.mu.lastSplitSuggestion,
)
if ld.mu.splitFinder != nil {
w.Printf(" %v", ld.mu.splitFinder)
}
}

func (ld *lockedDecider) String() string {
return redact.StringWithoutMarkers(ld)
}

// Record notifies the Decider that 'n' operations are being carried out which
// operate on the span returned by the supplied method. The closure will only
// be called when necessary, that is, when the Decider is considering a split
Expand Down Expand Up @@ -231,20 +253,25 @@ func (d *Decider) recordLocked(
if s.Key != nil {
d.mu.splitFinder.Record(span(), float64(n))
}
if d.mu.splitFinder.Ready(now) {
if d.mu.splitFinder.Key() != nil {
if now.Sub(d.mu.lastSplitSuggestion) > minSplitSuggestionInterval {
d.mu.lastSplitSuggestion = now
return true
}
// We don't want to check for a split key if we don't need to as it
// requires some computation. When the splitFinder isn't ready or we
// recently suggested a split, skip the key check.
if d.mu.splitFinder.Ready(now) &&
now.Sub(d.mu.lastSplitSuggestion) > minSplitSuggestionInterval {
if splitKey := d.mu.splitFinder.Key(); splitKey != nil {
log.KvDistribution.VEventf(ctx, 3, "suggesting split key %v splitter_state=%v",
splitKey, (*lockedDecider)(d))
d.mu.lastSplitSuggestion = now
d.mu.suggestionsMade++
return true
} else {
if now.Sub(d.mu.lastNoSplitKeyLoggingMetrics) > minNoSplitKeyLoggingMetricsInterval {
d.mu.lastNoSplitKeyLoggingMetrics = now
noSplitKeyCauseLogMsg := d.mu.splitFinder.NoSplitKeyCauseLogMsg()
if noSplitKeyCauseLogMsg != "" {
if causeMsg := d.mu.splitFinder.NoSplitKeyCauseLogMsg(); causeMsg != "" {
popularKeyFrequency := d.mu.splitFinder.PopularKeyFrequency()
noSplitKeyCauseLogMsg += fmt.Sprintf(", most popular key occurs in %d%% of samples", int(popularKeyFrequency*100))
log.KvDistribution.Infof(ctx, "%s", noSplitKeyCauseLogMsg)
log.KvDistribution.Infof(ctx, "%s, most popular key occurs in %d%% of samples",
causeMsg, int(popularKeyFrequency*100))
log.KvDistribution.VInfof(ctx, 3, "splitter_state=%v", (*lockedDecider)(d))
if popularKeyFrequency >= splitKeyThreshold {
d.loadSplitterMetrics.PopularKeyCount.Inc(1)
}
Expand Down Expand Up @@ -349,6 +376,7 @@ func (d *Decider) resetLocked(now time.Time) {
d.mu.count = 0
d.mu.maxStat.reset(now, d.config.StatRetention())
d.mu.splitFinder = nil
d.mu.suggestionsMade = 0
d.mu.lastSplitSuggestion = time.Time{}
d.mu.lastNoSplitKeyLoggingMetrics = time.Time{}
}
Expand Down
49 changes: 36 additions & 13 deletions pkg/kv/kvserver/split/load_based_splitter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"fmt"
"math"
"sort"
"strings"
"testing"
"text/tabwriter"
"time"
Expand Down Expand Up @@ -342,6 +343,7 @@ type lbsTestSettings struct {
finderConfig *finderConfig
seed uint64
iterations int
showLastState bool
}

func uint32ToKey(key uint32) roachpb.Key {
Expand Down Expand Up @@ -444,6 +446,7 @@ type repeatedResult struct {
avgOptimalPercentDifference, maxOptimalPercentDifference float64
avgRecordExecutionTime, avgKeyExecutionTime time.Duration
noKeyFoundPercent float64
lastStateString string
}

func resultTable(configs []multiReqConfig, rr []repeatedResult, showTiming bool) string {
Expand Down Expand Up @@ -506,7 +509,6 @@ func resultTable(configs []multiReqConfig, rr []repeatedResult, showTiming bool)
_ = w.Flush()

return buf.String()

}

func runTestRepeated(settings *lbsTestSettings) repeatedResult {
Expand All @@ -515,13 +517,15 @@ func runTestRepeated(settings *lbsTestSettings) repeatedResult {
avgOptimalPercentDifference, maxOptimalPercentDifference float64
avgRecordExecutionTime, avgKeyExecutionTime time.Duration
noKeyFoundPercent float64
lastStateString string
)
randSource := rand.New(rand.NewSource(settings.seed))
requestGen := settings.requestConfig.makeGenerator(randSource)

for i := 0; i < settings.iterations; i++ {
var recordFn func(span roachpb.Span, weight int)
var keyFn func() roachpb.Key
var stringFn func() string

if settings.deciderConfig != nil {
d := settings.deciderConfig.makeDecider(randSource)
Expand All @@ -540,6 +544,7 @@ func runTestRepeated(settings *lbsTestSettings) repeatedResult {
keyFn = func() roachpb.Key {
return d.MaybeSplitKey(ctx, now)
}
stringFn = (*lockedDecider)(d).String
} else {
f := settings.finderConfig.makeFinder(randSource)
recordFn = func(span roachpb.Span, weight int) {
Expand All @@ -548,6 +553,7 @@ func runTestRepeated(settings *lbsTestSettings) repeatedResult {
keyFn = func() roachpb.Key {
return f.Key()
}
stringFn = f.String
}
ret := runTest(recordFn, keyFn, randSource, requestGen)

Expand All @@ -571,6 +577,9 @@ func runTestRepeated(settings *lbsTestSettings) repeatedResult {
avgRecordExecutionTime += ret.recordExecutionTime
avgKeyExecutionTime += ret.keyExecutionTime

if i == settings.iterations-1 && settings.showLastState {
lastStateString = stringFn()
}
}

nRuns := settings.iterations
Expand All @@ -592,6 +601,7 @@ func runTestRepeated(settings *lbsTestSettings) repeatedResult {
avgRecordExecutionTime: avgRecordExecutionTime,
avgKeyExecutionTime: avgKeyExecutionTime,
noKeyFoundPercent: noKeyFoundPercent,
lastStateString: lastStateString,
}
}

Expand Down Expand Up @@ -821,7 +831,7 @@ func TestDataDriven(t *testing.T) {
findConfig = &finderConfig{
weighted: weighted,
}

decConfig = nil
case "decider":
var duration int
var threshold int
Expand All @@ -841,24 +851,20 @@ func TestDataDriven(t *testing.T) {
objective: splitObj,
duration: time.Duration(duration) * time.Second,
}
findConfig = nil
case "eval":
var seed uint64
var iterations, mixCount int
var showTiming, cartesian, all bool
var showTiming, cartesian, all, showLastState bool
var mix string
var mixT mixType

d.ScanArgs(t, "seed", &seed)
d.ScanArgs(t, "iterations", &iterations)
if d.HasArg("timing") {
d.ScanArgs(t, "timing", &showTiming)
}
if d.HasArg("cartesian") {
d.ScanArgs(t, "cartesian", &cartesian)
}
if d.HasArg("all") {
d.ScanArgs(t, "all", &all)
}
d.MaybeScanArgs(t, "timing", &showTiming)
d.MaybeScanArgs(t, "cartesian", &cartesian)
d.MaybeScanArgs(t, "all", &all)
d.MaybeScanArgs(t, "show_last", &showLastState)
if d.HasArg("mix") {
d.ScanArgs(t, "mix", &mix)
d.ScanArgs(t, "mix_count", &mixCount)
Expand Down Expand Up @@ -900,9 +906,26 @@ func TestDataDriven(t *testing.T) {
finderConfig: findConfig,
seed: seed,
iterations: iterations,
showLastState: showLastState,
})
}
return resultTable(evalRequestConfigs, repeatedResults, showTiming)
retTable := resultTable(evalRequestConfigs, repeatedResults, showTiming)
if showLastState {
showRequestConfgDesc := len(evalRequestConfigs) > 1
var buf strings.Builder
for i := range evalRequestConfigs {
if i > 0 {
buf.WriteString("\n")
}
if showRequestConfgDesc {
buf.WriteString(evalRequestConfigs[i].String())
}
fmt.Fprintf(&buf, "\t%s",
repeatedResults[i].lastStateString)
}
return fmt.Sprintf("%s%s", retTable, buf.String())
}
return retTable
default:
return fmt.Sprintf("unknown command: %s", d.Cmd)
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/kv/kvserver/split/testdata/cpu_decider_cartesian
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ w=zip(10000)/k=zip(1000000)/s=zip(1000)/s(%)=95/10000
mixed_requests(2) 0.00 3.69 19.97 0.03 0.08
w=uni(100)/k=zip(10000)/s=zip(1000)/s(%)=20/10000
w=zip(10000)/k=zip(10000)/s=uni(1000)/s(%)=20/10000
mixed_requests(2) 0.00 6.36 15.63 0.01 0.02
mixed_requests(2) 0.00 6.64 21.33 0.01 0.02
w=zip(100)/k=uni(10000)/s=uni(1)/s(%)=0/10000
w=zip(100)/k=uni(10000)/s=zip(1000)/s(%)=95/10000
mixed_requests(2) 0.00 4.11 16.75 0.00 0.01
Expand Down Expand Up @@ -87,7 +87,7 @@ w=zip(100)/k=uni(1000000)/s=uni(1000)/s(%)=20/10000
mixed_requests(2) 0.00 6.29 24.99 0.00 0.02
w=uni(10000)/k=zip(10000)/s=uni(1)/s(%)=0/10000
w=zip(10000)/k=uni(1000000)/s=uni(1000)/s(%)=95/10000
mixed_requests(2) 0.00 6.39 13.28 0.02 0.07
mixed_requests(2) 0.00 6.24 13.28 0.02 0.07
w=zip(100)/k=uni(1000000)/s=zip(1000)/s(%)=95/10000
w=zip(10000)/k=uni(10000)/s=zip(1000)/s(%)=20/10000
mixed_requests(2) 0.00 4.08 11.02 0.03 0.08
Expand All @@ -102,7 +102,7 @@ w=zip(100)/k=uni(1000000)/s=uni(1000)/s(%)=95/10000
mixed_requests(2) 0.00 4.89 12.20 0.00 0.01
w=uni(100)/k=zip(1000000)/s=uni(1000)/s(%)=95/10000
w=uni(100)/k=uni(1000000)/s=uni(1000)/s(%)=20/10000
mixed_requests(2) 0.00 5.03 16.77 0.00 0.00
mixed_requests(2) 0.00 4.99 16.77 0.00 0.00
w=uni(10000)/k=uni(1000000)/s=uni(1000)/s(%)=95/10000
w=uni(10000)/k=zip(1000000)/s=zip(1000)/s(%)=95/10000
mixed_requests(2) 0.00 5.07 15.26 0.02 0.06
Expand Down
Loading

0 comments on commit b5cdee7

Please sign in to comment.