Merge cockroachdb#101012 cockroachdb#101226

101012: kvserver: deflake `TestLeaseRenewer` r=erikgrinaker a=erikgrinaker

**kvserver: fix lease renewal registration of expired leases**

The store lease renewer begins tracking an system range's expiration-based lease for renewal when it's first applied. However, it only did this if the lease was still valid when first applied. It was possible for the node to apply a lease after it's lapsed, but then to retain and extend the lease, and in this case it would not be registered with the lease renewer.

Epic: none
Release note: None

**kvserver: deflake `TestLeaseRenewer`**

Resolves cockroachdb#100689.

Epic: none
Release note: None

101226: ui: fix filter for database r=maryliag a=maryliag

A recent change made the format of the databases change on the UI and the filter no longer work.
This commit changes the display, to use the format: `db1, db2, db3`, instead of `["db1", "db2", "db3"]`. It also fixes the filter for database, now checking for both cases (database as a single value in a string and when is an array in a string form).

Before
https://www.loom.com/share/885465d973d647cd8e66040899ab38b0

After
https://www.loom.com/share/b04bd95fa82c4e5d93ae34e76b9f0fc1

Epic: None
Release note: None

Co-authored-by: Erik Grinaker <[email protected]>
Co-authored-by: maryliag <[email protected]>

pkg/kv/kvserver/replica_lease_renewal_test.go

@@ -36,7 +36,10 @@ func TestLeaseRenewer(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
-	skip.WithIssue(t, 100689)
+	// stressrace and deadlock make the test too slow, resulting in an inability
+	// to maintain leases and Raft leadership.
+	skip.UnderStressRace(t)
+	skip.UnderDeadlock(t)
 
 	// When kv.expiration_leases_only.enabled is true, the Raft scheduler is
 	// responsible for extensions, but we still track expiration leases for system
@@ -48,9 +51,18 @@ func TestLeaseRenewer(t *testing.T) {
 		tc := serverutils.StartNewTestCluster(t, 3, base.TestClusterArgs{
 			ServerArgs: base.TestServerArgs{
 				Settings: st,
+				// Speed up lease extensions to speed up the test, but adjust tick-based
+				// timeouts to retain their default wall-time values.
 				RaftConfig: base.RaftConfig{
-					RangeLeaseDuration: time.Second,
-					RaftTickInterval:   100 * time.Millisecond,
+					RangeLeaseRenewalFraction:  0.95,
+					RaftTickInterval:           100 * time.Millisecond,
+					RaftElectionTimeoutTicks:   20,
+					RaftReproposalTimeoutTicks: 30,
+				},
+				Knobs: base.TestingKnobs{
+					Store: &StoreTestingKnobs{
+						LeaseRenewalDurationOverride: 100 * time.Millisecond,
+					},
 				},
 			},
 		})
@@ -173,7 +185,8 @@ func TestLeaseRenewer(t *testing.T) {
 		assertStoreLeaseRenewer(desc.RangeID)
 
 		// Transfer the lease to a different leaseholder, and assert that the lease is
-		// still extended.
+		// still extended. Wait for the split to apply on all nodes first.
+		require.NoError(t, tc.WaitForFullReplication())
 		lease, _ := getNodeReplica(1, desc.RangeID).GetLease()
 		target := tc.Target(lookupNode(lease.Replica.NodeID%3 + 1))
 		tc.TransferRangeLeaseOrFatal(t, desc, target)

pkg/kv/kvserver/replica_proposal.go

@@ -372,8 +372,8 @@ func (r *Replica) leasePostApplyLocked(
 		r.gossipFirstRangeLocked(ctx)
 	}
 
-	if newLease.Type() == roachpb.LeaseExpiration && (leaseChangingHands || maybeSplit) &&
-		iAmTheLeaseHolder && r.ownsValidLeaseRLocked(ctx, now) {
+	isExpirationLease := newLease.Type() == roachpb.LeaseExpiration
+	if isExpirationLease && (leaseChangingHands || maybeSplit) && iAmTheLeaseHolder {
 		if r.requiresExpirationLeaseRLocked() {
 			// Whenever we first acquire an expiration-based lease for a range that
 			// requires it (i.e. the liveness or meta ranges), notify the lease
@@ -388,7 +388,7 @@ func (r *Replica) leasePostApplyLocked(
 			case r.store.renewableLeasesSignal <- struct{}{}:
 			default:
 			}
-		} else if !r.shouldUseExpirationLeaseRLocked() {
+		} else if !r.shouldUseExpirationLeaseRLocked() && r.ownsValidLeaseRLocked(ctx, now) {
 			// We received an expiration lease for a range that shouldn't keep using
 			// it, most likely as part of a lease transfer (which is always
 			// expiration-based). We've also applied it before it has expired. Upgrade

pkg/ui/workspaces/cluster-ui/src/sqlActivity/util.tsx

@@ -43,10 +43,18 @@ export function filteredStatementsData(
   // Current filters: search text, database, fullScan, service latency,
   // SQL Type, nodes and regions.
   return statements
-    .filter(
-      statement =>
-        databases.length == 0 || databases.includes(statement.database),
-    )
+    .filter(statement => {
+      try {
+        // Case where the database is returned as an array in a string form.
+        const dbList = JSON.parse(statement.database);
+        return (
+          databases.length === 0 || databases.some(d => dbList.includes(d))
+        );
+      } catch (e) {
+        // Case where the database is a single value as a string.
+        return databases.length === 0 || databases.includes(statement.database);
+      }
+    })
     .filter(statement => (filters.fullScan ? statement.fullScan : true))
     .filter(
       statement =>

pkg/ui/workspaces/cluster-ui/src/statementsTable/statementsTable.tsx

@@ -101,6 +101,16 @@ export function shortStatement(
   }
 }
 
+function formatStringArray(databases: string): string {
+  try {
+    // Case where the database is returned as an array in a string form.
+    return JSON.parse(databases).join(", ");
+  } catch (e) {
+    // Case where the database is a single value as a string.
+    return databases;
+  }
+}
+
 export function makeStatementsColumns(
   statements: AggregateStatistics[],
   selectedApps: string[],
@@ -173,7 +183,7 @@ export function makeStatementsColumns(
       name: "database",
       title: statisticsTableTitles.database(statType),
       className: cx("statements-table__col-database"),
-      cell: (stmt: AggregateStatistics) => stmt.database,
+      cell: (stmt: AggregateStatistics) => formatStringArray(stmt.database),
       sort: (stmt: AggregateStatistics) => stmt.database,
       showByDefault: false,
     },