Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

server: use no-op cost controller for shared-process tenants #99113

Merged
merged 2 commits into from
Mar 21, 2023
Merged

server: use no-op cost controller for shared-process tenants #99113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
96175f1
server: use no-op cost controller for shared-process tenants
stevendanna Mar 21, 2023
9e05d17
bench: exempt shared process tenants from rate limits
stevendanna Mar 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions pkg/bench/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,15 @@ go_library(
deps = [
"//pkg/base",
"//pkg/ccl",
"//pkg/multitenant/tenantcapabilities",
"//pkg/roachpb",
"//pkg/server",
"//pkg/testutils",
"//pkg/testutils/serverutils",
"//pkg/testutils/skip",
"//pkg/testutils/sqlutils",
"//pkg/testutils/testcluster",
"@com_github_cockroachdb_errors//:errors",
"@com_github_go_sql_driver_mysql//:mysql",
"@com_github_lib_pq//:pq",
"@com_github_stretchr_testify//require",
Expand Down
24 changes: 24 additions & 0 deletions pkg/bench/foreachdb.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,15 @@ import (

"github.com/cockroachdb/cockroach/pkg/base"
_ "github.com/cockroachdb/cockroach/pkg/ccl"
"github.com/cockroachdb/cockroach/pkg/multitenant/tenantcapabilities"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/server"
"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/testutils/testcluster"
"github.com/cockroachdb/errors"
_ "github.com/go-sql-driver/mysql" // registers the MySQL driver to gosql
_ "github.com/lib/pq" // registers the pg driver to gosql
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -78,6 +81,27 @@ func benchmarkSharedProcessTenantCockroach(b *testing.B, f BenchmarkFn) {
_, err = db.Exec(`ALTER TENANT ALL SET CLUSTER SETTING "spanconfig.tenant_limit" = 10000000`)
require.NoError(b, err)

// Exempt the tenant from rate limiting. We expect most
// shared-process tenants will run without rate limiting in
// the near term.
_, err = db.Exec(`ALTER TENANT benchtenant GRANT CAPABILITY exempt_from_rate_limiting`)
require.NoError(b, err)

var tenantID uint64
require.NoError(b, db.QueryRow(`SELECT id FROM [SHOW TENANT benchtenant]`).Scan(&tenantID))

err = testutils.SucceedsSoonError(func() error {
capabilities, found := s.(*server.TestServer).Server.TenantCapabilitiesReader().GetCapabilities(roachpb.MustMakeTenantID(tenantID))
if !found {
return errors.Newf("capabilities not yet ready")
}
if !capabilities.GetBool(tenantcapabilities.ExemptFromRateLimiting) {
return errors.Newf("capabilities not yet ready")
}
return nil
})
require.NoError(b, err)

_, err = tenantDB.Exec(`CREATE DATABASE bench`)
require.NoError(b, err)

Expand Down
29 changes: 23 additions & 6 deletions pkg/server/tenant.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,8 @@ func (s *SQLServerWrapper) Drain(
return s.drainServer.runDrain(ctx, verbose)
}

type costControllerFactory func(*cluster.Settings, roachpb.TenantID, kvtenant.TokenBucketProvider) (multitenant.TenantSideCostController, error)

// NewSeparateProcessTenantServer creates a tenant-specific, SQL-only
// server against a KV backend, with defaults appropriate for a
// SQLServer that is not located in the same process as a KVServer.
Expand All @@ -164,7 +166,8 @@ func NewSeparateProcessTenantServer(
tenantNameContainer *roachpb.TenantNameContainer,
) (*SQLServerWrapper, error) {
instanceIDContainer := baseCfg.IDContainer.SwitchToSQLIDContainerForStandaloneSQLInstance()
return newTenantServer(ctx, stopper, baseCfg, sqlCfg, tenantNameContainer, instanceIDContainer)
costControllerBuilder := NewTenantSideCostController
return newTenantServer(ctx, stopper, baseCfg, sqlCfg, tenantNameContainer, instanceIDContainer, costControllerBuilder)
}

// NewSharedProcessTenantServer creates a tenant-specific, SQL-only
Expand All @@ -184,7 +187,15 @@ func NewSharedProcessTenantServer(
return nil, errors.AssertionFailedf("programming error: NewSharedProcessTenantServer called before NodeID was assigned.")
}
instanceIDContainer := base.NewSQLIDContainerForNode(baseCfg.IDContainer)
return newTenantServer(ctx, stopper, baseCfg, sqlCfg, tenantNameContainer, instanceIDContainer)
// 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)
}

// newTenantServer constructs a SQLServerWrapper.
Expand All @@ -197,6 +208,7 @@ func newTenantServer(
sqlCfg SQLConfig,
tenantNameContainer *roachpb.TenantNameContainer,
instanceIDContainer *base.SQLIDContainer,
costControllerFactory costControllerFactory,
) (*SQLServerWrapper, error) {
// TODO(knz): Make the license application a per-server thing
// instead of a global thing.
Expand All @@ -208,7 +220,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)
args, err := makeTenantSQLServerArgs(ctx, stopper, baseCfg, sqlCfg, tenantNameContainer, instanceIDContainer, costControllerFactory)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -894,6 +906,7 @@ func makeTenantSQLServerArgs(
sqlCfg SQLConfig,
tenantNameContainer *roachpb.TenantNameContainer,
instanceIDContainer *base.SQLIDContainer,
costControllerFactory costControllerFactory,
) (sqlServerArgs, error) {
st := baseCfg.Settings

Expand Down Expand Up @@ -986,7 +999,7 @@ func makeTenantSQLServerArgs(
tenantKnobs.OverrideTokenBucketProvider != nil {
provider = tenantKnobs.OverrideTokenBucketProvider(provider)
}
costController, err := NewTenantSideCostController(st, sqlCfg.TenantID, provider)
costController, err := costControllerFactory(st, sqlCfg.TenantID, provider)
if err != nil {
return sqlServerArgs{}, err
}
Expand Down Expand Up @@ -1218,8 +1231,12 @@ func makeNextLiveInstanceIDFn(

// NewTenantSideCostController is a hook for CCL code which implements the
// controller.
var NewTenantSideCostController = func(
st *cluster.Settings, tenantID roachpb.TenantID, provider kvtenant.TokenBucketProvider,
var NewTenantSideCostController costControllerFactory = NewNoopTenantSideCostController

// NewNoopTenantSideCostController returns a noop cost
// controller. Used by shared-process tenants.
func NewNoopTenantSideCostController(
*cluster.Settings, roachpb.TenantID, kvtenant.TokenBucketProvider,
) (multitenant.TenantSideCostController, error) {
// Return a no-op implementation.
return noopTenantSideCostController{}, nil
Expand Down