kvserver: make some cluster settings system only #98353
Merged
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
|
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
|
This change is |
kvoli
force-pushed
the
230309.kvserver-system-only
branch
4 times, most recently
from
22b5ed1
to
423314f
Compare
kvoli
changed the title
kvoli
force-pushed
the
230309.kvserver-system-only
branch
2 times, most recently
from
3b41e9a
to
7ee7b06
Compare
kvoli
force-pushed
the
230309.kvserver-system-only
branch
6 times, most recently
from
6a02d2c
to
cc17d78
Compare
kvoli
changed the title
kvoli
force-pushed
the
230309.kvserver-system-only
branch
4 times, most recently
from
d1662f7
to
16c3c17
Compare
andrewbaptist
approved these changes
Mar 20, 2023
Update cluster settings in the `kv/kvserver` pkg to be `SystemOnly`. Previously, there were many cluster settings which which were `TenantWritable` or `TenantReadOnly`. These settings, even if altered by a tenant have no effect. There are settings which are not updated, due due to tests relying on modifying the setting value using a non-system tenant. We ignore these in this commit and defer to cockroachdb#98723 for handling these. These settings are updated to be `SystemOnly`: ``` kv.bulk_io_write.max_rate kv.bulk_sst.max_allowed_overage kv.bulk_sst.target_size kv.closed_timestamp.follower_reads_enabled kv.log_range_and_node_events.enabled kv.range_split.by_load_enabled kv.range_split.load_cpu_threshold kv.range_split.load_qps_threshold kv.replica_stats.addsst_request_size_factor kv.replication_reports.interval server.shutdown.lease_transfer_wait ``` Resolves: cockroachdb#98347 Release note (ops change): Some KV server cluster settings are now system only. These settings could previously be written or read by tenants, however writing to these settings had no effect.
kvoli
force-pushed
the
230309.kvserver-system-only
branch
from
16c3c17
to
7503b1a
Compare
|
bors r=andrewbaptist |
|
Build failed (retrying...): |
|
Build failed (retrying...): |
|
Build failed (retrying...): |
|
Build succeeded: |
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.
Update cluster settings in the
kv/kvserverpkg to beSystemOnly.Previously, there were many cluster settings which which were
TenantWritableorTenantReadOnly. These settings, even if alteredby a tenant have no effect.
There are settings which are not updated, due due to tests relying on
modifying the setting value using a non-system tenant. We ignore these
in this commit and defer to #98723 for handling these.
These settings are updated to be
SystemOnly:Resolves: #98347
Release note (ops change): Some KV server cluster settings are now system
only. These settings could previously be written or read by
tenants, however writing to these settings had no effect.