Merge #154640 #155155 #155168 #155172

154640: roachtest: add mma-and-count versions of allocbench r=wenyihu6 a=tbg

Fixes #154638.

Epic: CRDB-55052



155155: build: increase timeouts for microbenchmark CI run groups r=tbg,rickystewart a=herkolategan

We've been hitting these limits due to some minor changes in the underlying benchmarks in these groups. This ups the limit so the job doesn't time out.

Epic: None
Release note: None

155168: tests: disable lease queue in Benchmark{,Parallel}Sysbench r=tbg a=tbg

**tests: disable lease queue in Benchmark{,Parallel}Sysbench**

This keeps leases on s1, which reduces variance
in the benchmark.

---

**tests: also wait for full replication on sysbenchKV**

Only the SQL flavor was previously checking for full replication.

Epic: None

155172: kv: migrate off log.Dev r=tbg a=tbg

These are the last calls to `log.Dev`.

Closes #152584.
Epic: CRDB-52421

Co-authored-by: Tobias Grieger <[email protected]>
Co-authored-by: Herko Lategan <[email protected]>

Author: 3 people

.github/workflows/microbenchmarks-ci.yaml

@@ -47,7 +47,7 @@ jobs:
           pkg: ${{ env.PACKAGE }}
   run-group-1:
     runs-on: [self-hosted, ubuntu_2004_microbench]
-    timeout-minutes: 60
+    timeout-minutes: 90
     needs: [base, head]
     steps:
       - name: Checkout
@@ -60,7 +60,7 @@ jobs:
           group: 1
   run-group-2:
     runs-on: [self-hosted, ubuntu_2004_microbench]
-    timeout-minutes: 60
+    timeout-minutes: 90
     needs: [base, head]
     steps:
       - name: Checkout

pkg/cmd/roachtest/tests/allocation_bench.go

