Merge #110378 #110394 #110407

110378: build: add new `engflowpublic` config r=rail a=rickystewart

This uses all the same settings, except a different front-end for the RBE scheduler.

Epic: CRDB-8308
Release note: None

110394: ptcache: fix TestStart r=yuzefovich a=yuzefovich

We were using the system tenant's stopper even when the default test tenant is started which then led to a panic because two different tracers were involved.

Fixes: #109959.

Release note: None

110407: sql/tests: deflake TestRandomSyntaxSchemaChangeColumn r=rafiss a=rafiss

The schema changes in this test are expected to take a longer time than other randomized tests. Bump the timeout.

A CPU profile also showed that this test spends a lot of its time constructing the call stack, and this occurs while building an error message in GetAttribute. It turns out that production code never actually looks at this error, so we can make the message much simpler.

informs #109304
Release note: None

Co-authored-by: Ricky Stewart <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>

.bazelrc

@@ -113,22 +113,28 @@ build:macos --host_action_env=PATH=/opt/homebrew/bin:/opt/local/bin:/usr/local/b
 # more precise --workspace_status_command option.
 build:simplestamp --stamp --workspace_status_command=./build/bazelutil/stamp.sh
 
-build:engflow --define=EXECUTOR=remote
-build:engflow --disk_cache=
-build:engflow --experimental_inmemory_dotd_files
-build:engflow --experimental_inmemory_jdeps_files
-build:engflow --incompatible_strict_action_env=true
-build:engflow --remote_timeout=600
-build:engflow --nolegacy_important_outputs
-build:engflow --grpc_keepalive_time=30s
-build:engflow --experimental_remote_cache_compression=true
-build:engflow --action_env=BAZEL_DO_NOT_DETECT_CPP_TOOLCHAIN=1
+build:engflowbase --define=EXECUTOR=remote
+build:engflowbase --disk_cache=
+build:engflowbase --experimental_inmemory_dotd_files
+build:engflowbase --experimental_inmemory_jdeps_files
+build:engflowbase --incompatible_strict_action_env=true
+build:engflowbase --remote_timeout=600
+build:engflowbase --nolegacy_important_outputs
+build:engflowbase --grpc_keepalive_time=30s
+build:engflowbase --experimental_remote_cache_compression=true
+build:engflowbase --action_env=BAZEL_DO_NOT_DETECT_CPP_TOOLCHAIN=1
+build:engflowbase --extra_execution_platforms=//build/toolchains:cross_linux
+test:engflowbase --test_env=REMOTE_EXEC=1
+build:engflow --config=engflowbase
 build:engflow --remote_cache=grpcs://tanzanite.cluster.engflow.com
 build:engflow --remote_executor=grpcs://tanzanite.cluster.engflow.com
 build:engflow --bes_backend=grpcs://tanzanite.cluster.engflow.com
 build:engflow --bes_results_url=https://tanzanite.cluster.engflow.com/invocation/
-build:engflow --extra_execution_platforms=//build/toolchains:cross_linux
-test:engflow --test_env=REMOTE_EXEC=1
+build:engflowpublic --config=engflowbase
+build:engflowpublic --remote_cache=grpcs://mesolite.cluster.engflow.com
+build:engflowpublic --remote_executor=grpcs://mesolite.cluster.engflow.com
+build:engflowpublic --bes_backend=grpcs://mesolite.cluster.engflow.com
+build:engflowpublic --bes_results_url=https://mesolite.cluster.engflow.com/invocation/
 
 try-import %workspace%/.bazelrc.user
 

pkg/kv/kvserver/protectedts/ptcache/cache_test.go

