Skip to content

Commit

Permalink
kv: deflake TestTenantRateLimiter
Browse files Browse the repository at this point in the history
A recent change (cockroachdb#70328) increased the rate limit and that exposed a
fragility in this test. This change restores the test to use the
original (small) rate limit.

Fixes cockroachdb#70456.

Release note: None

Release justification: test-only fix.
  • Loading branch information
RaduBerinde committed Sep 24, 2021
1 parent e4e9a78 commit c37f179
Showing 1 changed file with 10 additions and 5 deletions.
15 changes: 10 additions & 5 deletions pkg/kv/kvserver/client_tenant_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
"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/util/encoding"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
Expand Down Expand Up @@ -143,7 +142,6 @@ func TestTenantsStorageMetricsOnSplit(t *testing.T) {
// and report the correct metrics.
func TestTenantRateLimiter(t *testing.T) {
defer leaktest.AfterTest(t)()
skip.WithIssue(t, 70456, "flaky test")
defer log.Scope(t).Close(t)

// This test utilizes manual time to make the rate-limiting calculations more
Expand All @@ -164,6 +162,10 @@ func TestTenantRateLimiter(t *testing.T) {
ctx := context.Background()
defer s.Stopper().Stop(ctx)

// Set a small rate limit so the test doesn't take a long time.
runner := sqlutils.MakeSQLRunner(sqlDB)
runner.Exec(t, `SET CLUSTER SETTING kv.tenant_rate_limiter.rate_limit = 200`)

tenantID := serverutils.TestTenantID()
codec := keys.MakeSQLCodec(tenantID)

Expand Down Expand Up @@ -224,8 +226,7 @@ func TestTenantRateLimiter(t *testing.T) {

// Create some tooling to read and verify metrics off of the prometheus
// endpoint.
sqlutils.MakeSQLRunner(sqlDB).Exec(t,
`SET CLUSTER SETTING server.child_metrics.enabled = true`)
runner.Exec(t, `SET CLUSTER SETTING server.child_metrics.enabled = true`)
httpClient, err := s.GetHTTPClient()
require.NoError(t, err)
getMetrics := func() string {
Expand All @@ -247,5 +248,9 @@ func TestTenantRateLimiter(t *testing.T) {

// Ensure that the metric for the admitted requests reflects the number of
// admitted requests.
require.Contains(t, getMetrics(), makeMetricStr(int64(tooManyWrites)))
// TODO(radu): this is fragile because a background write could sneak in and
// the count wouldn't match exactly.
m := getMetrics()
exp := makeMetricStr(int64(tooManyWrites))
require.Contains(t, m, exp, "could not find %s in metrics: \n%s\n", exp, m)
}

0 comments on commit c37f179

Please sign in to comment.