@@ -44,6 +44,7 @@ const (
 type allocationBenchSpec struct {
 	nodes, cpus int
 	load        allocBenchLoad
+	lbrMode     string // see kvserverbase.LoadBasedRebalancingMode
 
 	startRecord time.Duration
 	samples     int
@@ -172,7 +173,7 @@ func (r kvAllocBenchEventRunner) run(ctx context.Context, c cluster.Cluster, t t
 	return c.RunE(ctx, option.WithNodes(c.WorkloadNode()), runCmd)
 }
 func registerAllocationBench(r registry.Registry) {
-	for _, spec := range []allocationBenchSpec{
+	specTemplates := []allocationBenchSpec{
 		// TODO(kvoli): Add a background event runner and implement events for
 		// import and index backfills.
 		{
@@ -242,8 +243,19 @@ func registerAllocationBench(r registry.Registry) {
 				},
 			},
 		},
-	} {
-		registerAllocationBenchSpec(r, spec)
+	}
+	for _, spec := range specTemplates {
+		{
+			spec := spec
+			spec.lbrMode = "leases and replicas"
+			registerAllocationBenchSpec(r, spec)
+		}
+		{
+			spec := spec
+			spec.lbrMode = "multi-metric and count"
+			spec.load.desc += "/lbr=mmc"
+			registerAllocationBenchSpec(r, spec)
+		}
 	}
 }
 
@@ -260,7 +272,7 @@ func registerAllocationBenchSpec(r registry.Registry, allocSpec allocationBenchS
 		Timeout:           time.Duration(allocSpec.samples) * time.Hour,
 		NonReleaseBlocker: true,
 		CompatibleClouds:  registry.AllExceptAWS,
-		Suites:            registry.Suites(registry.Nightly),
+		Suites:            registry.Suites(registry.Weekly),
 		Run: func(ctx context.Context, t test.Test, c cluster.Cluster) {
 			runAllocationBench(ctx, t, c, allocSpec)
 		},
@@ -273,11 +285,18 @@ func setupAllocationBench(
 	t.Status("starting cluster")
 	for i := 1; i <= spec.nodes; i++ {
 		// Don't start a backup schedule as this test reports to roachperf.
+		settings := install.MakeClusterSettings()
+		settings.Env = append(settings.Env, "COCKROACH_ALLOW_MMA=true")
 		startOpts := option.NewStartOpts(option.NoBackupSchedule)
 		startOpts.RoachprodOpts.ExtraArgs = append(startOpts.RoachprodOpts.ExtraArgs,
 			"--vmodule=store_rebalancer=2,allocator=2,replicate_queue=2")
-		c.Start(ctx, t.L(), startOpts, install.MakeClusterSettings(), c.Node(i))
+		c.Start(ctx, t.L(), startOpts, settings, c.Node(i))
 	}
+	require.NotEmpty(t, spec.lbrMode, "lbrMode must be set")
+	_, err := c.Conn(ctx, t.L(), 1).ExecContext(ctx, fmt.Sprintf(
+		`SET CLUSTER SETTING kv.allocator.load_based_rebalancing = '%s';`,
+		spec.lbrMode))
+	require.NoError(t, err)
 
 	return setupStatCollector(ctx, t, c, spec)
 }

pkg/kv/client_test.go

@@ -176,7 +176,7 @@ func TestClientRetryNonTxn(t *testing.T) {
 					}
 					notify <- struct{}{}
 					if err != nil {
-						log.Dev.Errorf(context.Background(), "error on non-txn request: %s", err)
+						log.KvExec.Errorf(context.Background(), "error on non-txn request: %s", err)
 					}
 					doneCall <- errors.Wrapf(
 						err, "%d: expected success on non-txn call to %s",

pkg/kv/db.go

@@ -218,7 +218,7 @@ func (s *CrossRangeTxnWrapperSender) Send(
 	ctx context.Context, ba *kvpb.BatchRequest,
 ) (*kvpb.BatchResponse, *kvpb.Error) {
 	if ba.Txn != nil {
-		log.Dev.Fatalf(ctx, "CrossRangeTxnWrapperSender can't handle transactional requests")
+		log.KvExec.Fatalf(ctx, "CrossRangeTxnWrapperSender can't handle transactional requests")
 	}
 
 	br, pErr := s.wrapped.Send(ctx, ba)
@@ -1167,7 +1167,7 @@ func (db *DB) sendUsingSender(
 	br, pErr := sender.Send(ctx, ba)
 	if pErr != nil {
 		if log.V(1) {
-			log.Dev.Infof(ctx, "failed batch: %s", pErr)
+			log.KvExec.Infof(ctx, "failed batch: %s", pErr)
 		}
 		return nil, pErr
 	}

pkg/kv/range_lookup.go

@@ -249,13 +249,13 @@ func RangeLookup(
 		if rc == kvpb.INCONSISTENT {
 			return nil, nil, nil
 		}
-		log.Dev.Warningf(ctx, "range lookup of key %s found only non-matching ranges %v; retrying",
+		log.KvExec.Warningf(ctx, "range lookup of key %s found only non-matching ranges %v; retrying",
 			key, prefetchedRanges)
 	}
 
 	ctxErr := ctx.Err()
 	if ctxErr == nil {
-		log.Dev.Fatalf(ctx, "retry loop broke before context expired")
+		log.KvExec.Fatalf(ctx, "retry loop broke before context expired")
 	}
 	return nil, nil, ctxErr
 }
@@ -320,7 +320,7 @@ func lookupRangeFwdScan(
 		RequestHeader: kvpb.RequestHeaderFromSpan(bounds.AsRawSpanWithNoLocals()),
 	})
 	if !TestingIsRangeLookup(ba) {
-		log.Dev.Fatalf(ctx, "BatchRequest %v not detectable as RangeLookup", ba)
+		log.KvExec.Fatalf(ctx, "BatchRequest %v not detectable as RangeLookup", ba)
 	}
 
 	br, pErr := sender.Send(ctx, ba)
@@ -394,7 +394,7 @@ func lookupRangeRevScan(
 		RequestHeader: kvpb.RequestHeaderFromSpan(revBounds.AsRawSpanWithNoLocals()),
 	})
 	if !TestingIsRangeLookup(ba) {
-		log.Dev.Fatalf(ctx, "BatchRequest %v not detectable as RangeLookup", ba)
+		log.KvExec.Fatalf(ctx, "BatchRequest %v not detectable as RangeLookup", ba)
 	}
 
 	br, pErr := sender.Send(ctx, ba)

pkg/kv/txn.go

@@ -406,7 +406,7 @@ func (txn *Txn) TestingSetPriority(priority enginepb.TxnPriority) {
 	// non-randomized, priority for the transaction.
 	txn.mu.userPriority = roachpb.UserPriority(-priority)
 	if err := txn.mu.sender.SetUserPriority(txn.mu.userPriority); err != nil {
-		log.Dev.Fatalf(context.TODO(), "%+v", err)
+		log.KvExec.Fatalf(context.TODO(), "%+v", err)
 	}
 	txn.mu.Unlock()
 }
@@ -1056,7 +1056,7 @@ func (txn *Txn) rollback(ctx context.Context) *kvpb.Error {
 						// already committed. We don't spam the logs with those.
 						log.VEventf(ctx, 2, "async rollback failed: %s", pErr)
 					} else {
-						log.Dev.Infof(ctx, "async rollback failed: %s", pErr)
+						log.KvExec.Infof(ctx, "async rollback failed: %s", pErr)
 					}
 				}
 				return nil
@@ -1172,7 +1172,7 @@ func (txn *Txn) exec(ctx context.Context, fn func(context.Context, *Txn) error)
 					// We sent transactional requests, so the TxnCoordSender was supposed to
 					// turn retryable errors into TransactionRetryWithProtoRefreshError. Note that this
 					// applies only in the case where this is the root transaction.
-					log.Dev.Fatalf(ctx, "unexpected UnhandledRetryableError at the txn.exec() level: %s", err)
+					log.KvExec.Fatalf(ctx, "unexpected UnhandledRetryableError at the txn.exec() level: %s", err)
 				}
 			} else if t := (*kvpb.TransactionRetryWithProtoRefreshError)(nil); errors.As(err, &t) {
 				if txn.ID() != t.PrevTxnID {
@@ -1219,13 +1219,13 @@ func (txn *Txn) exec(ctx context.Context, fn func(context.Context, *Txn) error)
 					txn.DebugName(), attempt, err, maxRetries, rollbackErr,
 				),
 				ErrAutoRetryLimitExhausted)
-			log.Dev.Warningf(ctx, "%v", err)
+			log.KvExec.Warningf(ctx, "%v", err)
 			break
 		}
 
 		const warnEvery = 10
 		if attempt%warnEvery == 0 {
-			log.Dev.Warningf(ctx, "have retried transaction: %s %d times, most recently because of the "+
+			log.KvExec.Warningf(ctx, "have retried transaction: %s %d times, most recently because of the "+
 				"retryable error: %s. Is the transaction stuck in a retry loop?", txn.DebugName(), attempt, err)
 		}
 
@@ -1395,7 +1395,7 @@ func (txn *Txn) Send(
 		if requestTxnID != retryErr.PrevTxnID {
 			// KV should not return errors for transactions other than the one that sent
 			// the request.
-			log.Dev.Fatalf(ctx, "retryable error for the wrong txn. "+
+			log.KvExec.Fatalf(ctx, "retryable error for the wrong txn. "+
 				"requestTxnID: %s, retryErr.PrevTxnID: %s. retryErr: %s",
 				requestTxnID, retryErr.PrevTxnID, retryErr)
 		}
@@ -1629,7 +1629,7 @@ func (txn *Txn) UpdateStateOnRemoteRetryableErr(ctx context.Context, pErr *kvpb.
 	defer txn.mu.Unlock()
 
 	if pErr.TransactionRestart() == kvpb.TransactionRestart_NONE {
-		log.Dev.Fatalf(ctx, "unexpected non-retryable error: %s", pErr)
+		log.KvExec.Fatalf(ctx, "unexpected non-retryable error: %s", pErr)
 	}
 
 	// If the transaction has been reset since this request was sent,

pkg/kv/txn_test.go

@@ -432,7 +432,7 @@ func TestWrongTxnRetry(t *testing.T) {
 
 	var retries int
 	txnClosure := func(ctx context.Context, outerTxn *Txn) error {
-		log.Dev.Infof(ctx, "outer retry")
+		log.KvExec.Infof(ctx, "outer retry")
 		retries++
 		// Ensure the KV transaction is created.
 		if err := outerTxn.Put(ctx, "a", "b"); err != nil {

pkg/sql/tests/sysbench_test.go

@@ -26,6 +26,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/keys"
 	"github.com/cockroachdb/cockroach/pkg/kv"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
+	"github.com/cockroachdb/cockroach/pkg/kv/kvserver"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/rpc"
 	"github.com/cockroachdb/cockroach/pkg/rpc/nodedialer"
@@ -175,14 +176,29 @@ func newTestCluster(
 				NoLoopbackDialer: !localRPCFastPath,
 			},
 		},
+		Store: &kvserver.StoreTestingKnobs{
+			// Disable the lease queue to keep leases on s1 (otherwise, lease
+			// count rebalancing might move one lease, and splits might copy
+			// that lease to a number of additional ranges). Communication
+			// between the gateway node (always n1) and the KV servers always
+			// goes through TCP regardless of whether the gateway node equals
+			// the KV node, but there are still subtle (and not well understood)
+			// performance differences between then n1->n1 and n1->n[23] cases,
+			// which add variance to the results.
+			DisableLeaseQueue: true,
+		},
 	}
-	return serverutils.StartCluster(b, nodes, base.TestClusterArgs{
+	tc := serverutils.StartCluster(b, nodes, base.TestClusterArgs{
 		ServerArgs: base.TestServerArgs{
 			Settings:  st,
 			CacheSize: cacheSize,
 			Knobs:     knobs,
 		}},
 	)
+	if nodes > 1 {
+		try0(tc.WaitForFullReplication())
+	}
+	return tc
 }
 
 // sysbenchSQL is SQL-based implementation of sysbenchDriver. It runs SQL
@@ -199,7 +215,6 @@ func newSysbenchSQL(nodes int, localRPCFastPath bool) sysbenchDriverConstructor
 		for i := 0; i < nodes; i++ {
 			tc.Server(i).SQLServer().(*sql.Server).GetExecutorConfig().LicenseEnforcer.Disable(ctx)
 		}
-		try0(tc.WaitForFullReplication())
 		pgURL, cleanupURL := tc.ApplicationLayer(0).PGUrl(b, serverutils.DBName(sysbenchDB))
 		cleanup := func() {
 			cleanupURL()

scripts/trigger-pr-roachtest.sh

@@ -42,6 +42,9 @@ dump_json() {
   fi
 }
 
+# NB: if you're trying to trigger a weekly roachtest, need to use
+# 'Cockroach_Nightlies_RoachtestWeeklyBazel' as the buildType below.
+
 json_payload=$(jq -n \
   --arg branch_name "$pr" \
   --arg tests "$tests" \