Skip to content

Commit ebfcb56

Browse files
craig[bot]angeladietz
craig[bot]
and
angeladietz
committed
Merge #158599
158599: mmaprototype: dont redact store state logs r=tbg a=angeladietz Many of the logs coming from the mmaprototype package were previously redacted which is unnecessary since they don't contain any sensitive information. This fixes some of the logs statements and SafeFormat implementations to ensure logs like store usages and capacities are not redacted. Informs #158203 Epic: [CRDB-55052](https://cockroachlabs.atlassian.net/browse/CRDB-55052) Release note: None Co-authored-by: Angela Dietz <[email protected]>
2 parents 804fa04 + fe91662 commit ebfcb56

File tree

5 files changed

+31
-26
lines changed

5 files changed

+31
-26
lines changed

‎pkg/kv/kvserver/allocator/mmaprototype/allocator_state.go‎

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,7 @@ func sortTargetCandidateSetAndPick(
494494
maxFractionPendingThreshold float64,
495495
) roachpb.StoreID {
496496
var b strings.Builder
497-
var formatCandidatesLog = func(b *strings.Builder, candidates []candidateInfo) bool {
497+
var formatCandidatesLog = func(b *strings.Builder, candidates []candidateInfo) redact.SafeString {
498498
b.Reset()
499499
for _, c := range candidates {
500500
if overloadedDim != NumLoadDimensions {
@@ -503,11 +503,10 @@ func sortTargetCandidateSetAndPick(
503503
fmt.Fprintf(b, " s%v(SLS:%v)", c.StoreID, c.storeLoadSummary)
504504
}
505505
}
506-
if b.Len() > 0 {
506+
if len(candidates) > 0 {
507507
fmt.Fprintf(b, ", overloadedDim:%s", overloadedDim)
508-
return true
509508
}
510-
return false
509+
return redact.SafeString(b.String())
511510
}
512511

513512
if loadThreshold <= loadNoChange {
@@ -536,8 +535,8 @@ func sortTargetCandidateSetAndPick(
536535
bestDiversity = cand.diversityScore
537536
} else {
538537
// Have a set of candidates.
539-
if formatCandidatesLog(&b, cands.candidates[i:]) {
540-
log.KvDistribution.VEventf(ctx, 2, "discarding candidates due to lower diversity score: %s", b.String())
538+
if s := formatCandidatesLog(&b, cands.candidates[i:]); s != "" {
539+
log.KvDistribution.VEventf(ctx, 2, "discarding candidates due to lower diversity score: %s", s)
541540
}
542541
break
543542
}
@@ -579,9 +578,9 @@ func sortTargetCandidateSetAndPick(
579578
// Never go to the next set if we have discarded candidates that have
580579
// pending changes. We will wait for those to have no pending changes
581580
// before we consider later sets.
582-
if formatCandidatesLog(&b, cands.candidates[i:]) {
581+
if s := formatCandidatesLog(&b, cands.candidates[i:]); s != "" {
583582
log.KvDistribution.VEventf(ctx, 2,
584-
"candidate with pending changes was discarded, discarding remaining candidates with higher load: %s", b.String())
583+
"candidate with pending changes was discarded, discarding remaining candidates with higher load: %s", s)
585584
}
586585
break
587586
}
@@ -593,9 +592,9 @@ func sortTargetCandidateSetAndPick(
593592
lowestLoadSet = cand.sls
594593
} else if ignoreLevel < ignoreHigherThanLoadThreshold || overloadedDim == NumLoadDimensions {
595594
// Past the lowestLoad set. We don't care about these.
596-
if formatCandidatesLog(&b, cands.candidates[i:]) {
595+
if s := formatCandidatesLog(&b, cands.candidates[i:]); s != "" {
597596
log.KvDistribution.VEventf(ctx, 2,
598-
"discarding candidates with higher load than lowestLoadSet(%s): %s", lowestLoadSet.String(), b.String())
597+
"discarding candidates with higher load than lowestLoadSet(%s): %s", lowestLoadSet.String(), s)
599598
}
600599
break
601600
}
@@ -604,9 +603,9 @@ func sortTargetCandidateSetAndPick(
604603
// cand.sls <= loadThreshold.
605604
}
606605
if cand.sls > loadThreshold {
607-
if formatCandidatesLog(&b, cands.candidates[i:]) {
606+
if s := formatCandidatesLog(&b, cands.candidates[i:]); s != "" {
608607
log.KvDistribution.VEventf(ctx, 2,
609-
"discarding candidates with higher load than loadThreshold(%s): %s", loadThreshold.String(), b.String())
608+
"discarding candidates with higher load than loadThreshold(%s): %s", loadThreshold.String(), s)
610609
}
611610
break
612611
}
@@ -713,14 +712,14 @@ func sortTargetCandidateSetAndPick(
713712
}
714713
j++
715714
}
716-
if formatCandidatesLog(&b, cands.candidates[j:]) {
717-
log.KvDistribution.VEventf(ctx, 2, "discarding candidates due to overloadedDim: %s", b.String())
715+
if s := formatCandidatesLog(&b, cands.candidates[j:]); s != "" {
716+
log.KvDistribution.VEventf(ctx, 2, "discarding candidates due to overloadedDim: %s", s)
718717
}
719718
cands.candidates = cands.candidates[:j]
720719
}
721-
formatCandidatesLog(&b, cands.candidates)
720+
s := formatCandidatesLog(&b, cands.candidates)
722721
j = rng.Intn(j)
723-
log.KvDistribution.VEventf(ctx, 2, "sortTargetCandidateSetAndPick: candidates:%s, picked s%v", b.String(), cands.candidates[j].StoreID)
722+
log.KvDistribution.VEventf(ctx, 2, "sortTargetCandidateSetAndPick: candidates:%s, picked s%v", s, cands.candidates[j].StoreID)
724723
if ignoreLevel == ignoreLoadNoChangeAndHigher && cands.candidates[j].sls >= loadNoChange ||
725724
ignoreLevel == ignoreLoadThresholdAndHigher && cands.candidates[j].sls >= loadThreshold ||
726725
ignoreLevel == ignoreHigherThanLoadThreshold && cands.candidates[j].sls > loadThreshold {

‎pkg/kv/kvserver/allocator/mmaprototype/cluster_state_rebalance_stores.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"github.com/cockroachdb/cockroach/pkg/util/log"
1919
"github.com/cockroachdb/errors"
2020
"github.com/cockroachdb/logtags"
21+
"github.com/cockroachdb/redact"
2122
)
2223

2324
// rebalanceEnv tracks the state and outcomes of a rebalanceStores invocation.
@@ -311,7 +312,7 @@ func (re *rebalanceEnv) rebalanceStore(
311312
_, _ = fmt.Fprintf(&b, " r%d:%v", rangeID, load)
312313
}
313314
log.KvDistribution.Infof(ctx, "top-K[%s] ranges for s%d with lease on local s%d:%s",
314-
topKRanges.dim, store.StoreID, localStoreID, b.String())
315+
topKRanges.dim, store.StoreID, localStoreID, redact.SafeString(b.String()))
315316
} else {
316317
log.KvDistribution.Infof(ctx, "no top-K[%s] ranges found for s%d with lease on local s%d",
317318
topKRanges.dim, store.StoreID, localStoreID)

‎pkg/kv/kvserver/allocator/mmaprototype/load.go‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,11 @@ const (
3737
func (dim LoadDimension) SafeFormat(w redact.SafePrinter, _ rune) {
3838
switch dim {
3939
case CPURate:
40-
w.Printf("CPURate")
40+
w.SafeString("CPURate")
4141
case WriteBandwidth:
42-
w.Print("WriteBandwidth")
42+
w.SafeString("WriteBandwidth")
4343
case ByteSize:
44-
w.Printf("ByteSize")
44+
w.SafeString("ByteSize")
4545
default:
4646
panic("unknown LoadDimension")
4747
}
@@ -54,6 +54,8 @@ func (dim LoadDimension) String() string {
5454
// LoadValue is the load on a resource.
5555
type LoadValue int64
5656

57+
func (LoadValue) SafeValue() {}
58+
5759
// LoadVector represents a vector of loads, with one element for each resource
5860
// dimension.
5961
type LoadVector [NumLoadDimensions]LoadValue
@@ -472,15 +474,15 @@ func (ls loadSummary) String() string {
472474
func (ls loadSummary) SafeFormat(w redact.SafePrinter, _ rune) {
473475
switch ls {
474476
case loadLow:
475-
w.Print("loadLow")
477+
w.SafeString("loadLow")
476478
case loadNormal:
477-
w.Print("loadNormal")
479+
w.SafeString("loadNormal")
478480
case loadNoChange:
479-
w.Print("loadNoChange")
481+
w.SafeString("loadNoChange")
480482
case overloadSlow:
481-
w.Print("overloadSlow")
483+
w.SafeString("overloadSlow")
482484
case overloadUrgent:
483-
w.Print("overloadUrgent")
485+
w.SafeString("overloadUrgent")
484486
default:
485487
panic("unknown loadSummary")
486488
}

‎pkg/server/server.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -926,7 +926,7 @@ func NewServer(cfg Config, stopper *stop.Stopper) (serverctl.ServerStartupInterf
926926
}
927927
storeLoadMsg := mmaintegration.MakeStoreLoadMsg(storeDesc, origTimestampNanos)
928928
mmaAlloc.SetStore(state.StoreAttrAndLocFromDesc(storeDesc))
929-
mmaAlloc.ProcessStoreLoadMsg(context.TODO(), &storeLoadMsg)
929+
mmaAlloc.ProcessStoreLoadMsg(ctx, &storeLoadMsg)
930930
},
931931
)
932932

‎pkg/testutils/lint/passes/redactcheck/redactcheck.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,9 @@ func runAnalyzer(pass *analysis.Pass) (interface{}, error) {
110110
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/allocator/storepool": {
111111
"storeStatus": {},
112112
},
113+
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/allocator/mmaprototype": {
114+
"LoadValue": {},
115+
},
113116
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/closedts/ctpb": {
114117
"SeqNum": {},
115118
},

0 commit comments

Comments
 (0)