Merge 'config: mark "experimental" option unused and cleanups' from Kefu Chai

in this series, the "experimental" option is marked `Unused` as it has been marked deprecated for almost 2 years since scylla 4.6. and use `experimental_features` to specify the used experimental features explicitly.

Closes #14948

* github.com:scylladb/scylladb:
  config: remove unused namespace alias
  config: use std::ranges when appropriate
  config: drop "experimental" option
  test: disable 'enable_user_defined_functions' if experimental_features does not include udf
  test: pylib: specify experimental_features explicitly
This commit is contained in:
Avi Kivity
2023-08-17 20:42:02 +03:00
11 changed files with 24 additions and 33 deletions

View File

@@ -861,7 +861,7 @@ db::config::config(std::shared_ptr<db::extensions> exts)
, developer_mode(this, "developer_mode", value_status::Used, DEVELOPER_MODE_DEFAULT, "Relax environment checks. Setting to true can reduce performance and reliability significantly.") , developer_mode(this, "developer_mode", value_status::Used, DEVELOPER_MODE_DEFAULT, "Relax environment checks. Setting to true can reduce performance and reliability significantly.")
, skip_wait_for_gossip_to_settle(this, "skip_wait_for_gossip_to_settle", value_status::Used, -1, "An integer to configure the wait for gossip to settle. -1: wait normally, 0: do not wait at all, n: wait for at most n polls. Same as -Dcassandra.skip_wait_for_gossip_to_settle in cassandra.") , skip_wait_for_gossip_to_settle(this, "skip_wait_for_gossip_to_settle", value_status::Used, -1, "An integer to configure the wait for gossip to settle. -1: wait normally, 0: do not wait at all, n: wait for at most n polls. Same as -Dcassandra.skip_wait_for_gossip_to_settle in cassandra.")
, force_gossip_generation(this, "force_gossip_generation", liveness::LiveUpdate, value_status::Used, -1 , "Force gossip to use the generation number provided by user") , force_gossip_generation(this, "force_gossip_generation", liveness::LiveUpdate, value_status::Used, -1 , "Force gossip to use the generation number provided by user")
, experimental(this, "experimental", value_status::Used, false, "[Deprecated] Set to true to unlock all experimental features (except 'consistent-topology-changes' feature, which should be enabled explicitly via 'experimental-features' option). Please use 'experimental-features', instead.") , experimental(this, "experimental", value_status::Unused, false, "[Deprecated] Set to true to unlock all experimental features (except 'consistent-topology-changes' feature, which should be enabled explicitly via 'experimental-features' option). Please use 'experimental-features', instead.")
, experimental_features(this, "experimental_features", value_status::Used, {}, experimental_features_help_string()) , experimental_features(this, "experimental_features", value_status::Used, {}, experimental_features_help_string())
, lsa_reclamation_step(this, "lsa_reclamation_step", value_status::Used, 1, "Minimum number of segments to reclaim in a single step") , lsa_reclamation_step(this, "lsa_reclamation_step", value_status::Used, 1, "Minimum number of segments to reclaim in a single step")
, prometheus_port(this, "prometheus_port", value_status::Used, 9180, "Prometheus port, set to zero to disable") , prometheus_port(this, "prometheus_port", value_status::Used, 9180, "Prometheus port, set to zero to disable")
@@ -1148,19 +1148,13 @@ db::fs::path db::config::get_conf_sub(db::fs::path sub) {
} }
bool db::config::check_experimental(experimental_features_t::feature f) const { bool db::config::check_experimental(experimental_features_t::feature f) const {
if (experimental() enum_option<experimental_features_t> to_check{f};
&& f != experimental_features_t::feature::UNUSED return std::ranges::any_of(experimental_features(),
&& f != experimental_features_t::feature::CONSISTENT_TOPOLOGY_CHANGES [to_check](auto& enabled) {
&& f != experimental_features_t::feature::BROADCAST_TABLES return to_check == enabled;
&& f != experimental_features_t::feature::TABLETS) { });
return true;
}
const auto& optval = experimental_features();
return find(begin(optval), end(optval), enum_option<experimental_features_t>{f}) != end(optval);
} }
namespace bpo = boost::program_options;
logging::settings db::config::logging_settings(const log_cli::options& opts) const { logging::settings db::config::logging_settings(const log_cli::options& opts) const {
auto value = [&](auto& v, const auto& opt) { auto value = [&](auto& v, const auto& opt) {
if (opt.defaulted() && v.is_set()) { if (opt.defaulted() && v.is_set()) {

View File

@@ -97,8 +97,6 @@ namespace db {
/// Enumeration of all valid values for the `experimental` config entry. /// Enumeration of all valid values for the `experimental` config entry.
struct experimental_features_t { struct experimental_features_t {
// NOTE: CONSISTENT_TOPOLOGY_CHANGES and BROADCAST_TABLES features are not enabled via `experimental` umbrella flag.
// These options should be enabled explicitly.
enum class feature { enum class feature {
UNUSED, UNUSED,
UDF, UDF,

View File

@@ -1035,18 +1035,6 @@ SEASTAR_TEST_CASE(test_parse_experimental_features_invalid) {
return make_ready_future(); return make_ready_future();
} }
SEASTAR_TEST_CASE(test_parse_experimental_true) {
auto cfg_ptr = std::make_unique<config>();
config& cfg = *cfg_ptr;
cfg.read_from_yaml("experimental: true", throw_on_error);
BOOST_CHECK(!cfg.check_experimental(ef::UNUSED));
BOOST_CHECK(cfg.check_experimental(ef::UDF));
BOOST_CHECK(cfg.check_experimental(ef::ALTERNATOR_STREAMS));
BOOST_CHECK(!cfg.check_experimental(ef::CONSISTENT_TOPOLOGY_CHANGES));
BOOST_CHECK(cfg.check_experimental(ef::KEYSPACE_STORAGE_OPTIONS));
return make_ready_future();
}
SEASTAR_TEST_CASE(test_parse_experimental_false) { SEASTAR_TEST_CASE(test_parse_experimental_false) {
auto cfg_ptr = std::make_unique<config>(); auto cfg_ptr = std::make_unique<config>();
config& cfg = *cfg_ptr; config& cfg = *cfg_ptr;

View File

@@ -1,2 +1,5 @@
type: Python type: Python
extra_scylla_cmdline_options: ["--consistent-cluster-management=true", "--experimental-features=broadcast-tables"] extra_scylla_cmdline_options:
- "--consistent-cluster-management=true"
- "--experimental-features=broadcast-tables"
- "--enable-user-defined-functions=false"

View File

@@ -71,8 +71,12 @@ def make_scylla_conf(workdir: pathlib.Path, host_addr: str, seed_addrs: List[str
# Allow testing experimental features. Following issue #9467, we need # Allow testing experimental features. Following issue #9467, we need
# to add here specific experimental features as they are introduced. # to add here specific experimental features as they are introduced.
'enable_user_defined_functions': True, 'enable_user_defined_functions': True,
'experimental': True, 'experimental_features': ['udf',
'experimental_features': ['udf', 'consistent-topology-changes'], 'alternator-streams',
'consistent-topology-changes',
'broadcast-tables',
'keyspace-storage-options',
'tablets'],
'consistent_cluster_management': True, 'consistent_cluster_management': True,

View File

@@ -21,7 +21,8 @@ async def test_boot_after_ip_change(manager: ManagerClient) -> None:
"""Bootstrap a new node after existing one changed its IP. """Bootstrap a new node after existing one changed its IP.
Regression test for #14468. Does not apply to Raft-topology mode. Regression test for #14468. Does not apply to Raft-topology mode.
""" """
cfg = {'experimental_features': list[str]()} cfg = {'enable_user_defined_functions': False,
'experimental_features': list[str]()}
logger.info(f"Booting initial cluster") logger.info(f"Booting initial cluster")
servers = [await manager.server_add(config=cfg) for _ in range(2)] servers = [await manager.server_add(config=cfg) for _ in range(2)]
await wait_for_token_ring_and_group0_consistency(manager, time.time() + 30) await wait_for_token_ring_and_group0_consistency(manager, time.time() + 30)

View File

@@ -25,7 +25,8 @@ async def test_replace_ignore_nodes(manager: ManagerClient) -> None:
we don't want to run it in debug mode. we don't want to run it in debug mode.
Preferably run it only in one mode e.g. dev. Preferably run it only in one mode e.g. dev.
""" """
cfg = {'experimental_features': list[str]()} cfg = {'enable_user_defined_functions': False,
'experimental_features': list[str]()}
logger.info(f"Booting initial cluster") logger.info(f"Booting initial cluster")
servers = [await manager.server_add(config=cfg) for _ in range(7)] servers = [await manager.server_add(config=cfg) for _ in range(7)]
s2_id = await manager.get_host_id(servers[2].server_id) s2_id = await manager.get_host_id(servers[2].server_id)

View File

@@ -24,7 +24,8 @@ async def test_remove_garbage_group0_members(manager: ManagerClient):
even though the node is no longer a token ring member. Does not apply to Raft-topology mode. even though the node is no longer a token ring member. Does not apply to Raft-topology mode.
""" """
# 4 servers, one dead # 4 servers, one dead
cfg = {'experimental_features': list[str]()} cfg = {'enable_user_defined_functions': False,
'experimental_features': list[str]()}
servers = [await manager.server_add(config=cfg) for _ in range(4)] servers = [await manager.server_add(config=cfg) for _ in range(4)]
removed_host_id = await manager.get_host_id(servers[0].server_id) removed_host_id = await manager.get_host_id(servers[0].server_id)
await manager.server_stop_gracefully(servers[0].server_id) await manager.server_stop_gracefully(servers[0].server_id)

View File

@@ -5,6 +5,7 @@ cluster:
extra_scylla_config_options: extra_scylla_config_options:
authenticator: AllowAllAuthenticator authenticator: AllowAllAuthenticator
authorizer: AllowAllAuthorizer authorizer: AllowAllAuthorizer
enable_user_defined_functions: False
experimental_features: ['consistent-topology-changes', 'tablets'] experimental_features: ['consistent-topology-changes', 'tablets']
skip_in_release: skip_in_release:
- test_blocked_bootstrap - test_blocked_bootstrap

View File

@@ -5,6 +5,7 @@ cluster:
extra_scylla_config_options: extra_scylla_config_options:
authenticator: AllowAllAuthenticator authenticator: AllowAllAuthenticator
authorizer: AllowAllAuthorizer authorizer: AllowAllAuthorizer
enable_user_defined_functions: False
experimental_features: [] experimental_features: []
consistent_cluster_management: False consistent_cluster_management: False
run_first: run_first:

View File

@@ -564,7 +564,6 @@ future<schema_ptr> load_one_schema_from_file(std::filesystem::path path) {
schema_ptr load_system_schema(std::string_view keyspace, std::string_view table) { schema_ptr load_system_schema(std::string_view keyspace, std::string_view table) {
db::config cfg; db::config cfg;
cfg.experimental.set(true);
cfg.experimental_features.set(db::experimental_features_t::all()); cfg.experimental_features.set(db::experimental_features_t::all());
const std::unordered_map<std::string_view, std::vector<schema_ptr>> schemas{ const std::unordered_map<std::string_view, std::vector<schema_ptr>> schemas{
{db::schema_tables::NAME, db::schema_tables::all_tables(db::schema_features::full())}, {db::schema_tables::NAME, db::schema_tables::all_tables(db::schema_features::full())},