Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
95013: sql: add ability set, edit, read tenant capabilities r=knz a=ecwall

Fixes #87851

Add new SQL syntax for
1) Setting tenant capabilities:
`ALTER TENANT t GRANT CAPABILITY capabilitiy_name=capability_value;`
2) Resetting tenant capabilities:
`ALTER TENANT t REVOKE CAPABILITIY capability_name;`
3) Reading tenant capabilities:
`SHOW TENANT t WITH CAPABILITIES;`

Release note: None

95797: sql: improve stack trace for get-user-timeout timeouts r=knz a=ecwall

Fixes #95794

The cause of the `get-user-timeout` errors is unknown. Part of the problem is that the stack trace gets cut off at
```
  |   | github.com/cockroachdb/cockroach/pkg/sql.retrieveSessionInitInfoWithCache
  |   | 	github.com/cockroachdb/cockroach/pkg/sql/user.go:238
```
which does not explain what is actually being blocked.

The reason that the stack trace is cut off is that the timeout is initiated by `contextutil.RunWithTimeout` which results in a "simple" (no stack trace) `context.DeadlineExceeded` error.

`retrieveSessionInitInfoWithCache` is the first line in the stack trace because it calls `errors.Wrap` on `context.DeadlineExceeded`.

To get a fuller stack trace, `context.DeadlineExceeded` must be wrapped immediately (`errors.Wrap` or `errors.WithStack`) before it bubbles up.

Release note: None

95830: validate: use immutable descriptors only r=postamar a=postamar

The descriptor validation logic will accept any implementation of catalog.Descriptor be it mutable or immutable, it doesn't care. However, using mutable implementations can have a significant performance impact especially in the case of tables, where every column or index or constraint lookup will lead to the cache being regenerated for the whole descriptor.

This commit fixes this by having validate.Validate replace any mutable descriptor instances it encounters with immutable copies. This doesn't change anything except performance.

Fixes #95827.

Release note: None

95852: ui: cache sqlroles results r=maryliag a=maryliag

Previously, the call to get sql roles was constantly being requested. This commits adds a cache limit, so it will only get request after the expiration time.

https://www.loom.com/share/6814309f91234fa2b17490df8160bde6

Epic: None
Release note: None

95863: storage: reorder EventListeners r=jbowens a=jbowens

To be defensive, sequence the EventListener responsible for crashing the process during a disk stall first, before the Pebble logging event listener.

Informs #94373.
Epic: None
Release note: None

Co-authored-by: Evan Wall <[email protected]>
Co-authored-by: Marius Posta <[email protected]>
Co-authored-by: maryliag <[email protected]>
Co-authored-by: Jackson Owens <[email protected]>
  • Loading branch information
