From ba2c2d7fe2c0d6f5520eacc6fa9e7a1c40da30da Mon Sep 17 00:00:00 2001
From: Erik Grinaker <grinaker@cockroachlabs.com>
Date: Sun, 19 Mar 2023 10:52:54 +0000
Subject: [PATCH] kvserver: remove
 `TestStoreMaxBehindNanosOnlyTracksEpochBasedLeases`

This test attempts to assert something about expiration leases and
closed timestamps that is no longer true: closed timestamps work just
fine with expiration leases.

Epic: none
Release note: None
---
 pkg/kv/kvserver/client_metrics_test.go | 61 --------------------------
 1 file changed, 61 deletions(-)

diff --git a/pkg/kv/kvserver/client_metrics_test.go b/pkg/kv/kvserver/client_metrics_test.go
index 975c1e419b64..7527ed2c9d73 100644
--- a/pkg/kv/kvserver/client_metrics_test.go
+++ b/pkg/kv/kvserver/client_metrics_test.go
@@ -16,7 +16,6 @@ import (
 	"strconv"
 	"sync"
 	"testing"
-	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/base"
 	"github.com/cockroachdb/cockroach/pkg/keys"
@@ -30,13 +29,11 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/testutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/skip"
-	"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/testcluster"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/metric"
-	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/errors"
 	"github.com/stretchr/testify/require"
 )
@@ -379,61 +376,3 @@ func TestStoreMetrics(t *testing.T) {
 	verifyStorageStats(t, tc.GetFirstStoreFromServer(t, 1))
 	verifyStorageStats(t, tc.GetFirstStoreFromServer(t, 2))
 }
-
-// TestStoreMaxBehindNanosOnlyTracksEpochBasedLeases ensures that the metric
-// ClosedTimestampMaxBehindNanos does not follow the start time of expiration
-// based leases. Expiration based leases don't publish closed timestamps.
-func TestStoreMaxBehindNanosOnlyTracksEpochBasedLeases(t *testing.T) {
-	defer leaktest.AfterTest(t)()
-	defer log.Scope(t).Close(t)
-
-	ctx := context.Background()
-	tc := testcluster.StartTestCluster(t, 3, base.TestClusterArgs{
-		ServerArgs: base.TestServerArgs{
-			// Set a long timeout so that no lease or liveness ever times out.
-			RaftConfig: base.RaftConfig{RaftElectionTimeoutTicks: 100},
-		},
-	})
-	defer tc.Stopper().Stop(ctx)
-	tdb := sqlutils.MakeSQLRunner(tc.ServerConn(0))
-	// We want to choose setting values such that this test doesn't take too long
-	// with the caveat that under extreme stress, we need to make sure that the
-	// subsystem remains live.
-	const closedTimestampDuration = 15 * time.Millisecond
-	tdb.Exec(t, "SET CLUSTER SETTING kv.closed_timestamp.target_duration = $1",
-		closedTimestampDuration.String())
-	tdb.Exec(t, "SET CLUSTER SETTING kv.closed_timestamp.side_transport_interval = $1",
-		closedTimestampDuration.String())
-
-	// Let's get to a point where we know that we have an expiration based lease
-	// with a start time more than some time ago and then we have a max closed
-	// value more recent.
-	_, meta2Repl1 := getFirstStoreReplica(t, tc.Server(0), keys.Meta2Prefix)
-
-	// Transfer the lease for the meta range to ensure that it has a non-zero
-	// start time.
-	require.NoError(t, tc.TransferRangeLease(*meta2Repl1.Desc(), tc.Target(1)))
-
-	testutils.SucceedsSoon(t, func() error {
-		_, metaRepl := getFirstStoreReplica(t, tc.Server(1), keys.Meta2Prefix)
-		l, _ := metaRepl.GetLease()
-		if l.Start.IsEmpty() {
-			return errors.Errorf("don't have a lease for meta1 yet: %v %v", l, meta2Repl1)
-		}
-		sinceExpBasedLeaseStart := timeutil.Since(timeutil.Unix(0, l.Start.WallTime))
-		for i := 0; i < tc.NumServers(); i++ {
-			s, _ := getFirstStoreReplica(t, tc.Server(i), keys.Meta1Prefix)
-			require.NoError(t, s.ComputeMetrics(ctx))
-			maxBehind := time.Duration(s.Metrics().ClosedTimestampMaxBehindNanos.Value())
-			// We want to make sure that maxBehind ends up being much smaller than the
-			// start of an expiration based lease.
-			const behindMultiple = 5
-			if maxBehind*behindMultiple > sinceExpBasedLeaseStart {
-				return errors.Errorf("store %v has a ClosedTimestampMaxBehindNanos"+
-					" of %v which is not way less than the an expiration-based lease start, %v",
-					s.StoreID(), maxBehind, sinceExpBasedLeaseStart)
-			}
-		}
-		return nil
-	})
-}