Merge #108802 #109160 #109177 #109236 #109239

108802: roachtest/cdc/mixed-versions: use mixed version framework r=renatolabs a=jayshrivastava

This change updates the `cdc/mixed-versions` roachtest to use the mixed version testing framework. This mixed version testing framework is better than the previous framework because it offers testing for multiple upgrades. It will also make it easier to maintain and expand this cdc-specific test.

Informs: #107451
Epic: None
Release note: None

109160: roachtest: simplify address functions r=renatolabs,smg260 a=herkolategan

Combine the duplicated logic for internal and external address functions in cluster into one function. This prevents changing one and forgetting to update the other.

Epic: None
Release note: None

109177: concurrency: misc cleanup related to multiple transactions holding locks on a single key r=nvanbenschoten a=arulajmani

Cleanup following-on from #109049. See individual commits for details.



109236: enginepb,concurrency: clean up {,the usage of} TxnSeqIsIgnored r=nvanbenschoten a=arulajmani

Prior to this patch, we were rolling out bespoke logic to determine whether a sequence number was ignored or not when acquiring/updating locks in the lock table. This patch switches that usage to call into enginepb.TxnSeqIsIgnored instead.

While here, we also change the implementation of TxnSeqIsIgnored to use a binary search and update some commentary.

Epic: none

Release note: None

109239: authserver: clarify a skip r=ericharmeling a=knz

Release note: None
Epic: CRDB-28893

Co-authored-by: Jayant Shrivastava <[email protected]>
Co-authored-by: Herko Lategan <[email protected]>
Co-authored-by: Arul Ajmani <[email protected]>
Co-authored-by: Raphael 'kena' Poss <[email protected]>

build/bazelutil/check.sh

@@ -22,7 +22,7 @@ pkg/cmd/roachtest/tests/drt.go://go:generate mockgen -package tests -destination
 pkg/kv/kvclient/kvcoord/transport.go://go:generate mockgen -package=kvcoord -destination=mocks_generated_test.go . Transport
 pkg/kv/kvclient/rangecache/range_cache.go://go:generate mockgen -package=rangecachemock -destination=rangecachemock/mocks_generated.go . RangeDescriptorDB
 pkg/kv/kvclient/rangefeed/rangefeed.go://go:generate mockgen -destination=mocks_generated_test.go --package=rangefeed . DB
-pkg/kv/kvserver/concurrency/lock_table.go://go:generate ../../../util/interval/generic/gen.sh *lockState concurrency
+pkg/kv/kvserver/concurrency/lock_table.go://go:generate ../../../util/interval/generic/gen.sh *keyLocks concurrency
 pkg/kv/kvserver/spanlatch/manager.go://go:generate ../../../util/interval/generic/gen.sh *latch spanlatch
 pkg/kv/kvpb/api.go://go:generate mockgen -package=kvpbmock -destination=kvpbmock/mocks_generated.go . InternalClient,Internal_RangeFeedClient,Internal_MuxRangeFeedClient
 pkg/kv/kvpb/batch.go://go:generate go run gen/main.go --filename batch_generated.go *.pb.go

pkg/cmd/roachtest/cluster.go