5 people committed Jan 25, 2023
6 parents 63aa313 + 0b29509 + 662e19c + b3713c7 + 62d314d + e40bd52 commit 4858095
Show file tree
Hide file tree
Showing 32 changed files with 667 additions and 114 deletions.
3 changes: 3 additions & 0 deletions docs/generated/sql/bnf/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ FILES = [
"opt_frame_clause",
"opt_locality",
"opt_persistence_temp_table",
"opt_show_tenant_options",
"opt_with_storage_parameter_list",
"pause_all_jobs_stmt",
"pause_job",
Expand Down Expand Up @@ -266,6 +267,8 @@ FILES = [
"table_clause",
"table_constraint",
"table_ref",
"tenant_capability",
"tenant_capability_list",
"transaction_stmt",
"truncate_stmt",
"unique_column_level",
Expand Down
2 changes: 2 additions & 0 deletions docs/generated/sql/bnf/opt_show_tenant_options.bnf
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
opt_show_tenant_options ::=
'WITH' show_tenant_options
9 changes: 3 additions & 6 deletions docs/generated/sql/bnf/show_tenant_stmt.bnf
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
show_tenant_stmt ::=
'SHOW' 'TENANTS'
| 'SHOW' 'TENANT_ALL' 'ALL'
| 'SHOW' 'TENANTS' 'WITH' 'REPLICATION' 'STATUS'
| 'SHOW' 'TENANT_ALL' 'ALL' 'WITH' 'REPLICATION' 'STATUS'
| 'SHOW' 'TENANT' tenant_spec
| 'SHOW' 'TENANT' tenant_spec 'WITH' 'REPLICATION' 'STATUS'
'SHOW' 'TENANTS' opt_show_tenant_options
| 'SHOW' 'TENANT_ALL' 'ALL' opt_show_tenant_options
| 'SHOW' 'TENANT' tenant_spec opt_show_tenant_options
17 changes: 11 additions & 6 deletions docs/generated/sql/bnf/stmt_block.bnf
Original file line number Diff line number Diff line change
Expand Up @@ -917,12 +917,9 @@ show_tables_stmt ::=
| 'SHOW' 'TABLES' with_comment

show_tenant_stmt ::=
'SHOW' 'TENANTS'
| 'SHOW' 'TENANT_ALL' 'ALL'
| 'SHOW' 'TENANTS' 'WITH' 'REPLICATION' 'STATUS'
| 'SHOW' 'TENANT_ALL' 'ALL' 'WITH' 'REPLICATION' 'STATUS'
| 'SHOW' 'TENANT' tenant_spec
| 'SHOW' 'TENANT' tenant_spec 'WITH' 'REPLICATION' 'STATUS'
'SHOW' 'TENANTS' opt_show_tenant_options
| 'SHOW' 'TENANT_ALL' 'ALL' opt_show_tenant_options
| 'SHOW' 'TENANT' tenant_spec opt_show_tenant_options

show_trace_stmt ::=
'SHOW' opt_compact 'TRACE' 'FOR' 'SESSION'
Expand Down Expand Up @@ -1031,6 +1028,8 @@ unreserved_keyword ::=
| 'CALLED'
| 'CANCEL'
| 'CANCELQUERY'
| 'CAPABILITIES'
| 'CAPABILITY'
| 'CASCADE'
| 'CHANGEFEED'
| 'CLOSE'
Expand Down Expand Up @@ -1938,6 +1937,9 @@ statements_or_queries ::=
opt_show_ranges_options ::=
'WITH' show_ranges_options

opt_show_tenant_options ::=
'WITH' show_tenant_options

opt_compact ::=
'COMPACT'
|
Expand Down Expand Up @@ -2647,6 +2649,9 @@ targets_roles ::=
show_ranges_options ::=
( 'TABLES' | 'INDEXES' | 'DETAILS' | 'KEYS' | 'EXPLAIN' ) ( ( ',' 'TABLES' | ',' 'INDEXES' | ',' 'DETAILS' | ',' 'EXPLAIN' | ',' 'KEYS' ) )*

show_tenant_options ::=
( 'REPLICATION' 'STATUS' | 'CAPABILITIES' ) ( ( ',' 'REPLICATION' 'STATUS' | ',' 'CAPABILITIES' ) )*

partition ::=
'PARTITION' partition_name

Expand Down
3 changes: 3 additions & 0 deletions docs/generated/sql/bnf/tenant_capability.bnf
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
tenant_capability ::=
var_name
| var_name to_or_eq var_value
2 changes: 2 additions & 0 deletions docs/generated/sql/bnf/tenant_capability_list.bnf
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
tenant_capability_list ::=
tenant_capability ( ( ',' tenant_capability ) )*
43 changes: 43 additions & 0 deletions pkg/ccl/logictestccl/testdata/logic_test/tenant_capability
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# LogicTest: local

statement ok
SELECT crdb_internal.create_tenant(5, 'five')

statement error error parsing capability "not_a_capability": invalid capability
ALTER TENANT [5] GRANT CAPABILITY not_a_capability=true

statement error error parsing capability "can_admin_split": value must be bool
ALTER TENANT [5] GRANT CAPABILITY can_admin_split=1

statement error error parsing capability "not_a_capability": invalid capability
ALTER TENANT [5] REVOKE CAPABILITY not_a_capability

statement error error parsing capability "can_admin_split": revoke must not specify value
ALTER TENANT [5] REVOKE CAPABILITY can_admin_split=false

query ITTTT colnames
SHOW TENANT 'five' WITH CAPABILITIES
----
id name status capability_name capability_value
5 five ACTIVE can_admin_split false
5 five ACTIVE can_admin_unsplit false

statement ok
ALTER TENANT [5] GRANT CAPABILITY can_admin_split=true

query ITTTT colnames
SHOW TENANT 'five' WITH CAPABILITIES
----
id name status capability_name capability_value
5 five ACTIVE can_admin_split true
5 five ACTIVE can_admin_unsplit false

statement ok
ALTER TENANT [5] REVOKE CAPABILITY can_admin_split

query ITTTT colnames
SHOW TENANT 'five' WITH CAPABILITIES
----
id name status capability_name capability_value
5 five ACTIVE can_admin_split false
5 five ACTIVE can_admin_unsplit false
7 changes: 7 additions & 0 deletions pkg/ccl/logictestccl/tests/local/generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions pkg/gen/bnf.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ BNF_SRCS = [
"//docs/generated/sql/bnf:opt_frame_clause.bnf",
"//docs/generated/sql/bnf:opt_locality.bnf",
"//docs/generated/sql/bnf:opt_persistence_temp_table.bnf",
"//docs/generated/sql/bnf:opt_show_tenant_options.bnf",
"//docs/generated/sql/bnf:opt_with_storage_parameter_list.bnf",
"//docs/generated/sql/bnf:pause_all_jobs_stmt.bnf",
"//docs/generated/sql/bnf:pause_job.bnf",
Expand Down Expand Up @@ -266,6 +267,8 @@ BNF_SRCS = [
"//docs/generated/sql/bnf:table_clause.bnf",
"//docs/generated/sql/bnf:table_constraint.bnf",
"//docs/generated/sql/bnf:table_ref.bnf",
"//docs/generated/sql/bnf:tenant_capability.bnf",
"//docs/generated/sql/bnf:tenant_capability_list.bnf",
"//docs/generated/sql/bnf:transaction_stmt.bnf",
"//docs/generated/sql/bnf:truncate_stmt.bnf",
"//docs/generated/sql/bnf:unique_column_level.bnf",
Expand Down
3 changes: 3 additions & 0 deletions pkg/gen/diagrams.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ DIAGRAMS_SRCS = [
"//docs/generated/sql/bnf:opt_frame_clause.html",
"//docs/generated/sql/bnf:opt_locality.html",
"//docs/generated/sql/bnf:opt_persistence_temp_table.html",
"//docs/generated/sql/bnf:opt_show_tenant_options.html",
"//docs/generated/sql/bnf:opt_with_storage_parameter_list.html",
"//docs/generated/sql/bnf:pause.html",
"//docs/generated/sql/bnf:pause_all_jobs.html",
Expand Down Expand Up @@ -260,6 +261,8 @@ DIAGRAMS_SRCS = [
"//docs/generated/sql/bnf:table_clause.html",
"//docs/generated/sql/bnf:table_constraint.html",
"//docs/generated/sql/bnf:table_ref.html",
"//docs/generated/sql/bnf:tenant_capability.html",
"//docs/generated/sql/bnf:tenant_capability_list.html",
"//docs/generated/sql/bnf:transaction.html",
"//docs/generated/sql/bnf:truncate.html",
"//docs/generated/sql/bnf:unique_column_level.html",
Expand Down
3 changes: 3 additions & 0 deletions pkg/gen/docs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ DOCS_SRCS = [
"//docs/generated/sql/bnf:opt_frame_clause.bnf",
"//docs/generated/sql/bnf:opt_locality.bnf",
"//docs/generated/sql/bnf:opt_persistence_temp_table.bnf",
"//docs/generated/sql/bnf:opt_show_tenant_options.bnf",
"//docs/generated/sql/bnf:opt_with_storage_parameter_list.bnf",
"//docs/generated/sql/bnf:pause_all_jobs_stmt.bnf",
"//docs/generated/sql/bnf:pause_job.bnf",
Expand Down Expand Up @@ -278,6 +279,8 @@ DOCS_SRCS = [
"//docs/generated/sql/bnf:table_clause.bnf",
"//docs/generated/sql/bnf:table_constraint.bnf",
"//docs/generated/sql/bnf:table_ref.bnf",
"//docs/generated/sql/bnf:tenant_capability.bnf",
"//docs/generated/sql/bnf:tenant_capability_list.bnf",
"//docs/generated/sql/bnf:transaction_stmt.bnf",
"//docs/generated/sql/bnf:truncate_stmt.bnf",
"//docs/generated/sql/bnf:unique_column_level.bnf",
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ go_library(
"telemetry_logging.go",
"temporary_schema.go",
"tenant_accessors.go",
"tenant_capability.go",
"tenant_creation.go",
"tenant_deletion.go",
"tenant_gc.go",
Expand Down
7 changes: 7 additions & 0 deletions pkg/sql/catalog/colinfo/result_columns.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,13 @@ var TenantColumnsWithReplication = ResultColumns{
{Name: "cutover_time", Typ: types.Decimal},
}

// TenantColumnsWithCapabilities is appended to TenantColumns for
// SHOW TENANT ... WITH CAPABILITIES queries.
var TenantColumnsWithCapabilities = ResultColumns{
{Name: "capability_name", Typ: types.String},
{Name: "capability_value", Typ: types.String},
}

// RangesNoLeases is the schema for crdb_internal.ranges_no_leases.
var RangesNoLeases = ResultColumns{
{Name: "range_id", Typ: types.Int},
Expand Down
21 changes: 19 additions & 2 deletions pkg/sql/catalog/internal/validate/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,15 @@ func Validate(
targetLevel catalog.ValidationLevel,
descriptors ...catalog.Descriptor,
) catalog.ValidationErrors {
for i, d := range descriptors {
// Replace mutable descriptors with immutable copies. Validation is
// read-only in any case, and using immutables can have a significant
// impact on performance when validating tables due to columns, indexes,
// and so forth being cached.
if mut, ok := d.(catalog.MutableDescriptor); ok {
descriptors[i] = mut.ImmutableCopy()
}
}
vea := validationErrorAccumulator{
ValidationTelemetry: telemetry,
targetLevel: targetLevel,
Expand Down Expand Up @@ -409,9 +418,17 @@ func (cs *collectorState) getMissingDescs(
return nil, err
}
for _, desc := range resps {
if desc != nil {
cs.vdg.descriptors[desc.GetID()] = desc
if desc == nil {
continue
}
if mut, ok := desc.(catalog.MutableDescriptor); ok {
// Replace mutable descriptors with immutable copies. Validation is
// read-only in any case, and using immutables can have a significant
// impact on performance when validating tables due to columns, indexes,
// and so forth being cached.
desc = mut.ImmutableCopy()
}
cs.vdg.descriptors[desc.GetID()] = desc
}
return resps, nil
}
Expand Down
3 changes: 3 additions & 0 deletions pkg/sql/opaque.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ func planOpaque(ctx context.Context, p *planner, stmt tree.Statement) (planNode,
return p.AlterTableOwner(ctx, n)
case *tree.AlterTableSetSchema:
return p.AlterTableSetSchema(ctx, n)
case *tree.AlterTenantCapability:
return p.AlterTenantCapability(ctx, n)
case *tree.AlterTenantSetClusterSetting:
return p.AlterTenantSetClusterSetting(ctx, n)
case *tree.AlterTenantRename:
Expand Down Expand Up @@ -312,6 +314,7 @@ func init() {
&tree.AlterTableLocality{},
&tree.AlterTableOwner{},
&tree.AlterTableSetSchema{},
&tree.AlterTenantCapability{},
&tree.AlterTenantRename{},
&tree.AlterTenantSetClusterSetting{},
&tree.AlterType{},
Expand Down
Loading

0 comments on commit 4858095

Please sign in to comment.