From b1253a8ba6c7c25b907bab9a92ea8d7078dfc1cc Mon Sep 17 00:00:00 2001 From: Pavel Kalinnikov Date: Wed, 18 Dec 2024 19:02:19 +0000 Subject: [PATCH 1/4] kvflowcontrolpb: fix SafeFormat redaction Epic: none Release note: none --- pkg/util/admission/admissionpb/admissionpb.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/util/admission/admissionpb/admissionpb.go b/pkg/util/admission/admissionpb/admissionpb.go index e70576248656..3985a6ea4df6 100644 --- a/pkg/util/admission/admissionpb/admissionpb.go +++ b/pkg/util/admission/admissionpb/admissionpb.go @@ -52,7 +52,7 @@ func (w WorkPriority) String() string { // SafeFormat implements the redact.SafeFormatter interface. func (w WorkPriority) SafeFormat(p redact.SafePrinter, verb rune) { if s, ok := WorkPriorityDict[w]; ok { - p.Print(s) + p.SafeString(redact.SafeString(s)) return } p.Printf("custom-pri=%d", int8(w)) @@ -232,11 +232,11 @@ func (w WorkClass) String() string { func (w WorkClass) SafeFormat(p redact.SafePrinter, verb rune) { switch w { case RegularWorkClass: - p.Printf("regular") + p.SafeString("regular") case ElasticWorkClass: - p.Printf("elastic") + p.SafeString("elastic") default: - p.Printf("") + p.SafeString("") } } From 9cb9ca8b8e58cd16b9f25eb2de78b9d91641715d Mon Sep 17 00:00:00 2001 From: Pavel Kalinnikov Date: Thu, 19 Dec 2024 10:43:39 +0000 Subject: [PATCH 2/4] rac2: make AdmittedVector printing redaction-safe Epic: none Release note: none --- pkg/kv/kvserver/kvflowcontrol/rac2/log_tracker.go | 6 +++--- pkg/raft/raftpb/raft.go | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/kv/kvserver/kvflowcontrol/rac2/log_tracker.go b/pkg/kv/kvserver/kvflowcontrol/rac2/log_tracker.go index 40c11181b408..27fa3054fbf9 100644 --- a/pkg/kv/kvserver/kvflowcontrol/rac2/log_tracker.go +++ b/pkg/kv/kvserver/kvflowcontrol/rac2/log_tracker.go @@ -57,12 +57,12 @@ func (av AdmittedVector) SafeFormat(w redact.SafePrinter, _ rune) { buf.Printf("term:%d, admitted:[", av.Term) for pri, index := range av.Admitted { if pri > 0 { - buf.Printf(",") + buf.SafeRune(',') } buf.Printf("%s:%d", raftpb.Priority(pri), index) } - buf.Printf("]") - w.Printf("%v", buf) + buf.SafeRune(']') + w.SafeString(redact.SafeString(buf.String())) } // LogTracker tracks the durable and logically admitted state of a raft log. diff --git a/pkg/raft/raftpb/raft.go b/pkg/raft/raftpb/raft.go index 2169809f9339..019b97d60335 100644 --- a/pkg/raft/raftpb/raft.go +++ b/pkg/raft/raftpb/raft.go @@ -75,13 +75,13 @@ func (p Priority) String() string { func (p Priority) SafeFormat(w redact.SafePrinter, _ rune) { switch p { case LowPri: - w.Printf("LowPri") + w.SafeString("LowPri") case NormalPri: - w.Printf("NormalPri") + w.SafeString("NormalPri") case AboveNormalPri: - w.Printf("AboveNormalPri") + w.SafeString("AboveNormalPri") case HighPri: - w.Printf("HighPri") + w.SafeString("HighPri") default: panic("invalid raft priority") } From 6febaab021d49b3868130a81773bfcf1b2bd0501 Mon Sep 17 00:00:00 2001 From: Pavel Kalinnikov Date: Thu, 19 Dec 2024 12:38:14 +0000 Subject: [PATCH 3/4] kvflowcontrolpb: fix admitted vector redaction Strings are redactable by default. Remove the String() invocation so that the Admitted field is printed using its SafeFormat method and marked as safe. Epic: none Release note: none --- pkg/kv/kvserver/kvflowcontrol/kvflowcontrolpb/kvflowcontrol.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/kv/kvserver/kvflowcontrol/kvflowcontrolpb/kvflowcontrol.go b/pkg/kv/kvserver/kvflowcontrol/kvflowcontrolpb/kvflowcontrol.go index c5c02592e033..2ddbd4ce68e9 100644 --- a/pkg/kv/kvserver/kvflowcontrol/kvflowcontrolpb/kvflowcontrol.go +++ b/pkg/kv/kvserver/kvflowcontrol/kvflowcontrolpb/kvflowcontrol.go @@ -71,5 +71,5 @@ func (a PiggybackedAdmittedState) String() string { func (a PiggybackedAdmittedState) SafeFormat(w redact.SafePrinter, _ rune) { w.Printf("[r%s,s%s,%d->%d] %s", - a.RangeID, a.ToStoreID, a.FromReplicaID, a.ToReplicaID, a.Admitted.String()) + a.RangeID, a.ToStoreID, a.FromReplicaID, a.ToReplicaID, a.Admitted) } From acb087d23472f0f8d88bd0b342aa42c83975e068 Mon Sep 17 00:00:00 2001 From: Pavel Kalinnikov Date: Thu, 19 Dec 2024 12:40:46 +0000 Subject: [PATCH 4/4] kvflowcontrolpb: rm unused type Epic: none Release note: none --- .../kvflowcontrolpb/kvflowcontrol.go | 8 ------- .../kvflowcontrolpb/kvflowcontrol.proto | 22 ------------------- 2 files changed, 30 deletions(-) diff --git a/pkg/kv/kvserver/kvflowcontrol/kvflowcontrolpb/kvflowcontrol.go b/pkg/kv/kvserver/kvflowcontrol/kvflowcontrolpb/kvflowcontrol.go index 2ddbd4ce68e9..e316f8a771f9 100644 --- a/pkg/kv/kvserver/kvflowcontrol/kvflowcontrolpb/kvflowcontrol.go +++ b/pkg/kv/kvserver/kvflowcontrol/kvflowcontrolpb/kvflowcontrol.go @@ -49,14 +49,6 @@ func (a AdmittedRaftLogEntries) SafeFormat(w redact.SafePrinter, _ rune) { a.RangeID, a.StoreID, admissionpb.WorkPriority(a.AdmissionPriority), a.UpToRaftLogPosition) } -func (a AdmittedResponseForRange) String() string { - return redact.StringWithoutMarkers(a) -} - -func (a AdmittedResponseForRange) SafeFormat(w redact.SafePrinter, _ rune) { - w.Printf("admitted-response (s%s r%s %s)", a.LeaderStoreID, a.RangeID, a.Msg.String()) -} - func (a AdmittedState) String() string { return redact.StringWithoutMarkers(a) } diff --git a/pkg/kv/kvserver/kvflowcontrol/kvflowcontrolpb/kvflowcontrol.proto b/pkg/kv/kvserver/kvflowcontrol/kvflowcontrolpb/kvflowcontrol.proto index aac139ec313c..cec7ac1f58b9 100644 --- a/pkg/kv/kvserver/kvflowcontrol/kvflowcontrolpb/kvflowcontrol.proto +++ b/pkg/kv/kvserver/kvflowcontrol/kvflowcontrolpb/kvflowcontrol.proto @@ -125,28 +125,6 @@ message RaftLogPosition { uint64 index = 2; } -// AdmittedResponseForRange is only used in RACv2. It contains a MsgAppResp -// from a follower to a leader, that was generated to advance the admitted -// vector for that follower, maintained by the leader. -// -// TODO(pav-kv): remove this type and use PiggybackedAdmittedState. -message AdmittedResponseForRange { - option (gogoproto.goproto_stringer) = false; - - // LeaderStoreID is used to route the request when this message is received - // at the leader node. - uint64 leader_store_id = 1 [(gogoproto.customname) = "LeaderStoreID", - (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/roachpb.StoreID"]; - - // RangeID of the raft group to which the MsgAppResp is directed. Used for - // routing at the leader node. - int64 range_id = 2 [(gogoproto.customname) = "RangeID", - (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/roachpb.RangeID"]; - - // Msg is the MsgAppResp containing the admitted vector. - raftpb.Message msg = 3 [(gogoproto.nullable) = false]; -} - // AdmittedState communicates a replica's vector of admitted log indices at // different priorities to the leader of a range. //