sql/catalog/lease: decrease lock contention

This committ takes two steps to decrease lock contention. Firstly, it makes
the name cache data structure concurrent for readers with a RWLock. This
goes a pretty long way to diminish contention on the acquisition path. The
second change is to reduce the contention footprint when mucking with the
refcounts by doing less work in the common case.

Benchmark `kv95/nodes=1/cpu=32` delta over previous commit:
```
name             old ops/s   new ops/s   delta
KV95-throughput  96.4k ± 0%  99.6k ± 1%  +3.36%  (p=0.008 n=5+5)

name             old ms/s    new ms/s    delta
KV95-P50          1.40 ± 0%   1.40 ± 0%    ~     (all equal)
KV95-Avg          0.50 ± 0%   0.50 ± 0%    ~     (all equal)
```

Release note: None

pkg/sql/catalog/lease/descriptor_state.go

@@ -94,6 +94,7 @@ type descriptorState struct {
 func (t *descriptorState) findForTimestamp(
 	ctx context.Context, timestamp hlc.Timestamp,
 ) (*descriptorVersionState, bool, error) {
+	expensiveLogEnabled := log.ExpensiveLogEnabled(ctx, 2)
 	t.mu.Lock()
 	defer t.mu.Unlock()
 
@@ -109,7 +110,7 @@ func (t *descriptorState) findForTimestamp(
 			latest := i+1 == len(t.mu.active.data)
 			if !desc.hasExpired(timestamp) {
 				// Existing valid descriptor version.
-				desc.incRefCount(ctx)
+				desc.incRefCount(ctx, expensiveLogEnabled)
 				return desc, latest, nil
 			}
 
@@ -220,29 +221,37 @@ func (t *descriptorState) release(ctx context.Context, s *descriptorVersionState
 
 	// Decrements the refcount and returns true if the lease has to be removed
 	// from the store.
-	decRefcount := func(s *descriptorVersionState) *storedLease {
+	expensiveLoggingEnabled := log.ExpensiveLogEnabled(ctx, 2)
+	decRefCount := func(s *descriptorVersionState) (shouldRemove bool) {
+		s.mu.Lock()
+		defer s.mu.Unlock()
+		s.mu.refcount--
+		if expensiveLoggingEnabled {
+			log.Infof(ctx, "release: %s", s.stringLocked())
+		}
+		return s.mu.refcount == 0
+	}
+	maybeMarkRemoveStoredLease := func(s *descriptorVersionState) *storedLease {
 		// Figure out if we'd like to remove the lease from the store asap (i.e.
 		// when the refcount drops to 0). If so, we'll need to mark the lease as
 		// invalid.
-		removeOnceDereferenced := t.m.removeOnceDereferenced() ||
+		removeOnceDereferenced :=
 			// Release from the store if the descriptor has been dropped or taken
 			// offline.
 			t.mu.takenOffline ||
-			// Release from the store if the lease is not for the latest
-			// version; only leases for the latest version can be acquired.
-			s != t.mu.active.findNewest() ||
-			s.GetVersion() < t.mu.maxVersionSeen
-
+				// Release from the store if the lease is not for the latest
+				// version; only leases for the latest version can be acquired.
+				s != t.mu.active.findNewest() ||
+				s.GetVersion() < t.mu.maxVersionSeen ||
+				t.m.removeOnceDereferenced()
+		if !removeOnceDereferenced {
+			return nil
+		}
 		s.mu.Lock()
 		defer s.mu.Unlock()
-		s.mu.refcount--
-		if log.ExpensiveLogEnabled(ctx, 2) {
-			log.Infof(ctx, "release: %s", s.stringLocked())
-		}
 		if s.mu.refcount < 0 {
 			panic(errors.AssertionFailedf("negative ref count: %s", s))
 		}
-
 		if s.mu.refcount == 0 && s.mu.lease != nil && removeOnceDereferenced {
 			l := s.mu.lease
 			s.mu.lease = nil
@@ -251,9 +260,12 @@ func (t *descriptorState) release(ctx context.Context, s *descriptorVersionState
 		return nil
 	}
 	maybeRemoveLease := func() *storedLease {
+		if shouldRemove := decRefCount(s); !shouldRemove {
+			return nil
+		}
 		t.mu.Lock()
 		defer t.mu.Unlock()
-		if l := decRefcount(s); l != nil {
+		if l := maybeMarkRemoveStoredLease(s); l != nil {
 			t.mu.active.remove(s)
 			return l
 		}

pkg/sql/catalog/lease/descriptor_version_state.go

@@ -77,9 +77,7 @@ func (s *descriptorVersionState) Underlying() catalog.Descriptor {
 }
 
 func (s *descriptorVersionState) Expiration() hlc.Timestamp {
-	s.mu.Lock()
-	defer s.mu.Unlock()
-	return s.mu.expiration
+	return s.getExpiration()
 }
 
 func (s *descriptorVersionState) SafeMessage() string {
@@ -113,15 +111,15 @@ func (s *descriptorVersionState) hasExpiredLocked(timestamp hlc.Timestamp) bool
 	return s.mu.expiration.LessEq(timestamp)
 }
 
-func (s *descriptorVersionState) incRefCount(ctx context.Context) {
+func (s *descriptorVersionState) incRefCount(ctx context.Context, expensiveLogEnabled bool) {
 	s.mu.Lock()
 	defer s.mu.Unlock()
-	s.incRefCountLocked(ctx)
+	s.incRefCountLocked(ctx, expensiveLogEnabled)
 }
 
-func (s *descriptorVersionState) incRefCountLocked(ctx context.Context) {
+func (s *descriptorVersionState) incRefCountLocked(ctx context.Context, expensiveLogEnabled bool) {
 	s.mu.refcount++
-	if log.ExpensiveLogEnabled(ctx, 2) {
+	if expensiveLogEnabled {
 		log.VEventf(ctx, 2, "descriptorVersionState.incRefCount: %s", s.stringLocked())
 	}
 }

pkg/sql/catalog/lease/lease.go

@@ -666,9 +666,9 @@ func NewLeaseManager(
 func NameMatchesDescriptor(
 	desc catalog.Descriptor, parentID descpb.ID, parentSchemaID descpb.ID, name string,
 ) bool {
-	return desc.GetParentID() == parentID &&
-		desc.GetParentSchemaID() == parentSchemaID &&
-		desc.GetName() == name
+	return desc.GetName() == name &&
+		desc.GetParentID() == parentID &&
+		desc.GetParentSchemaID() == parentSchemaID
 }
 
 // findNewest returns the newest descriptor version state for the ID.

pkg/sql/catalog/lease/name_cache.go

@@ -16,6 +16,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/nstree"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
+	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
 	"github.com/cockroachdb/errors"
 )
@@ -29,7 +30,7 @@ func makeNameCache() nameCache {
 // from the store. The cache maintains the latest version for each name.
 // All methods are thread-safe.
 type nameCache struct {
-	mu          syncutil.Mutex
+	mu          syncutil.RWMutex
 	descriptors nstree.Map
 }
 
@@ -47,14 +48,15 @@ func (c *nameCache) get(
 	name string,
 	timestamp hlc.Timestamp,
 ) *descriptorVersionState {
-	c.mu.Lock()
+	c.mu.RLock()
 	desc, ok := c.descriptors.GetByName(
 		parentID, parentSchemaID, name,
 	).(*descriptorVersionState)
-	c.mu.Unlock()
+	c.mu.RUnlock()
 	if !ok {
 		return nil
 	}
+	expensiveLogEnabled := log.ExpensiveLogEnabled(ctx, 2)
 	desc.mu.Lock()
 	if desc.mu.lease == nil {
 		desc.mu.Unlock()
@@ -80,7 +82,7 @@ func (c *nameCache) get(
 		return nil
 	}
 
-	desc.incRefCountLocked(ctx)
+	desc.incRefCountLocked(ctx, expensiveLogEnabled)
 	return desc
 }