Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
98972: kvserver: remove `TestStoreMaxBehindNanosOnlyTracksEpochBasedLeases` r=erikgrinaker a=erikgrinaker

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.

Resolves cockroachdb#98968.

Epic: none
Release note: None

Co-authored-by: Erik Grinaker <[email protected]>
  • Loading branch information
craig[bot] and erikgrinaker committed Mar 20, 2023
2 parents de5b08a + ba2c2d7 commit 6839729
Showing 1 changed file with 0 additions and 61 deletions.
61 changes: 0 additions & 61 deletions pkg/kv/kvserver/client_metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
"strconv"
"sync"
"testing"
"time"

"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/keys"
Expand All @@ -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"
)
Expand Down Expand Up @@ -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
})
}

0 comments on commit 6839729

Please sign in to comment.