kv,changefeedccl: make kv.rangefeed.enabled a TenantReadOnly setting #79224
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This setting is used by kvserver and the value in the tenant does not matter. However, we do what to read the value from the tenant so we can generate nice error messages for CHANGEFEED users. Currently, the cluster setting system doesn't have a class of setting that lets us do this. However, by changing the setting to a TenantReadOnly setting, we can at least make it so that tenants do not have to set a no-op setting only to have their changefeeds still fail because of a host-level cluster setting. The downside is that an operator of a multi-tenant cluster will have to run two commands to enable rangefeeds for all tenants: ``` SET CLUSTER SETTING kv.rangefeed.enabled = true; ALTER TENANT ALL SET CLUSTER SETTING kv.rangefeed.enabled = true; ``` Release note: `kv.rangefeed.enabled` no longer needs to be set in a tenant in addition to the host cluster.
Since the goal of the demo command is to allow users to quickly explore features of the database, it makes sense to enable rangefeeds by default so that users can use changefeeds with little friction. Release note (cli change): The demo command now enables rangefeeds by default.
|
This change is |
|
I'm not sure this is the right trade-off. If we merge this we'll have to make sure that CC takes it into account when deploying a version that includes it since they'll have to add an override. I don't think we have many users hosting their own multi-tenant clusters, so perhaps that isn't the end of the world. I opened #79223 to track a possible change that would make this a bit better. |
|
Alternatively, rather than checking the cluster setting, perhaps we can add some kvserver API where we can ask whether we are able to run a rangefeed or not. |
miretskiy
approved these changes
Apr 1, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This setting is used by kvserver and the value in the tenant does not
matter. However, we do what to read the value from the tenant so we
can generate nice error messages for CHANGEFEED users.
Currently, the cluster setting system doesn't have a class of setting
that lets us do this. However, by changing the setting to a
TenantReadOnly setting, we can at least make it so that tenants do not
have to set a no-op setting only to have their changefeeds still fail
because of a host-level cluster setting.
The downside is that an operator of a multi-tenant cluster will have
to run two commands to enable rangefeeds for all tenants:
Release note:
kv.rangefeed.enabledno longer needs to be set in atenant in addition to the host cluster.