diff --git a/db/config.cc b/db/config.cc index d5f009088c..c10e1c7bff 100644 --- a/db/config.cc +++ b/db/config.cc @@ -253,7 +253,7 @@ static db::tri_mode_restriction_t::mode strict_allow_filtering_default() { static std::vector experimental_feature_names() { std::vector ret; for (const auto& f : db::experimental_features_t::map()) { - if (f.second != db::experimental_features_t::UNUSED) { + if (f.second != db::experimental_features_t::feature::UNUSED) { ret.push_back(f.first); } } @@ -1009,8 +1009,8 @@ db::fs::path db::config::get_conf_sub(db::fs::path sub) { bool db::config::check_experimental(experimental_features_t::feature f) const { if (experimental() - && f != experimental_features_t::UNUSED - && f != experimental_features_t::RAFT) { + && f != experimental_features_t::feature::UNUSED + && f != experimental_features_t::feature::RAFT) { return true; } const auto& optval = experimental_features(); @@ -1047,20 +1047,20 @@ std::unordered_map db::experiment // to UNUSED switch for a while, and can be eventually // removed altogether. return { - {"lwt", UNUSED}, - {"udf", UDF}, - {"cdc", UNUSED}, - {"alternator-streams", ALTERNATOR_STREAMS}, - {"alternator-ttl", ALTERNATOR_TTL}, - {"raft", RAFT}, - {"keyspace-storage-options", KEYSPACE_STORAGE_OPTIONS}, + {"lwt", feature::UNUSED}, + {"udf", feature::UDF}, + {"cdc", feature::UNUSED}, + {"alternator-streams", feature::ALTERNATOR_STREAMS}, + {"alternator-ttl", feature::ALTERNATOR_TTL}, + {"raft", feature::RAFT}, + {"keyspace-storage-options", feature::KEYSPACE_STORAGE_OPTIONS}, }; } std::vector> db::experimental_features_t::all() { std::vector> ret; for (const auto& f : db::experimental_features_t::map()) { - if (f.second != db::experimental_features_t::UNUSED) { + if (f.second != db::experimental_features_t::feature::UNUSED) { ret.push_back(f.second); } } diff --git a/db/config.hh b/db/config.hh index d63a11c390..3e59325ec5 100644 --- a/db/config.hh +++ b/db/config.hh @@ -83,7 +83,7 @@ namespace db { struct experimental_features_t { // NOTE: RAFT feature is not enabled via `experimental` umbrella flag. // This option should be enabled explicitly. - enum feature { UNUSED, UDF, ALTERNATOR_STREAMS, ALTERNATOR_TTL, RAFT, + enum class feature { UNUSED, UDF, ALTERNATOR_STREAMS, ALTERNATOR_TTL, RAFT, KEYSPACE_STORAGE_OPTIONS }; static std::unordered_map map(); // See enum_option. static std::vector> all(); diff --git a/db/system_keyspace.cc b/db/system_keyspace.cc index bbf9156de0..f513fc036a 100644 --- a/db/system_keyspace.cc +++ b/db/system_keyspace.cc @@ -2685,7 +2685,7 @@ std::vector system_keyspace::all_tables(const db::config& cfg) { v3::truncated(), v3::cdc_local(), }); - if (cfg.check_experimental(db::experimental_features_t::RAFT)) { + if (cfg.check_experimental(db::experimental_features_t::feature::RAFT)) { r.insert(r.end(), {raft(), raft_snapshots(), raft_config(), group0_history(), discovery()}); } // legacy schema diff --git a/gms/feature_service.cc b/gms/feature_service.cc index a17e852f31..ec3aa1d2a7 100644 --- a/gms/feature_service.cc +++ b/gms/feature_service.cc @@ -50,20 +50,20 @@ feature_config feature_config_from_db_config(db::config& cfg, std::set if (!cfg.enable_user_defined_functions()) { fcfg._disabled_features.insert("UDF"); } else { - if (!cfg.check_experimental(db::experimental_features_t::UDF)) { + if (!cfg.check_experimental(db::experimental_features_t::feature::UDF)) { throw std::runtime_error( "You must use both enable_user_defined_functions and experimental_features:udf " "to enable user-defined functions"); } } - if (!cfg.check_experimental(db::experimental_features_t::ALTERNATOR_STREAMS)) { + if (!cfg.check_experimental(db::experimental_features_t::feature::ALTERNATOR_STREAMS)) { fcfg._disabled_features.insert("ALTERNATOR_STREAMS"s); } - if (!cfg.check_experimental(db::experimental_features_t::ALTERNATOR_TTL)) { + if (!cfg.check_experimental(db::experimental_features_t::feature::ALTERNATOR_TTL)) { fcfg._disabled_features.insert("ALTERNATOR_TTL"s); } - if (!cfg.check_experimental(db::experimental_features_t::RAFT)) { + if (!cfg.check_experimental(db::experimental_features_t::feature::RAFT)) { fcfg._disabled_features.insert("SUPPORTS_RAFT_CLUSTER_MANAGEMENT"s); fcfg._disabled_features.insert("USES_RAFT_CLUSTER_MANAGEMENT"s); } else { @@ -73,7 +73,7 @@ feature_config feature_config_from_db_config(db::config& cfg, std::set // advertised via gossip ahead of time. fcfg._masked_features.insert("USES_RAFT_CLUSTER_MANAGEMENT"s); } - if (!cfg.check_experimental(db::experimental_features_t::KEYSPACE_STORAGE_OPTIONS)) { + if (!cfg.check_experimental(db::experimental_features_t::feature::KEYSPACE_STORAGE_OPTIONS)) { fcfg._disabled_features.insert("KEYSPACE_STORAGE_OPTIONS"s); } diff --git a/main.cc b/main.cc index 898bd3d5dc..b8f9fb36f1 100644 --- a/main.cc +++ b/main.cc @@ -1012,7 +1012,7 @@ To start the scylla server proper, simply invoke as: scylla server (or just scyl // #293 - do not stop anything // engine().at_exit([&proxy] { return proxy.stop(); }); - raft_gr.start(cfg->check_experimental(db::experimental_features_t::RAFT), + raft_gr.start(cfg->check_experimental(db::experimental_features_t::feature::RAFT), std::ref(messaging), std::ref(gossiper), std::ref(fd)).get(); // gropu0 client exists only on shard 0 @@ -1033,7 +1033,7 @@ To start the scylla server proper, simply invoke as: scylla server (or just scyl auto stop_raft = defer_verbose_shutdown("Raft", [&raft_gr] { raft_gr.stop().get(); }); - if (cfg->check_experimental(db::experimental_features_t::RAFT)) { + if (cfg->check_experimental(db::experimental_features_t::feature::RAFT)) { supervisor::notify("starting Raft Group Registry service"); } raft_gr.invoke_on_all(&service::raft_group_registry::start).get(); @@ -1543,7 +1543,7 @@ To start the scylla server proper, simply invoke as: scylla server (or just scyl // only Alternator uses it for its TTL feature. But in the // future if we add a CQL interface to it, we may want to // start this outside the Alternator if(). - if (cfg->check_experimental(db::experimental_features_t::ALTERNATOR_TTL)) { + if (cfg->check_experimental(db::experimental_features_t::feature::ALTERNATOR_TTL)) { supervisor::notify("starting the expiration service"); es.start(seastar::sharded_parameter([] (const replica::database& db) { return db.as_data_dictionary(); }, std::ref(db)), std::ref(proxy)).get(); diff --git a/service/storage_service.cc b/service/storage_service.cc index 709b1ccb4c..2e4b7a9bf4 100644 --- a/service/storage_service.cc +++ b/service/storage_service.cc @@ -429,7 +429,7 @@ future<> storage_service::join_token_ring(cdc::generation_service& cdc_gen_servi // - it's a fresh node start (in a fresh cluster) // - it's a restart of an existing node, which have already joined some group0 const bool can_join_with_raft = - _db.local().get_config().check_experimental(db::experimental_features_t::RAFT) && ( + _db.local().get_config().check_experimental(db::experimental_features_t::feature::RAFT) && ( _sys_ks.local().bootstrap_needed() || !(co_await _sys_ks.local().get_raft_group0_id()).is_null()); if (can_join_with_raft) { diff --git a/test/boost/config_test.cc b/test/boost/config_test.cc index 875ea3af05..050b6ab31b 100644 --- a/test/boost/config_test.cc +++ b/test/boost/config_test.cc @@ -911,8 +911,8 @@ SEASTAR_TEST_CASE(test_parse_broken) { return make_ready_future<>(); } -using ef = experimental_features_t; -using features = std::vector>; +using ef = experimental_features_t::feature; +using features = std::vector>; SEASTAR_TEST_CASE(test_parse_experimental_features_cdc) { auto cfg_ptr = std::make_unique(); diff --git a/test/boost/schema_change_test.cc b/test/boost/schema_change_test.cc index 3b52f159ac..c3cf9bc751 100644 --- a/test/boost/schema_change_test.cc +++ b/test/boost/schema_change_test.cc @@ -640,7 +640,7 @@ future<> test_schema_digest_does_not_change_with_disabled_features(sstring data_ auto db_cfg_ptr = ::make_shared(std::move(extensions)); auto& db_cfg = *db_cfg_ptr; db_cfg.enable_user_defined_functions({true}, db::config::config_source::CommandLine); - db_cfg.experimental_features({experimental_features_t::UDF, experimental_features_t::KEYSPACE_STORAGE_OPTIONS}, db::config::config_source::CommandLine); + db_cfg.experimental_features({experimental_features_t::feature::UDF, experimental_features_t::feature::KEYSPACE_STORAGE_OPTIONS}, db::config::config_source::CommandLine); if (regenerate) { db_cfg.data_file_directories({data_dir}, db::config::config_source::CommandLine); } else { diff --git a/test/boost/user_function_test.cc b/test/boost/user_function_test.cc index f984a86a88..b6844d149b 100644 --- a/test/boost/user_function_test.cc +++ b/test/boost/user_function_test.cc @@ -55,7 +55,7 @@ static future<> with_udf_enabled(Func&& func) { // Raise timeout to survive debug mode and contention, but keep in // mind that some tests expect timeout. db_cfg.user_defined_function_time_limit_ms(1000); - db_cfg.experimental_features({db::experimental_features_t::UDF}, db::config::config_source::CommandLine); + db_cfg.experimental_features({db::experimental_features_t::feature::UDF}, db::config::config_source::CommandLine); return do_with_cql_env_thread(std::forward(func), db_cfg_ptr); } @@ -985,7 +985,7 @@ SEASTAR_THREAD_TEST_CASE(test_user_function_db_init) { db_cfg.data_file_directories({data_dir.path().string()}, db::config::config_source::CommandLine); db_cfg.enable_user_defined_functions({true}, db::config::config_source::CommandLine); - db_cfg.experimental_features({db::experimental_features_t::UDF}, db::config::config_source::CommandLine); + db_cfg.experimental_features({db::experimental_features_t::feature::UDF}, db::config::config_source::CommandLine); do_with_cql_env_thread([] (cql_test_env& e) { e.execute_cql("CREATE FUNCTION my_func(a int, b float) CALLED ON NULL INPUT RETURNS int LANGUAGE Lua AS 'return 2';").get(); diff --git a/test/cql-pytest/test_virtual_tables.py b/test/cql-pytest/test_virtual_tables.py index ef29981a95..919e634b7b 100644 --- a/test/cql-pytest/test_virtual_tables.py +++ b/test/cql-pytest/test_virtual_tables.py @@ -59,7 +59,8 @@ def test_versions(scylla_only, cql): # parameters. As we noticed in issue #10047, each type of configuration # parameter can have a different function for printing it out, and some of # those may be wrong so we want to check as many as we can - including -# specifically the experimental_features option which was wrong in #10047. +# specifically the experimental_features option which was wrong in #10047 +# and #11003. def test_system_config_read(scylla_only, cql): # All rows should have the columns name, source, type and value: rows = list(cql.execute("SELECT name, source, type, value FROM system.config")) @@ -68,12 +69,14 @@ def test_system_config_read(scylla_only, cql): values[row.name] = row.value # Check that experimental_features exists and makes sense. # It needs to be a JSON-formatted strings, and the strings need to be - # ASCII feature names - not binary garbage as it was in #10047. + # ASCII feature names - not binary garbage as it was in #10047, + # and not numbers-formatted-as-string as in #11003. assert 'experimental_features' in values obj = json.loads(values['experimental_features']) assert isinstance(obj, list) assert isinstance(obj[0], str) assert obj[0] and obj[0].isascii() and obj[0].isprintable() + assert not obj[0].isnumeric() # issue #11003 # Check formatting of tri_mode_restriction like # restrict_replication_simplestrategy. These need to be one of # allowed string values 0, 1, true, false or warn - but in particular diff --git a/test/lib/cql_test_env.cc b/test/lib/cql_test_env.cc index a7d0ec1faf..aab7f7d8b7 100644 --- a/test/lib/cql_test_env.cc +++ b/test/lib/cql_test_env.cc @@ -616,7 +616,7 @@ public: fd.stop().get(); }); - raft_gr.start(cfg->check_experimental(db::experimental_features_t::RAFT), + raft_gr.start(cfg->check_experimental(db::experimental_features_t::feature::RAFT), std::ref(ms), std::ref(gossiper), std::ref(fd)).get(); auto stop_raft_gr = deferred_stop(raft_gr); raft_gr.invoke_on_all(&service::raft_group_registry::start).get(); @@ -915,7 +915,7 @@ reader_permit make_reader_permit(cql_test_env& env) { cql_test_config raft_cql_test_config() { cql_test_config c; - c.db_config->experimental_features({db::experimental_features_t::RAFT}); + c.db_config->experimental_features({db::experimental_features_t::feature::RAFT}); return c; } diff --git a/utils/enum_option.hh b/utils/enum_option.hh index c52ccebb1b..1ca5902a92 100644 --- a/utils/enum_option.hh +++ b/utils/enum_option.hh @@ -44,11 +44,11 @@ concept HasMapInterface = requires(T t) { /// Example: /// /// struct Type { -/// enum ty { a1, a2, b1 }; +/// enum class ty { a1, a2, b1 }; /// static unordered_map map(); /// }; /// unordered_map Type::map() { -/// return {{"a1", Type::a1}, {"a2", Type::a2}, {"b1", Type::b1}}; +/// return {{"a1", Type::ty::a1}, {"a2", Type::ty::a2}, {"b1", Type::ty::b1}}; /// } /// int main(int ac, char* av[]) { /// namespace po = boost::program_options;