@@ -2375,40 +2375,29 @@ func addrToHostPort(addr string) (string, int, error) {
 // InternalAdminUIAddr returns the internal Admin UI address in the form host:port
 // for the specified node.
 func (c *clusterImpl) InternalAdminUIAddr(
-	ctx context.Context, l *logger.Logger, node option.NodeListOption,
+	_ context.Context, l *logger.Logger, node option.NodeListOption,
 ) ([]string, error) {
-	var addrs []string
-	internalAddrs, err := roachprod.AdminURL(l, c.MakeNodes(node), "", "",
-		false, false, false)
-	if err != nil {
-		return nil, err
-	}
-	for _, u := range internalAddrs {
-		addr, err := urlToAddr(u)
-		if err != nil {
-			return nil, err
-		}
-		adminUIAddr, err := addrToAdminUIAddr(addr)
-		if err != nil {
-			return nil, err
-		}
-		addrs = append(addrs, adminUIAddr)
-	}
-	return addrs, nil
+	return c.adminUIAddr(l, node, false)
 }
 
 // ExternalAdminUIAddr returns the external Admin UI address in the form host:port
 // for the specified node.
 func (c *clusterImpl) ExternalAdminUIAddr(
 	_ context.Context, l *logger.Logger, node option.NodeListOption,
+) ([]string, error) {
+	return c.adminUIAddr(l, node, true)
+}
+
+func (c *clusterImpl) adminUIAddr(
+	l *logger.Logger, node option.NodeListOption, external bool,
 ) ([]string, error) {
 	var addrs []string
-	externalAddrs, err := roachprod.AdminURL(l, c.MakeNodes(node), "", "",
-		true, false, false)
+	adminURLs, err := roachprod.AdminURL(l, c.MakeNodes(node), "", "",
+		external, false, false)
 	if err != nil {
 		return nil, err
 	}
-	for _, u := range externalAddrs {
+	for _, u := range adminURLs {
 		addr, err := urlToAddr(u)
 		if err != nil {
 			return nil, err
@@ -2422,40 +2411,34 @@ func (c *clusterImpl) ExternalAdminUIAddr(
 	return addrs, nil
 }
 
-// InternalAddr returns the internal address in the form host:port for the
-// specified nodes.
-func (c *clusterImpl) InternalAddr(
+// InternalIP returns the internal IP addresses for the specified nodes.
+func (c *clusterImpl) InternalIP(
 	ctx context.Context, l *logger.Logger, node option.NodeListOption,
 ) ([]string, error) {
-	var addrs []string
-	urls, err := c.pgURLErr(ctx, l, node, false, "")
-	if err != nil {
-		return nil, err
-	}
-	for _, u := range urls {
-		addr, err := urlToAddr(u)
-		if err != nil {
-			return nil, err
-		}
-		addrs = append(addrs, addr)
-	}
-	return addrs, nil
+	return roachprod.IP(l, c.MakeNodes(node), false)
 }
 
-// InternalIP returns the internal IP addresses for the specified nodes.
-func (c *clusterImpl) InternalIP(
+// InternalAddr returns the internal address in the form host:port for the
+// specified nodes.
+func (c *clusterImpl) InternalAddr(
 	ctx context.Context, l *logger.Logger, node option.NodeListOption,
 ) ([]string, error) {
-	return roachprod.IP(l, c.MakeNodes(node), false)
+	return c.addr(ctx, l, node, false)
 }
 
 // ExternalAddr returns the external address in the form host:port for the
 // specified node.
 func (c *clusterImpl) ExternalAddr(
 	ctx context.Context, l *logger.Logger, node option.NodeListOption,
+) ([]string, error) {
+	return c.addr(ctx, l, node, true)
+}
+
+func (c *clusterImpl) addr(
+	ctx context.Context, l *logger.Logger, node option.NodeListOption, external bool,
 ) ([]string, error) {
 	var addrs []string
-	urls, err := c.pgURLErr(ctx, l, node, true, "")
+	urls, err := c.pgURLErr(ctx, l, node, external, "")
 	if err != nil {
 		return nil, err
 	}

pkg/cmd/roachtest/cluster/cluster_interface.go

@@ -73,6 +73,7 @@ type Cluster interface {
 	ExternalPGUrl(ctx context.Context, l *logger.Logger, node option.NodeListOption, tenant string) ([]string, error)
 
 	// SQL clients to nodes.
+
 	Conn(ctx context.Context, l *logger.Logger, node int, opts ...func(*option.ConnOption)) *gosql.DB
 	ConnE(ctx context.Context, l *logger.Logger, node int, opts ...func(*option.ConnOption)) (*gosql.DB, error)
 
@@ -110,7 +111,7 @@ type Cluster interface {
 	IsLocal() bool
 	// IsSecure returns true iff the cluster uses TLS.
 	IsSecure() bool
-	// Returns CPU architecture of the nodes.
+	// Architecture returns CPU architecture of the nodes.
 	Architecture() vm.CPUArch
 
 	// Deleting CockroachDB data and logs on nodes.

pkg/cmd/roachtest/tests/cdc.go

@@ -641,7 +641,7 @@ func runCDCBank(ctx context.Context, t test.Test, c cluster.Cluster) {
 		t.Fatal(err)
 	}
 
-	tc, err := kafka.consumer(ctx, "bank")
+	tc, err := kafka.newConsumer(ctx, "bank")
 	if err != nil {
 		t.Fatal(errors.Wrap(err, "could not create kafka consumer"))
 	}
@@ -2242,7 +2242,7 @@ func (k kafkaManager) createTopic(ctx context.Context, topic string) error {
 	})
 }
 
-func (k kafkaManager) consumer(ctx context.Context, topic string) (*topicConsumer, error) {
+func (k kafkaManager) newConsumer(ctx context.Context, topic string) (*topicConsumer, error) {
 	kafkaAddrs := []string{k.consumerURL(ctx)}
 	config := sarama.NewConfig()
 	// I was seeing "error processing FetchRequest: kafka: error decoding