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: embed zone configs within descriptors objects #66396

Closed
irfansharif opened this issue Jun 12, 2021 · 5 comments
Closed

sql: embed zone configs within descriptors objects #66396

irfansharif opened this issue Jun 12, 2021 · 5 comments
Labels
A-zone-configs 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

@irfansharif
Copy link
Contributor

irfansharif commented Jun 12, 2021

Part of #66348. This will let us deprecate system.zones, and later provide the optimizer the cache of zone configs it needs for locality-aware planning. We'll also use embedded zone config to generate span configs for KV (#66391). We'll need to be careful around ensuring compatibility with existing backups here.

For the migration, we can first introduce a cluster version v21.1-A, which when rolled into, will disallow setting new zone configs (we'll later re-allow it after v21.1-B). We'll then recurse through all descriptors, and for each one copy over the corresponding zone config from system.zones into it.

Jira issue: CRDB-8024

@blathers-crl
Copy link

blathers-crl bot commented Jun 12, 2021

Hi @irfansharif, please add a C-ategory label to your issue. Check out the label system docs.

While you're here, please consider adding an A- label to help keep our repository tidy.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@irfansharif irfansharif added A-multitenancy Related to multi-tenancy A-zone-configs C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) labels Jun 12, 2021
@blathers-crl blathers-crl bot added the T-kv KV Team label Jun 1, 2022
@ajstorm
Copy link
Collaborator

ajstorm commented Jun 1, 2022

@irfansharif curious why this issue was labeled MT. Can you please comment?

@irfansharif
Copy link
Contributor Author

No good reason, this came out of the multi-tenant zone configs work so likely applied the label too liberally.

@irfansharif irfansharif removed the A-multitenancy Related to multi-tenancy label Jun 1, 2022
@blathers-crl blathers-crl bot added the T-sql-schema-deprecated Use T-sql-foundations instead label Jun 1, 2022
@irfansharif irfansharif removed the T-kv KV Team label Jun 1, 2022
@ajwerner
Copy link
Contributor

ajwerner commented Jun 7, 2022

This isn't something we have to do, and indeed there is no pressing reason to do it right now. It would be cleaner to associate this state more directly with the descriptors given we cache and write it in a correlated way. In reality, we'll probably more towards this when implementing transactional schema change. When we need to more generally encapsulate under a unified concept, this would be an important step.

@postamar
Copy link

I don't think we're going to do this. There exist other things-that-join-on-descriptors like comments, and we're going to be content to just have the descriptors collection mediate access to them.

@exalate-issue-sync exalate-issue-sync bot added T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) and removed T-sql-schema-deprecated Use T-sql-foundations instead labels May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-zone-configs 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

No branches or pull requests

5 participants