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.

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

sql: add ability set, edit, read tenant capabilities #87851

Closed
3 tasks
knz opened this issue Sep 12, 2022 · 1 comment · Fixed by #95013
Closed
3 tasks

sql: add ability set, edit, read tenant capabilities #87851

knz opened this issue Sep 12, 2022 · 1 comment · Fixed by #95013
Assignees
Labels
A-multitenancy Related to multi-tenancy A-sql-builtins SQL built-in functions and semantics thereof. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@knz
Copy link
Contributor

knz commented Sep 12, 2022

See the RFC here: #85954.

We would like to have a new overload to crdb_internal.create_tenant() and a new built-in function crdb_internal.set_tenant_capabilities (maybe also with a getter) to edit tenant capabilities.

This issue should also cover:

  • Cache invalidation of capabilities on edit.
  • SHOW TENANT CAPABILITIES statement
  • Syntax to create both privileged tenants and unprivileged ones

Epic: CRDB-19504

Jira issue: CRDB-19569

@knz knz added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-sql-builtins SQL built-in functions and semantics thereof. A-multitenancy Related to multi-tenancy T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) labels Sep 12, 2022
@ecwall ecwall changed the title sql: define a built-in function to set or edit tenant capabilities sql: add ability set or edit tenant capabilities Nov 17, 2022
@ecwall
Copy link
Contributor

ecwall commented Nov 17, 2022

Instead of adding new overloads (going from 3 to 6) for crdb_internal.create_tenant(), I will modify the CREATE TENANT syntax.

Discussion: https://cockroachlabs.slack.com/archives/C02HWA24541/p1668711998299439

@ecwall ecwall changed the title sql: add ability set or edit tenant capabilities sql: add ability set, edit, read tenant capabilities Nov 17, 2022
craig bot pushed a commit that referenced this issue Jan 19, 2023
95518: roachtest/awsdms: add no pk full load test case r=Jeremyyang920 a=Jeremyyang920

This commit adds a new test case that will attempt a full load on a table with no primary key. We will
assert that there is a table error with the full
load.

Fixes: #95328

Release note: None

95526: multitenant: append `WITH REPLICATION STATUS` columns to `SHOW TENANT` columns r=knz a=ecwall

Informs #87851

`WITH CAPABILITIES` will be adding another set of columns so append relevant columns instead of creating all possible column sets ahead of time.

Release note: None

95536: grunning: add grunning.Elapsed() r=irfansharif a=irfansharif

Elapsed returns the running time spent doing some piece of work, with grunning.Time() measurements from the start and end. This only exists due to grunning.Time()'s non-monotonicity, a bug in our runtime patch: #95529. We can get rid of this, keeping just grunning.Difference(), if that bug is fixed. The bug results in slight {over,under}-estimation of the running time (the latter breaking monotonicity), but is livable with our current uses of this library.

Release note: None

Co-authored-by: Jeremy Yang <[email protected]>
Co-authored-by: Evan Wall <[email protected]>
Co-authored-by: irfan sharif <[email protected]>
craig bot pushed a commit that referenced this issue Jan 25, 2023
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]>
@craig craig bot closed this as completed in 0b29509 Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-multitenancy Related to multi-tenancy A-sql-builtins SQL built-in functions and semantics thereof. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants