Merge #100433

100433: batcheval: don't declare latches for `RequestLease` r=erikgrinaker a=erikgrinaker `RequestLease` does not take out latches. However, it still declared latches to appease the spanset asserter. This has a non-negligible cost with eagerly extended expiration leases, about 1.4% of CPU usage with 20.000 idle ranges. This patch removes latch declaration and disables spanset assertions. Touches #98433. Epic: none Release note: None Co-authored-by: Erik Grinaker <[email protected]>
cockroachdb · Apr 29, 2023 · 2c9bdbb · 2c9bdbb
2 parents 4619767 + 96df7f8
commit 2c9bdbb
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 8 deletions.
diff --git a/pkg/kv/kvserver/batcheval/cmd_lease_request.go b/pkg/kv/kvserver/batcheval/cmd_lease_request.go
@@ -14,7 +14,6 @@ import (
 	"context"
 	"time"
 
-	"github.com/cockroachdb/cockroach/pkg/keys"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/batcheval/result"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/readsummary/rspb"
@@ -36,12 +35,9 @@ func declareKeysRequestLease(
 ) {
 	// NOTE: RequestLease is run on replicas that do not hold the lease, so
 	// acquiring latches would not help synchronize with other requests. As
-	// such, the request does not actually acquire latches over these spans
-	// (see concurrency.shouldAcquireLatches). However, we continue to
-	// declare the keys in order to appease SpanSet assertions under race.
-	latchSpans.AddNonMVCC(spanset.SpanReadWrite, roachpb.Span{Key: keys.RangeLeaseKey(rs.GetRangeID())})
-	latchSpans.AddNonMVCC(spanset.SpanReadWrite, roachpb.Span{Key: keys.RangePriorReadSummaryKey(rs.GetRangeID())})
-	latchSpans.AddNonMVCC(spanset.SpanReadOnly, roachpb.Span{Key: keys.RangeDescriptorKey(rs.GetStartKey())})
+	// such, the request does not declare latches. See also
+	// concurrency.shouldIgnoreLatches().
+	latchSpans.DisableUndeclaredAccessAssertions()
 }
 
 // RequestLease sets the range lease for this range. The command fails

diff --git a/pkg/kv/kvserver/batcheval/declare_test.go b/pkg/kv/kvserver/batcheval/declare_test.go
@@ -37,10 +37,15 @@ func TestRequestsSerializeWithAllKeys(t *testing.T) {
 			continue
 		}
 		method := kvpb.Method(i)
-		if method == kvpb.Probe {
+		switch method {
+		case kvpb.Probe:
 			// Probe is special since it's a no-op round-trip through the replication
 			// layer. It does not declare any keys.
 			continue
+		case kvpb.RequestLease:
+			// Lease requests ignore latches, since they can be evaluated on
+			// any replica.
+			continue
 		}
 		t.Run(method.String(), func(t *testing.T) {
 			var otherLatchSpans, otherLockSpans spanset.SpanSet