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

multitenant: re-evalutate SystemVisible cluster setting class #79223

Closed
stevendanna opened this issue Apr 1, 2022 · 2 comments
Closed

multitenant: re-evalutate SystemVisible cluster setting class #79223

stevendanna opened this issue Apr 1, 2022 · 2 comments
Labels
A-multitenancy Related to multi-tenancy C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-multitenant Issues owned by the multi-tenant virtual team

Comments

@stevendanna
Copy link
Collaborator

stevendanna commented Apr 1, 2022

Describe the problem

The early drafts of the multi-tenant cluster settings RFC (#73349) proposed a SystemVisible class of cluster settings. This class would be writable only from the system tenant but still be readable from other tenants.

The difference between SystemVisible and TenantReadOnly is that the value would only ever be read from the host tenant's value of the setting and it would not be overridable on a per tenant basis.

An example of where this might be useful are setting like kv.rangefeed.enabled where the host-tenant is responsible for changing behavior based on that setting, but where we also want to check the setting before creating changefeeds in order to provide a more useful error message.

Currently, the closest you can get to the behavior is to set the system cluster setting and also adding a tenant override that applies to all tenants. But, the tenants are then reading from a value that won't actually be used by the kvserver.

Jira issue: CRDB-14648

@stevendanna stevendanna added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Apr 1, 2022
@ajstorm ajstorm added the A-multitenancy Related to multi-tenancy label Apr 1, 2022
@ajwerner
Copy link
Contributor

ajwerner commented Apr 1, 2022

I feel like the solution here is actually to recast the problem as being poorly suited to cluster settings. In order to make sure that tenants could use rangefeeds for their system tables, we plumbed that into span configurations. That fact means that #70614 is pretty easy to realize. In fact, I think we could, if we wanted, make it such that the tenant's value of the cluster setting feeds into the spanconfig it reconciles. I feel like we should just scrap this issue and move rangefeed control into the zone config domain to wash our hands of this problem. More generally, cluster settings aren't the right framework for thinking about how replicas are configured from the perspective of the tenant. WDYT?

@stevendanna
Copy link
Collaborator Author

More generally, cluster settings aren't the right framework for thinking about how replicas are configured from the perspective of the tenant. WDYT?

I think that I agree with this.

In fact, I think we could, if we wanted, make it such that the tenant's value of the cluster setting feeds into the spanconfig it reconciles. I feel like we should just scrap this issue and move rangefeed control into the zone config domain to wash our hands of this problem.

I definitely think it would be cleaner to not rely on the cluster setting and if this is the only setting that needs this TenantReadOnly behavior then it makes sense to drop it as an idea.

@exalate-issue-sync exalate-issue-sync bot added the T-shared-systems Shared Systems Team label Jan 10, 2023
@knz knz added T-multitenant Issues owned by the multi-tenant virtual team and removed T-shared-systems Shared Systems Team labels Jun 30, 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 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-multitenant Issues owned by the multi-tenant virtual team
Projects
No open projects
Development

No branches or pull requests

5 participants