Skip to content

Commit

Permalink
kv: fix redaction in merge logging
Browse files Browse the repository at this point in the history
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
  • Loading branch information
nvanbenschoten committed Nov 9, 2023
1 parent 0ec3c69 commit b06849b
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 8 deletions.
14 changes: 7 additions & 7 deletions pkg/kv/kvserver/merge_queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ package kvserver

import (
"context"
"fmt"
"math"
"time"

Expand All @@ -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 (
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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()),
Expand Down Expand Up @@ -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
Expand All @@ -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,
)
Expand All @@ -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),
Expand All @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/replica_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit b06849b

Please sign in to comment.