Skip to content

Commit

Permalink
tenantcostclient: restrict allowed configuration from the tenant side
Browse files Browse the repository at this point in the history
This change restricts the configuration of tenant cost control from
the tenant side. In the future, we will want to have settings where
the values come from the host cluster but we don't have that
infrastructure today.

With tenants being able to set their own settings, they could easily
sabotage the cost control mechanisms. This change restricts the
allowed values for the target period and the CPU usage allowance, and
fixes the cost model configuration to the default.

Release note: None

Release justification: Necessary fix for the distributed rate limiting
functionality, which is vital for the upcoming Serverless MVP release.
It allows CRDB to throttle clusters that have run out of free or paid
request units (which measure CPU and I/O usage). This functionality is
only enabled in multi-tenant scenarios and should have no impact on
our dedicated customers.
  • Loading branch information
RaduBerinde committed Sep 12, 2021
1 parent f6848a5 commit e099791
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 4 deletions.
23 changes: 19 additions & 4 deletions pkg/ccl/multitenantccl/tenantcostclient/tenant_side.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ var TargetPeriodSetting = settings.RegisterDurationSetting(
"tenant_cost_control_period",
"target duration between token bucket requests from tenants (requires restart)",
10*time.Second,
settings.PositiveDuration,
checkDurationInRange(5*time.Second, 120*time.Second),
)

// CPUUsageAllowance is exported for testing purposes.
Expand All @@ -42,9 +42,22 @@ var CPUUsageAllowance = settings.RegisterDurationSetting(
"doesn't contribute to consumption; for example, if it is set to 10ms, "+
"that corresponds to 1% of a CPU",
10*time.Millisecond,
settings.NonNegativeDuration,
checkDurationInRange(0, 10*time.Millisecond),
)

// checkDurationInRange returns a function used to validate duration cluster
// settings. Because these values are currently settable by the tenant, we need
// to restrict the allowed values to avoid possible sabotage of the cost control
// mechanisms.
func checkDurationInRange(min, max time.Duration) func(v time.Duration) error {
return func(v time.Duration) error {
if v < min || v > max {
return errors.Errorf("value %s out of range (%s, %s)", v, min, max)
}
return nil
}
}

// mainLoopUpdateInterval is the period at which we collect CPU usage and
// evaluate whether we need to send a new token request.
const mainLoopUpdateInterval = 1 * time.Second
Expand Down Expand Up @@ -93,8 +106,10 @@ func newTenantSideCostController(
}
c.limiter.Init(c.timeSource, c.lowRUNotifyChan)

// TODO(radu): support changing the tenant configuration at runtime.
c.costCfg = tenantcostmodel.ConfigFromSettings(&st.SV)
// TODO(radu): these settings can currently be changed by the tenant (see
// #47918), which would made it very easy to evade cost control. For now, use
// the hardcoded default values.
c.costCfg = tenantcostmodel.DefaultConfig()
return c, nil
}

Expand Down
4 changes: 4 additions & 0 deletions pkg/multitenant/tenantcostmodel/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ import (
//
// The KV operation parameters are set based on experiments, where 1000 Request
// Units correspond to one CPU second of usage on the host cluster.
//
// TODO(radu): these settings are not currently used on the tenant side; there,
// only the defaults are used. Ideally, the tenant would always get the values
// from the host cluster.
var (
readRequestCost = settings.RegisterFloatSetting(
"tenant_cost_model.kv_read_request_cost",
Expand Down

0 comments on commit e099791

Please sign in to comment.