-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
settings: assert that SystemOnly settings are not accessed in virtual…
… clusters TLDR: tests will now fail (with a panic) if code running in virtual clusters mistakenly attempts to access a SystemOnly cluster setting. ---- Prior to this patch, it was possible for code (e.g. in SQL) running inside a virtual cluster to access a `SystemOnly` setting. This resulted in silently incorrect behavior: the default value of the setting would be observed always, without any relationship to values explicitly set in the storage layer. When the setting mechanisms for virtual clusters had been implemented orignally, this concern was known. We even had implemented a mechanism on `settings.Values` (`SetNonSystemTenant()`) by which we could cause further `.Get()` calls to fail with an error in tests if accessing a `SystemOnly` setting from a virtual cluster. Sadly, this mechanism was never hooked up anywhere. This patch closes that loop by annotating the `settings.Values` properly during server initialization. Since the property is stored inside the `settings.Values`, this means it is not any more possible to share a single `settings.Values` instance (nor `cluster.Settings`) between the system tenant and a secondary tenant. ---- The following bugs were fixed as a result of this change: - certain settings didn't have the right class, including: - kv.bulk_io_write.concurrent_export_requests - kv.closed_timestamp.follower_reads.enabled - kv.range_split.by_load_merge_delay - kv.rangefeed.enabled - sql.hash_sharded_range_pre_split.max - storage.max_sync_duration.fatal.enabled - storage.value_blocks.enabled - ui.display_timezone - kv.bulk_sst.target_size - the API endpoint that lists cluster settings was incorrectly trying to retrieve SystemOnly settings when accessed from a virtual cluster. Release note (sql change): It is not possible any more to control access to the RANGEFEED SQL syntax by modifying the cluster setting `kv.rangefeed.enabled`. Instead use `feature.changefeed.enabled`.
- Loading branch information
Showing
55 changed files
with
393 additions
and
182 deletions.
There are no files selected for viewing
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 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
Oops, something went wrong.