@@ -298,7 +298,7 @@ func TestStart(t *testing.T) {
 	defer log.Scope(t).Close(t)
 
 	ctx := context.Background()
-	setup := func() (serverutils.TestServerInterface, *ptcache.Cache) {
+	setup := func() (serverutils.ApplicationLayerInterface, *ptcache.Cache, func()) {
 		srv := serverutils.StartServerOnly(t,
 			base.TestServerArgs{
 				Knobs: base.TestingKnobs{
@@ -317,25 +317,26 @@ func TestStart(t *testing.T) {
 			DB:       execCfg.InternalDB,
 			Storage:  p,
 		})
-		return srv, c
+		return s, c, func() { srv.Stopper().Stop(ctx) }
 	}
 
 	t.Run("double start", func(t *testing.T) {
 		defer log.Scope(t).Close(t)
 
-		srv, c := setup()
-		defer srv.Stopper().Stop(ctx)
-		require.NoError(t, c.Start(ctx, srv.Stopper()))
-		require.EqualError(t, c.Start(ctx, srv.Stopper()),
+		s, c, cleanup := setup()
+		defer cleanup()
+		require.NoError(t, c.Start(ctx, s.AppStopper()))
+		require.EqualError(t, c.Start(ctx, s.AppStopper()),
 			"cannot start a Cache more than once")
 	})
 
 	t.Run("already stopped", func(t *testing.T) {
 		defer log.Scope(t).Close(t)
 
-		srv, c := setup()
-		srv.Stopper().Stop(ctx)
-		require.EqualError(t, c.Start(ctx, srv.Stopper()),
+		s, c, cleanup := setup()
+		defer cleanup()
+		s.AppStopper().Stop(ctx)
+		require.EqualError(t, c.Start(ctx, s.AppStopper()),
 			stop.ErrUnavailable.Error())
 	})
 }

pkg/server/tenant.go

@@ -1174,9 +1174,8 @@ func makeTenantSQLServerArgs(
 	circularJobRegistry := &jobs.Registry{}
 
 	// Initialize the protectedts subsystem in multi-tenant clusters.
-	var protectedTSProvider protectedts.Provider
 	protectedtsKnobs, _ := baseCfg.TestingKnobs.ProtectedTS.(*protectedts.TestingKnobs)
-	pp, err := ptprovider.New(ptprovider.Config{
+	protectedTSProvider, err := ptprovider.New(ptprovider.Config{
 		DB:       internalExecutorFactory,
 		Settings: st,
 		Knobs:    protectedtsKnobs,
@@ -1192,8 +1191,7 @@ func makeTenantSQLServerArgs(
 	if err != nil {
 		return sqlServerArgs{}, err
 	}
-	registry.AddMetricStruct(pp.Metrics())
-	protectedTSProvider = pp
+	registry.AddMetricStruct(protectedTSProvider.Metrics())
 
 	recorder := status.NewMetricsRecorder(
 		sqlCfg.TenantID, tenantNameContainer, nil /* nodeLiveness */, nil, /* remoteClocks */

pkg/sql/schemachanger/rel/schema_attribute.go

@@ -11,9 +11,8 @@
 package rel
 
 import (
+	"fmt"
 	"unsafe"
-
-	"github.com/cockroachdb/errors"
 )
 
 // GetAttribute returns the requested attribute from the passed entity. If
@@ -34,9 +33,12 @@ func (sc *Schema) GetAttribute(attribute Attr, v interface{}) (interface{}, erro
 
 	fi, ok := ti.attrFields[ord]
 	if !ok {
-		return nil, errors.Errorf(
-			"no field defined on %v for %v", ti.typ, attribute,
-		)
+		// We avoid using errors.Errorf here because this is a hot path that
+		// showed up when profiling TestRandomSyntaxSchemaChangeColumn. Errorf
+		// calls runtime.Callers to construct the call stack, which is expensive.
+		// No production code actually uses this error message, so using a
+		// simpler error doesn't have much downside.
+		return nil, fmt.Errorf("no field defined on type")
 	}
 
 	// There may be more than one field which stores this attribute. At most one

pkg/sql/tests/rsg_test.go

@@ -52,7 +52,7 @@ var (
 	flagRSGTime                    = flag.Duration("rsg", 0, "random syntax generator test duration")
 	flagRSGGoRoutines              = flag.Int("rsg-routines", 1, "number of Go routines executing random statements in each RSG test")
 	flagRSGExecTimeout             = flag.Duration("rsg-exec-timeout", 35*time.Second, "timeout duration when executing a statement")
-	flagRSGExecColumnChangeTimeout = flag.Duration("rsg-exec-column-change-timeout", 40*time.Second, "timeout duration when executing a statement for random column changes")
+	flagRSGExecColumnChangeTimeout = flag.Duration("rsg-exec-column-change-timeout", 50*time.Second, "timeout duration when executing a statement for random column changes")
 )
 
 func verifyFormat(sql string) error {