From b06849b9a2a2c0aab6950764ddff1bb125119df5 Mon Sep 17 00:00:00 2001 From: Nathan VanBenschoten Date: Thu, 9 Nov 2023 00:49:43 -0500 Subject: [PATCH] kv: fix redaction in merge logging This commit is like the previous commit, but for range merges instead of range splits. It improves the redaction so that the "reason" field will be usable when debugging cloud clusters. Epic: None Release note: None --- pkg/kv/kvserver/merge_queue.go | 14 +++++++------- pkg/kv/kvserver/replica_command.go | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/kv/kvserver/merge_queue.go b/pkg/kv/kvserver/merge_queue.go index b5534b67e9ef..17f2c9f45311 100644 --- a/pkg/kv/kvserver/merge_queue.go +++ b/pkg/kv/kvserver/merge_queue.go @@ -12,7 +12,6 @@ package kvserver import ( "context" - "fmt" "math" "time" @@ -28,6 +27,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/util/humanizeutil" "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/cockroachdb/errors" + "github.com/cockroachdb/redact" ) const ( @@ -273,7 +273,7 @@ func (mq *mergeQueue) process( mergedStats.Add(rhsStats) lhsLoadSplitSnap := lhsRepl.loadBasedSplitter.Snapshot(ctx, mq.store.Clock().PhysicalTime()) - var loadMergeReason string + var loadMergeReason redact.RedactableString if lhsRepl.SplitByLoadEnabled() { var canMergeLoad bool if canMergeLoad, loadMergeReason = canMergeRangeLoad( @@ -387,7 +387,7 @@ func (mq *mergeQueue) process( } log.VEventf(ctx, 2, "merging to produce range: %s-%s", mergedDesc.StartKey, mergedDesc.EndKey) - reason := fmt.Sprintf("lhs+rhs size (%s+%s=%s) below threshold (%s) %s", + reason := redact.Sprintf("lhs+rhs size (%s+%s=%s) below threshold (%s) %s", humanizeutil.IBytes(lhsStats.Total()), humanizeutil.IBytes(rhsStats.Total()), humanizeutil.IBytes(mergedStats.Total()), @@ -448,7 +448,7 @@ func (mq *mergeQueue) updateChan() <-chan time.Time { func canMergeRangeLoad( ctx context.Context, lhs, rhs split.LoadSplitSnapshot, -) (can bool, reason string) { +) (can bool, reason redact.RedactableString) { // When load is a consideration for splits and, by extension, merges, the // mergeQueue is fairly conservative. In an effort to avoid thrashing and to // avoid overreacting to temporary fluctuations in load, the mergeQueue will @@ -471,7 +471,7 @@ func canMergeRangeLoad( // just after changing the split objective to a different value, where // there is a mismatch. if lhs.SplitObjective != rhs.SplitObjective { - return false, fmt.Sprintf("LHS load measurement is a different type (%s) than the RHS (%s)", + return false, redact.Sprintf("LHS load measurement is a different type (%s) than the RHS (%s)", lhs.SplitObjective, rhs.SplitObjective, ) @@ -486,7 +486,7 @@ func canMergeRangeLoad( conservativeLoadBasedSplitThreshold := 0.5 * lhs.Threshold if merged >= conservativeLoadBasedSplitThreshold { - return false, fmt.Sprintf("lhs+rhs %s (%s+%s=%s) above threshold (%s)", + return false, redact.Sprintf("lhs+rhs %s (%s+%s=%s) above threshold (%s)", obj, obj.Format(lhs.Max), obj.Format(rhs.Max), @@ -495,7 +495,7 @@ func canMergeRangeLoad( ) } - return true, fmt.Sprintf("lhs+rhs %s (%s+%s=%s) below threshold (%s)", + return true, redact.Sprintf("lhs+rhs %s (%s+%s=%s) below threshold (%s)", obj, obj.Format(lhs.Max), obj.Format(rhs.Max), diff --git a/pkg/kv/kvserver/replica_command.go b/pkg/kv/kvserver/replica_command.go index 6f88af4da78d..75ea8ca228cf 100644 --- a/pkg/kv/kvserver/replica_command.go +++ b/pkg/kv/kvserver/replica_command.go @@ -595,7 +595,7 @@ func (r *Replica) executeAdminCommandWithDescriptor( // The supplied RangeDescriptor is used as a form of optimistic lock. See the // comment of "AdminSplit" for more information on this pattern. func (r *Replica) AdminMerge( - ctx context.Context, args kvpb.AdminMergeRequest, reason string, + ctx context.Context, args kvpb.AdminMergeRequest, reason redact.RedactableString, ) (kvpb.AdminMergeResponse, *kvpb.Error) { var reply kvpb.AdminMergeResponse