kv: include locking strength and durability in Get/Scan/RevScan SafeF…

…ormat fixes: #114475 Release note: None.
cockroachdb · Nov 15, 2023 · 14fb0cb · 14fb0cb
1 parent c114b1d
commit 14fb0cb
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 3 deletions.
diff --git a/pkg/kv/kvclient/kvcoord/dist_sender_test.go b/pkg/kv/kvclient/kvcoord/dist_sender_test.go
@@ -4535,7 +4535,7 @@ func TestDistSenderSlowLogMessage(t *testing.T) {
 	br.Error = kvpb.NewError(errors.New("boom"))
 	desc := &roachpb.RangeDescriptor{RangeID: 9, StartKey: roachpb.RKey("x"), EndKey: roachpb.RKey("z")}
 	{
-		exp := `have been waiting 8.16s (120 attempts) for RPC Get [‹"a"›,/Min) to` +
+		exp := `have been waiting 8.16s (120 attempts) for RPC Get [‹"a"›,/Min)[lockStrength=None lockingDurability=Unreplicated] to` +
 			` r9:‹{x-z}› [<no replicas>, next=0, gen=0]; resp: ‹(err: boom)›`
 		var s redact.StringBuilder
 		slowRangeRPCWarningStr(&s, ba, dur, attempts, desc, nil /* err */, br)

diff --git a/pkg/kv/kvpb/api.go b/pkg/kv/kvpb/api.go
@@ -194,6 +194,33 @@ type Request interface {
 	flags() flag
 }
 
+// SafeFormatterRequest is an optional extension interface used to allow request to do custom formatting.
+type SafeFormatterRequest interface {
+	Request
+	redact.SafeFormatter
+}
+
+var _ SafeFormatterRequest = (*GetRequest)(nil)
+
+// SafeFormat implements the redact.SafeFormatter interface.
+func (gr GetRequest) SafeFormat(s redact.SafePrinter, _ rune) {
+	s.Printf("[lockStrength=%s lockingDurability=%s]", gr.KeyLockingStrength, gr.KeyLockingDurability)
+}
+
+var _ SafeFormatterRequest = (*ScanRequest)(nil)
+
+// SafeFormat implements the redact.SafeFormatter interface.
+func (sr ScanRequest) SafeFormat(s redact.SafePrinter, _ rune) {
+	s.Printf("[lockStrength=%s lockingDurability=%s]", sr.KeyLockingStrength, sr.KeyLockingDurability)
+}
+
+var _ SafeFormatterRequest = (*ReverseScanRequest)(nil)
+
+// SafeFormat implements the redact.SafeFormatter interface.
+func (rsr ReverseScanRequest) SafeFormat(s redact.SafePrinter, _ rune) {
+	s.Printf("[lockStrength=%s lockingDurability=%s]", rsr.KeyLockingStrength, rsr.KeyLockingDurability)
+}
+
 // LockingReadRequest is an interface used to expose the key-level locking
 // strength of a read-only request.
 type LockingReadRequest interface {

diff --git a/pkg/kv/kvpb/batch.go b/pkg/kv/kvpb/batch.go
@@ -795,7 +795,7 @@ func (ba BatchRequest) Split(canSplitET bool) [][]RequestUnion {
 
 // SafeFormat implements redact.SafeFormatter.
 // It gives a brief summary of the contained requests and keys in the batch.
-func (ba BatchRequest) SafeFormat(s redact.SafePrinter, _ rune) {
+func (ba BatchRequest) SafeFormat(s redact.SafePrinter, verb rune) {
 	for count, arg := range ba.Requests {
 		// Limit the strings to provide just a summary. Without this limit
 		// a log message with a BatchRequest can be very long.
@@ -845,6 +845,9 @@ func (ba BatchRequest) SafeFormat(s redact.SafePrinter, _ rune) {
 				s.Print(req.Method())
 			}
 			s.Printf(" [%s,%s)", h.Key, h.EndKey)
+			if safeFormatterReq, ok := req.(SafeFormatterRequest); ok {
+				safeFormatterReq.SafeFormat(s, verb)
+			}
 		}
 	}
 	{

diff --git a/pkg/kv/kvpb/string_test.go b/pkg/kv/kvpb/string_test.go
@@ -62,7 +62,7 @@ func TestBatchRequestString(t *testing.T) {
 	ba.Requests = append(ba.Requests, ru)
 
 	{
-		exp := `Get [/Min,/Min), Get [/Min,/Min), Get [/Min,/Min), Get [/Min,/Min), Get [/Min,/Min), Get [/Min,/Min), Get [/Min,/Min), Get [/Min,/Min), Get [/Min,/Min), Get [/Min,/Min), Get [/Min,/Min), Get [/Min,/Min), Get [/Min,/Min), Get [/Min,/Min), Get [/Min,/Min), Get [/Min,/Min), Get [/Min,/Min), Get [/Min,/Min), Get [/Min,/Min), Get [/Min,/Min),... 76 skipped ..., Get [/Min,/Min), Get [/Min,/Min), Get [/Min,/Min), Get [/Min,/Min), EndTxn(abort) [/Min], [txn: 6ba7b810], [wait-policy: Error], [protect-ambiguous-replay], [can-forward-ts], [bounded-staleness, min_ts_bound: 0.000000001,0, min_ts_bound_strict, max_ts_bound: 0.000000002,0]`
+		exp := `Get [/Min,/Min)[lockStrength=None lockingDurability=Unreplicated], Get [/Min,/Min)[lockStrength=None lockingDurability=Unreplicated], Get [/Min,/Min)[lockStrength=None lockingDurability=Unreplicated], Get [/Min,/Min)[lockStrength=None lockingDurability=Unreplicated], Get [/Min,/Min)[lockStrength=None lockingDurability=Unreplicated], Get [/Min,/Min)[lockStrength=None lockingDurability=Unreplicated], Get [/Min,/Min)[lockStrength=None lockingDurability=Unreplicated], Get [/Min,/Min)[lockStrength=None lockingDurability=Unreplicated], Get [/Min,/Min)[lockStrength=None lockingDurability=Unreplicated], Get [/Min,/Min)[lockStrength=None lockingDurability=Unreplicated], Get [/Min,/Min)[lockStrength=None lockingDurability=Unreplicated], Get [/Min,/Min)[lockStrength=None lockingDurability=Unreplicated], Get [/Min,/Min)[lockStrength=None lockingDurability=Unreplicated], Get [/Min,/Min)[lockStrength=None lockingDurability=Unreplicated], Get [/Min,/Min)[lockStrength=None lockingDurability=Unreplicated], Get [/Min,/Min)[lockStrength=None lockingDurability=Unreplicated], Get [/Min,/Min)[lockStrength=None lockingDurability=Unreplicated], Get [/Min,/Min)[lockStrength=None lockingDurability=Unreplicated], Get [/Min,/Min)[lockStrength=None lockingDurability=Unreplicated], Get [/Min,/Min)[lockStrength=None lockingDurability=Unreplicated],... 76 skipped ..., Get [/Min,/Min)[lockStrength=None lockingDurability=Unreplicated], Get [/Min,/Min)[lockStrength=None lockingDurability=Unreplicated], Get [/Min,/Min)[lockStrength=None lockingDurability=Unreplicated], Get [/Min,/Min)[lockStrength=None lockingDurability=Unreplicated], EndTxn(abort) [/Min], [txn: 6ba7b810], [wait-policy: Error], [protect-ambiguous-replay], [can-forward-ts], [bounded-staleness, min_ts_bound: 0.000000001,0, min_ts_bound_strict, max_ts_bound: 0.000000002,0]`
 		act := ba.String()
 		require.Equal(t, exp, act)
 	}