diff --git a/pkg/ccl/serverccl/BUILD.bazel b/pkg/ccl/serverccl/BUILD.bazel index 240025badb29..b49b090c94d9 100644 --- a/pkg/ccl/serverccl/BUILD.bazel +++ b/pkg/ccl/serverccl/BUILD.bazel @@ -40,6 +40,7 @@ go_test( "server_controller_test.go", "server_sql_test.go", "server_startup_guardrails_test.go", + "shared_process_tenant_test.go", "tenant_decommissioned_host_test.go", "tenant_migration_test.go", "tenant_vars_test.go", diff --git a/pkg/ccl/serverccl/shared_process_tenant_test.go b/pkg/ccl/serverccl/shared_process_tenant_test.go new file mode 100644 index 000000000000..b836ace5b25f --- /dev/null +++ b/pkg/ccl/serverccl/shared_process_tenant_test.go @@ -0,0 +1,61 @@ +// Copyright 2023 The Cockroach Authors. +// +// Licensed as a CockroachDB Enterprise file under the Cockroach Community +// License (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// https://github.com/cockroachdb/cockroach/blob/master/licenses/CCL.txt + +package serverccl + +import ( + "context" + gosql "database/sql" + "testing" + + "github.com/cockroachdb/cockroach/pkg/base" + "github.com/cockroachdb/cockroach/pkg/testutils" + "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" + "github.com/cockroachdb/cockroach/pkg/util/leaktest" + "github.com/cockroachdb/cockroach/pkg/util/log" + "github.com/stretchr/testify/require" +) + +func TestSharedProcessTenantNoSpanLimit(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + + ctx := context.Background() + + tc := serverutils.StartNewTestCluster(t, 1, base.TestClusterArgs{ + ServerArgs: base.TestServerArgs{ + DisableDefaultTestTenant: true, + }}) + defer tc.Stopper().Stop(ctx) + + db := tc.ServerConn(0) + _, err := db.Exec("CREATE TENANT hello; ALTER TENANT hello START SERVICE SHARED") + require.NoError(t, err) + + _, err = db.Exec("ALTER TENANT ALL SET CLUSTER SETTING spanconfig.tenant_limit = 1000") + require.NoError(t, err) + + sqlAddr := tc.Server(0).ServingSQLAddr() + var tenantDB *gosql.DB + testutils.SucceedsSoon(t, func() error { + var err error + tenantDB, err = serverutils.OpenDBConnE(sqlAddr, "cluster:hello", false, tc.Stopper()) + if err != nil { + return err + } + + if err := tenantDB.Ping(); err != nil { + return err + } + return nil + }) + defer tenantDB.Close() + + _, err = tenantDB.Exec("SELECT crdb_internal.generate_test_objects('foo', 1001)") + require.NoError(t, err) +} diff --git a/pkg/server/server_sql.go b/pkg/server/server_sql.go index f950d7de9f61..ab692cd40263 100644 --- a/pkg/server/server_sql.go +++ b/pkg/server/server_sql.go @@ -247,7 +247,9 @@ type sqlServerOptionalKVArgs struct { // sqlServerOptionalTenantArgs are the arguments supplied to newSQLServer which // are only available if the SQL server runs as part of a standalone SQL node. type sqlServerOptionalTenantArgs struct { - tenantConnect kvtenant.Connector + tenantConnect kvtenant.Connector + spanLimiterFactory spanLimiterFactory + promRuleExporter *metric.PrometheusRuleExporter } @@ -748,13 +750,18 @@ func newSQLServer(ctx context.Context, cfg sqlServerArgs) (*SQLServer, error) { splitter spanconfig.Splitter limiter spanconfig.Limiter }{} + + spanConfigKnobs, _ := cfg.TestingKnobs.SpanConfig.(*spanconfig.TestingKnobs) if codec.ForSystemTenant() { - spanConfig.limiter = spanconfiglimiter.NoopLimiter{} spanConfig.splitter = spanconfigsplitter.NoopSplitter{} } else { - spanConfigKnobs, _ := cfg.TestingKnobs.SpanConfig.(*spanconfig.TestingKnobs) spanConfig.splitter = spanconfigsplitter.New(codec, spanConfigKnobs) - spanConfig.limiter = spanconfiglimiter.New( + } + + if cfg.spanLimiterFactory == nil { + spanConfig.limiter = spanconfiglimiter.NoopLimiter{} + } else { + spanConfig.limiter = cfg.spanLimiterFactory( cfg.circularInternalExecutor, cfg.Settings, spanConfigKnobs, diff --git a/pkg/server/tenant.go b/pkg/server/tenant.go index e25d2b87e094..2fcba5a11eb3 100644 --- a/pkg/server/tenant.go +++ b/pkg/server/tenant.go @@ -49,8 +49,11 @@ import ( "github.com/cockroachdb/cockroach/pkg/server/status" "github.com/cockroachdb/cockroach/pkg/server/systemconfigwatcher" "github.com/cockroachdb/cockroach/pkg/settings/cluster" + "github.com/cockroachdb/cockroach/pkg/spanconfig" + "github.com/cockroachdb/cockroach/pkg/spanconfig/spanconfiglimiter" "github.com/cockroachdb/cockroach/pkg/sql" "github.com/cockroachdb/cockroach/pkg/sql/flowinfra" + "github.com/cockroachdb/cockroach/pkg/sql/isql" "github.com/cockroachdb/cockroach/pkg/sql/optionalnodeliveness" "github.com/cockroachdb/cockroach/pkg/sql/pgwire" "github.com/cockroachdb/cockroach/pkg/sql/sessiondatapb" @@ -150,6 +153,19 @@ func (s *SQLServerWrapper) Drain( return s.drainServer.runDrain(ctx, verbose) } +// tenantServerDeps holds dependencies for the SQL server that we want +// to vary based on whether we are in a shared process or separate +// process tenant. +type tenantServerDeps struct { + instanceIDContainer *base.SQLIDContainer + + // The following should eventually be connected to tenant + // capabilities. + costControllerFactory costControllerFactory + spanLimiterFactory spanLimiterFactory +} + +type spanLimiterFactory func(isql.Executor, *cluster.Settings, *spanconfig.TestingKnobs) spanconfig.Limiter type costControllerFactory func(*cluster.Settings, roachpb.TenantID, kvtenant.TokenBucketProvider) (multitenant.TenantSideCostController, error) // NewSeparateProcessTenantServer creates a tenant-specific, SQL-only @@ -165,9 +181,15 @@ func NewSeparateProcessTenantServer( sqlCfg SQLConfig, tenantNameContainer *roachpb.TenantNameContainer, ) (*SQLServerWrapper, error) { - instanceIDContainer := baseCfg.IDContainer.SwitchToSQLIDContainerForStandaloneSQLInstance() - costControllerBuilder := NewTenantSideCostController - return newTenantServer(ctx, stopper, baseCfg, sqlCfg, tenantNameContainer, instanceIDContainer, costControllerBuilder) + deps := tenantServerDeps{ + instanceIDContainer: baseCfg.IDContainer.SwitchToSQLIDContainerForStandaloneSQLInstance(), + costControllerFactory: NewTenantSideCostController, + spanLimiterFactory: func(ie isql.Executor, st *cluster.Settings, knobs *spanconfig.TestingKnobs) spanconfig.Limiter { + return spanconfiglimiter.New(ie, st, knobs) + }, + } + + return newTenantServer(ctx, stopper, baseCfg, sqlCfg, tenantNameContainer, deps) } // NewSharedProcessTenantServer creates a tenant-specific, SQL-only @@ -186,16 +208,22 @@ func NewSharedProcessTenantServer( if baseCfg.IDContainer.Get() == 0 { return nil, errors.AssertionFailedf("programming error: NewSharedProcessTenantServer called before NodeID was assigned.") } - instanceIDContainer := base.NewSQLIDContainerForNode(baseCfg.IDContainer) - // TODO(ssd): The cost controller should instead be able to - // read from the capability system and return immediately if - // the tenant is exempt. For now we are turning off the - // tenant-side cost controller for shared-memory tenants until - // we have the abilility to read capabilities tenant-side. - // - // https://github.com/cockroachdb/cockroach/issues/84586 - costControllerBuilder := NewNoopTenantSideCostController - return newTenantServer(ctx, stopper, baseCfg, sqlCfg, tenantNameContainer, instanceIDContainer, costControllerBuilder) + + deps := tenantServerDeps{ + instanceIDContainer: base.NewSQLIDContainerForNode(baseCfg.IDContainer), + // TODO(ssd): The cost controller should instead be able to + // read from the capability system and return immediately if + // the tenant is exempt. For now we are turning off the + // tenant-side cost controller for shared-memory tenants until + // we have the abilility to read capabilities tenant-side. + // + // https://github.com/cockroachdb/cockroach/issues/84586 + costControllerFactory: NewNoopTenantSideCostController, + spanLimiterFactory: func(isql.Executor, *cluster.Settings, *spanconfig.TestingKnobs) spanconfig.Limiter { + return spanconfiglimiter.NoopLimiter{} + }, + } + return newTenantServer(ctx, stopper, baseCfg, sqlCfg, tenantNameContainer, deps) } // newTenantServer constructs a SQLServerWrapper. @@ -207,8 +235,7 @@ func newTenantServer( baseCfg BaseConfig, sqlCfg SQLConfig, tenantNameContainer *roachpb.TenantNameContainer, - instanceIDContainer *base.SQLIDContainer, - costControllerFactory costControllerFactory, + deps tenantServerDeps, ) (*SQLServerWrapper, error) { // TODO(knz): Make the license application a per-server thing // instead of a global thing. @@ -220,7 +247,7 @@ func newTenantServer( // Inform the server identity provider that we're operating // for a tenant server. baseCfg.idProvider.SetTenant(sqlCfg.TenantID) - args, err := makeTenantSQLServerArgs(ctx, stopper, baseCfg, sqlCfg, tenantNameContainer, instanceIDContainer, costControllerFactory) + args, err := makeTenantSQLServerArgs(ctx, stopper, baseCfg, sqlCfg, tenantNameContainer, deps) if err != nil { return nil, err } @@ -905,8 +932,7 @@ func makeTenantSQLServerArgs( baseCfg BaseConfig, sqlCfg SQLConfig, tenantNameContainer *roachpb.TenantNameContainer, - instanceIDContainer *base.SQLIDContainer, - costControllerFactory costControllerFactory, + deps tenantServerDeps, ) (sqlServerArgs, error) { st := baseCfg.Settings @@ -999,7 +1025,7 @@ func makeTenantSQLServerArgs( tenantKnobs.OverrideTokenBucketProvider != nil { provider = tenantKnobs.OverrideTokenBucketProvider(provider) } - costController, err := costControllerFactory(st, sqlCfg.TenantID, provider) + costController, err := deps.costControllerFactory(st, sqlCfg.TenantID, provider) if err != nil { return sqlServerArgs{}, err } @@ -1041,7 +1067,7 @@ func makeTenantSQLServerArgs( ) dbCtx := kv.DefaultDBContext(stopper) - dbCtx.NodeID = instanceIDContainer + dbCtx.NodeID = deps.instanceIDContainer db := kv.NewDBWithContext(baseCfg.AmbientCtx, tcsFactory, clock, dbCtx) rangeFeedKnobs, _ := baseCfg.TestingKnobs.RangeFeed.(*rangefeed.TestingKnobs) @@ -1156,13 +1182,14 @@ func makeTenantSQLServerArgs( externalStorageFromURI: externalStorageFromURI, // Set instance ID to 0 and node ID to nil to indicate // that the instance ID will be bound later during preStart. - nodeIDContainer: instanceIDContainer, + nodeIDContainer: deps.instanceIDContainer, spanConfigKVAccessor: tenantConnect, kvStoresIterator: kvserverbase.UnsupportedStoresIterator{}, }, sqlServerOptionalTenantArgs: sqlServerOptionalTenantArgs{ - tenantConnect: tenantConnect, - promRuleExporter: promRuleExporter, + spanLimiterFactory: deps.spanLimiterFactory, + tenantConnect: tenantConnect, + promRuleExporter: promRuleExporter, }, SQLConfig: &sqlCfg, BaseConfig: &baseCfg,