Skip to content

Commit

Permalink
sql: Exempt SET CLUSTER SETTING from throttling
Browse files Browse the repository at this point in the history
This change prevents the SET CLUSTER SETTING ... command from being
throttled. Previously, it was subject to throttling, but since this
command is used to install a new license and disable throttling, we want
to ensure that the action itself is not throttled. This adds an
exception to allow license updates.

This will be backported to versions 24.2, 24.1, 23.2, and 23.1.

Epic: CRDB-39988
Closes CRDB-42308
Release note: None
  • Loading branch information
spilchen committed Sep 25, 2024
1 parent c5e9b22 commit 3ac4dd7
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 14 deletions.
23 changes: 14 additions & 9 deletions pkg/server/license/enforcer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,19 +305,24 @@ func TestThrottleErrorMsg(t *testing.T) {
overThreshold bool
throttleCheckTS time.Time
telemetryTS time.Time
sql string
errRE string
}{
// NB: license expires at t30d and grace period ends at t60d.
{"at-threshold-valid-license-and-telemetry", true, t10d, t10d, ""},
{"at-threshold-expired-license-in-grace-valid-telemetry", true, t60d, t55d, ""},
{"at-threshold-valid-license-and-telemetry", true, t10d, t10d, "SELECT 1", ""},
{"at-threshold-expired-license-in-grace-valid-telemetry", true, t60d, t55d, "SELECT 1", ""},
{"at-threshold-expired-license-past-grace-valid-telemetry", true, t60d1ms, t55d,
"License expired .*. The maximum number of .*"},
{"below-threshold-expired-license-past-grace-valid-telemetry", false, t60d1ms, t55d, ""},
{"at-threshold-expired-license-in-grace-invalid-telemetry", true, t55d, t10d,
"SELECT 1", "License expired .*. The maximum number of .*"},
{"below-threshold-expired-license-past-grace-valid-telemetry", false, t60d1ms, t55d, "SELECT 1", ""},
{"at-threshold-expired-license-in-grace-invalid-telemetry", true, t55d, t10d, "SELECT 1",
"The maximum number of concurrently open transactions has been reached because the license requires diagnostic reporting"},
{"at-threshold-valid-license-invalid-telemetry", true, t10d, t0d,
{"at-threshold-valid-license-invalid-telemetry", true, t10d, t0d, "SELECT 1",
"The maximum number of concurrently open transactions has been reached because the license requires diagnostic reporting"},
{"below-threshold-invalid-telemetry", false, t10d, t0d, ""},
{"below-threshold-invalid-telemetry", false, t10d, t0d, "SELECT 1", ""},
{"at-threshold-expired-license-past-grace-valid-telemetry-SET_CLUSTER", true, t60d1ms, t55d,
"SET CLUSTER SETTING cluster.label = ''", ""},
{"at-threshold-expired-license-past-grace-invalid-telemetry-SET_CLUSTER", true, t60d1ms, t10d,
"SET CLUSTER SETTING cluster.label = ''", ""},
} {
t.Run(tc.desc, func(t *testing.T) {
// Adjust the throttle check time for this test unit
Expand All @@ -339,9 +344,9 @@ func TestThrottleErrorMsg(t *testing.T) {
// It may or may not be throttled, depending on tc parms.
tdb := sqlutils.MakeSQLRunner(sqlDB)
if tc.errRE != "" {
tdb.ExpectErr(t, tc.errRE, "SELECT 1")
tdb.ExpectErr(t, tc.errRE, tc.sql)
} else {
tdb.Exec(t, "SELECT 1")
tdb.Exec(t, tc.sql)
}

// Confirm that internal connections are never throttled
Expand Down
25 changes: 20 additions & 5 deletions pkg/sql/conn_executor_exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -505,11 +505,13 @@ func (ex *connExecutor) execStmtInOpenState(
}

// Enforce license policies. Throttling can occur if there is no valid
// license or if it has expired.
if notice, err := ex.server.cfg.LicenseEnforcer.MaybeFailIfThrottled(ctx, curOpen); err != nil {
return makeErrEvent(err)
} else if notice != nil {
res.BufferNotice(notice)
// license or if the existing one has expired.
if isSQLOkayToThrottle(ast) {
if notice, err := ex.server.cfg.LicenseEnforcer.MaybeFailIfThrottled(ctx, curOpen); err != nil {
return makeErrEvent(err)
} else if notice != nil {
res.BufferNotice(notice)
}
}
}

Expand Down Expand Up @@ -3446,3 +3448,16 @@ func (ex *connExecutor) execWithProfiling(
}
return err
}

// isSQLOkayToThrottle will return true if the given statement is allowed to be throttled.
func isSQLOkayToThrottle(ast tree.Statement) bool {
switch ast.(type) {
// We do not throttle the SET CLUSTER command, as this is how a new license
// would be installed to disable throttling. We want to avoid the situation
// where the action to disable throttling is itself throttled.
case *tree.SetClusterSetting:
return false
default:
return true
}
}

0 comments on commit 3ac4dd7

Please sign in to comment.