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/kv: how does multi-tenancy interact with zone configurations? #48375

Closed
nvanbenschoten opened this issue May 4, 2020 · 2 comments
Closed
Assignees
Labels
A-multitenancy Related to multi-tenancy

Comments

@nvanbenschoten
Copy link
Member

Up until now, we've been pretty handwavy around the interaction between mutli-tenancy and zone configurations. Our proposed split of the SQL keyspace would leave each tenant with its own Zone table, so it would be natural for each tenant to have their own set of zone configurations.

However, for this to work properly, the consumer of the zone configs needs to be made aware of SQL tenants and the distributed storage of their individual zone configurations. A KV range will need to know which Zone table to search in when determining the zone configs that apply to it. There would likely be a large amount of disruption that would fall out of this. For instance, config.ZoneConfigHook (and all of its uses) would need to become tenant-aware.

@nvanbenschoten nvanbenschoten self-assigned this May 4, 2020
nvanbenschoten added a commit to nvanbenschoten/cockroach that referenced this issue May 4, 2020
Informs cockroachdb#48123.

This commit continues with the plumbing that began an cockroachdb#48190. It pushes
a tenant-bound SQL codec into the other main source of key generation in
the SQL layer - descriptor manipulation and metadata handling. This
allows SQL tenants to properly handle metadata descriptors for its
database and tables.

This ended up being a larger undertaking than I had originally expected.
However, now that it's complete, we're in a pretty good spot:
1. `sqlbase.MetadataSchema` is ready to be used for cockroachdb#47904.
2. we can now run SQL migrations for a non-system tenant
3. there is only one remaining use of TODOSQLCodec in pkg/sql. See cockroachdb#48375.
@cockroachdb cockroachdb deleted a comment from blathers-crl bot May 4, 2020
@tbg
Copy link
Member

tbg commented May 5, 2020

I thought we were heavily leaning towards not giving tenants any say over zone configs, at least not in the foreseeable future (see here based not only on technical obstacles, but also because we may not want to hand this kind of control to users for some time.

We will want some way for the system tenant to manipulate zones for the tenant, but this is a much smaller undertaking without any low level work - we "only" need to add SQL syntax that generates zones for the right key spans (and the usual introspection machinery).

nvanbenschoten added a commit to nvanbenschoten/cockroach that referenced this issue May 5, 2020
Informs cockroachdb#48123.

This commit continues with the plumbing that began an cockroachdb#48190. It pushes
a tenant-bound SQL codec into the other main source of key generation in
the SQL layer - descriptor manipulation and metadata handling. This
allows SQL tenants to properly handle metadata descriptors for its
database and tables.

This ended up being a larger undertaking than I had originally expected.
However, now that it's complete, we're in a pretty good spot:
1. `sqlbase.MetadataSchema` is ready to be used for cockroachdb#47904.
2. we can now run SQL migrations for a non-system tenant
3. there is only one remaining use of TODOSQLCodec in pkg/sql. See cockroachdb#48375.
craig bot pushed a commit that referenced this issue May 5, 2020
48376: sql: push tenant-bound SQL codec into descriptor key generation r=nvanbenschoten a=nvanbenschoten

Informs #48123.

This commit continues with the plumbing that began an #48190. It pushes a tenant-bound SQL codec into the other main source of key generation in the SQL layer - descriptor manipulation and metadata handling. This allows SQL tenants to properly handle metadata descriptors for its database and tables.

This ended up being a larger undertaking than I had originally expected. However, now that it's complete, we're in a pretty good spot:
1. `sqlbase.MetadataSchema` is ready to be used for #47904.
2. we can now run SQL migrations for a non-system tenant.
3. there is only one remaining use of `keys.TODOSQLCodec` in pkg/sql. See #48375.

Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
Anzoteh96 pushed a commit to Anzoteh96/cockroach that referenced this issue May 5, 2020
48376: sql: push tenant-bound SQL codec into descriptor key generation r=nvanbenschoten a=nvanbenschoten

Informs cockroachdb#48123.

This commit continues with the plumbing that began an cockroachdb#48190. It pushes a tenant-bound SQL codec into the other main source of key generation in the SQL layer - descriptor manipulation and metadata handling. This allows SQL tenants to properly handle metadata descriptors for its database and tables.

This ended up being a larger undertaking than I had originally expected. However, now that it's complete, we're in a pretty good spot:
1. `sqlbase.MetadataSchema` is ready to be used for cockroachdb#47904.
2. we can now run SQL migrations for a non-system tenant.
3. there is only one remaining use of `keys.TODOSQLCodec` in pkg/sql. See cockroachdb#48375.

Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>

Release note (<category, see below>): <what> <show> <why>
nvanbenschoten added a commit to nvanbenschoten/cockroach that referenced this issue May 14, 2020
Relates to cockroachdb#48375.

Secondary tenants will not have a say over zone configurations, so
there's no reason to give them empty system.zones tables. This commit
addresses this by removing the table from secondary tenants. This allows
us to avoid populating this table with default zone configurations,
which would have been ignored.
@nvanbenschoten
Copy link
Member Author

Closing in favor of #49445.

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
Projects
None yet
Development

No branches or pull requests

2 participants