[QUEUED scylla next] config: Add `experimental_features` option

[Commit Bot]

From: Dejan Mircevski <de...@scylladb.com>
Committer: Dejan Mircevski <de...@scylladb.com>
Branch: next

config: Add `experimental_features` option

When the user wants to turn on only some experimental features, they
can use this new option. The existing `experimental` option is
preserved for backwards compatibility.

Signed-off-by: Dejan Mircevski <de...@scylladb.com>

---
diff --git a/conf/scylla.yaml b/conf/scylla.yaml
--- a/conf/scylla.yaml
+++ b/conf/scylla.yaml
@@ -245,7 +245,10 @@ batch_size_fail_threshold_in_kb: 50
# broadcast_rpc_address: 1.2.3.4

# Uncomment to enable experimental features
-# experimental: true
+# experimental_features:
+# - cdc
+# - lwt
+# - udf

# The directory where hints files are stored if hinted handoff is enabled.
# hints_directory: /var/lib/scylla/hints
diff --git a/cql3/statements/function_statement.cc
b/cql3/statements/function_statement.cc
--- a/cql3/statements/function_statement.cc
+++ b/cql3/statements/function_statement.cc
@@ -68,7 +68,7 @@ data_type
function_statement::prepare_type(service::storage_proxy& proxy, cql3_t

void function_statement::create_arg_types(service::storage_proxy& proxy)
const {
if
(!service::get_local_storage_service().cluster_supports_user_defined_functions())
{
- throw exceptions::invalid_request_exception("User defined
functions are disabled. Set enable_user_defined_functions to enable them");
+ throw exceptions::invalid_request_exception("User defined
functions are disabled. Set enable_user_defined_functions and
experimental_features:udf to enable them");
}

if (_arg_types.empty()) {
diff --git a/db/config.cc b/db/config.cc
--- a/db/config.cc
+++ b/db/config.cc
@@ -22,6 +22,7 @@

#include <unordered_map>
#include <regex>
+#include <sstream>

#include <boost/any.hpp>
#include <boost/program_options.hpp>
@@ -108,6 +109,10 @@ const config_type config_type_for<int32_t> =
config_type("integer", value_to_jso
template <>
const config_type config_type_for<db::seed_provider_type> =
config_type("seed provider", seed_provider_to_json);

+template <>
+const config_type
config_type_for<std::vector<enum_option<db::experimental_features_t>>> =
config_type(
+ "experimental features", value_to_json<std::vector<sstring>>);
+
}

namespace YAML {
@@ -153,6 +158,23 @@ struct convert<db::config::seed_provider_type> {
}
};

+template <>
+class convert<enum_option<db::experimental_features_t>> {
+public:
+ static bool decode(const Node& node,
enum_option<db::experimental_features_t>& rhs) {
+ std::string name;
+ if (!convert<std::string>::decode(node, name)) {
+ return false;
+ }
+ try {
+ std::istringstream(name) >> rhs;
+ } catch (boost::program_options::invalid_option_value&) {
+ return false;
+ }
+ return true;
+ }
+};
+
}

#if defined(DEBUG)
@@ -671,7 +693,8 @@ db::config::config(std::shared_ptr<db::extensions> exts)
, shutdown_announce_in_ms(this, "shutdown_announce_in_ms",
value_status::Used, 2 * 1000, "Time a node waits after sending gossip
shutdown message in milliseconds. Same as
-Dcassandra.shutdown_announce_in_ms in cassandra.")
, developer_mode(this, "developer_mode", value_status::Used,
false, "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.")
- , experimental(this, "experimental", value_status::Used, false, "Set
to true to unlock experimental features.")
+ , experimental(this, "experimental", value_status::Used, false, "Set
to true to unlock all experimental features.")
+ , experimental_features(this, "experimental_features",
value_status::Used, {}, "Unlock experimental features provided as the
option arguments (possible values: 'lwt', 'cdc', 'udf'). Can be repeated.")
, 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_address(this, "prometheus_address",
value_status::Used, "0.0.0.0", "Prometheus listening address")
@@ -706,7 +729,7 @@ db::config::config(std::shared_ptr<db::extensions> exts)
"Set to true if the cluster was initially installed from 3.1.0. If
it was upgraded from an earlier version,"
" or installed from a later version, leave this set to false. This
adjusts the communication protocol to"
" work around a bug in Scylla 3.1.0")
- , enable_user_defined_functions(this, "enable_user_defined_functions",
value_status::Used, false, "Enable user defined functions")
+ , enable_user_defined_functions(this, "enable_user_defined_functions",
value_status::Used, false, "Enable user defined functions. You must also
set experimental-features=udf")
,
user_defined_function_time_limit_ms(this, "user_defined_function_time_limit_ms",
value_status::Used, 10, "The time limit for each UDF invocation")
,
user_defined_function_allocation_limit_bytes(this, "user_defined_function_allocation_limit_bytes",
value_status::Used, 1024*1024, "How much memory each UDF invocation can
allocate")
,
user_defined_function_contiguous_allocation_limit_bytes(this, "user_defined_function_contiguous_allocation_limit_bytes",
value_status::Used, 1024*1024, "How much memory each UDF invocation can
allocate in one chunk")
@@ -827,10 +850,12 @@ db::fs::path db::config::get_conf_sub(db::fs::path
sub) {
return get_conf_dir() / sub;
}

-void db::config::check_experimental(const sstring& what) const {
- if (!experimental()) {
- throw std::runtime_error(format("{} is currently disabled. Start
Scylla with --experimental=on to enable.", what));
+bool db::config::check_experimental(experimental_features_t::feature f)
const {
+ if (experimental()) {
+ 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;
@@ -875,6 +900,12 @@ const db::extensions& db::config::extensions() const {
return *_extensions;
}

+std::unordered_map<sstring, db::experimental_features_t::feature>
db::experimental_features_t::map() {
+ // We decided against using the construct-on-first-use idiom here:
+ // https://github.com/scylladb/scylla/pull/5369#discussion_r353614807
+ return {{"lwt", LWT}, {"udf", UDF}, {"cdc", CDC}};
+}
+
template struct utils::config_file::named_value<seastar::log_level>;

namespace utils {
diff --git a/db/config.hh b/db/config.hh
--- a/db/config.hh
+++ b/db/config.hh
@@ -32,6 +32,7 @@

#include "seastarx.hh"
#include "utils/config_file.hh"
+#include "utils/enum_option.hh"

namespace seastar { class file; struct logging_settings; }

@@ -74,14 +75,21 @@ sstring config_value_as_json(const
std::unordered_map<sstring, log_level>& v);

namespace db {

+/// Enumeration of all valid values for the `experimental` config entry.
+struct experimental_features_t {
+ enum feature { LWT, UDF, CDC };
+ static std::unordered_map<sstring, feature> map(); // See enum_option.
+};
+
class config : public utils::config_file {
public:
config();
config(std::shared_ptr<db::extensions>);
~config();

- // Throws exception if experimental feature is disabled.
- void check_experimental(const sstring& what) const;
+ /// True iff the feature is enabled.
+ bool check_experimental(experimental_features_t::feature f) const;
+
void setup_directories();

/**
@@ -265,6 +273,7 @@ public:
named_value<bool> developer_mode;
named_value<int32_t> skip_wait_for_gossip_to_settle;
named_value<bool> experimental;
+ named_value<std::vector<enum_option<experimental_features_t>>>
experimental_features;
named_value<size_t> lsa_reclamation_step;
named_value<uint16_t> prometheus_port;
named_value<sstring> prometheus_address;
diff --git a/service/storage_proxy.cc b/service/storage_proxy.cc
--- a/service/storage_proxy.cc
+++ b/service/storage_proxy.cc
@@ -737,7 +737,9 @@ static
future<std::optional<paxos_response_handler::ballot_and_data>> sleep_and_
*/
future<paxos_response_handler::ballot_and_data>
paxos_response_handler::begin_and_repair_paxos(client_state& cs, unsigned&
contentions, bool is_write) {
- _proxy->get_db().local().get_config().check_experimental("Paxos");
+ if
(!_proxy->get_db().local().get_config().check_experimental(db::experimental_features_t::LWT))
{
+ throw std::runtime_error("Paxos is currently disabled. Start
Scylla with --experimental_features=lwt to enable.");
+ }
return do_with(api::timestamp_type(0), shared_from_this(), [this, &cs,
&contentions, is_write]
(api::timestamp_type& min_timestamp_micros_to_use,
shared_ptr<paxos_response_handler>& prh) {
return repeat_until_value([this, &contentions, &cs,
&min_timestamp_micros_to_use, is_write] {
diff --git a/service/storage_service.cc b/service/storage_service.cc
--- a/service/storage_service.cc
+++ b/service/storage_service.cc
@@ -351,17 +351,19 @@ std::set<sstring>
storage_service::get_config_supported_features_set() {
// This should only be true in tests (see
cql_test_env.cc:storage_service_for_tests)
auto& db = service::get_local_storage_service().db();
if (db.local_is_initialized()) {
- auto& config =
service::get_local_storage_service().db().local().get_config();
+ auto& config = db.local().get_config();
if (config.enable_sstables_mc_format()) {
features.insert(MC_SSTABLE_FEATURE);
}
if (config.enable_user_defined_functions()) {
- db.local().get_config().check_experimental("UDF");
+ if
(!config.check_experimental(db::experimental_features_t::UDF)) {
+ throw std::runtime_error(
+ "You must use both enable_user_defined_functions
and experimental_features:udf "
+ "to enable user-defined functions");
+ }
features.insert(UDF_FEATURE);
}
-
- if (config.experimental()) {
- // push additional experimental features
+ if (config.check_experimental(db::experimental_features_t::CDC)) {
features.insert(CDC_FEATURE);
}
}
diff --git a/tests/config_test.cc b/tests/config_test.cc
--- a/tests/config_test.cc
+++ b/tests/config_test.cc
@@ -844,14 +844,20 @@ inline std::basic_ostream<Args...> &
operator<<(std::basic_ostream<Args...> & os
}
}

+namespace {
+
+void throw_on_error(const sstring& opt, const sstring& msg,
std::optional<utils::config_file::value_status> status) {
+ if (status != config::value_status::Invalid) {
+ throw std::invalid_argument(msg + " : " + opt);
+ }
+}
+
+} // anonymous namespace
+
SEASTAR_TEST_CASE(test_parse_yaml) {
config cfg;

- cfg.read_from_yaml(cassandra_conf, [](auto& opt, auto& msg, auto
status) {
- if (status != config::value_status::Invalid) {
- throw std::invalid_argument(msg + " : " + opt);
- }
- });
+ cfg.read_from_yaml(cassandra_conf, throw_on_error);

BOOST_CHECK_EQUAL(cfg.cluster_name(), "Test Cluster");
BOOST_CHECK_EQUAL(cfg.cluster_name.is_set(), true);
@@ -917,3 +923,78 @@ SEASTAR_TEST_CASE(test_parse_broken) {

return make_ready_future<>();
}
+
+using ef = experimental_features_t;
+using features = std::vector<enum_option<ef>>;
+
+SEASTAR_TEST_CASE(test_parse_experimental_features_cdc) {
+ config cfg;
+ cfg.read_from_yaml("experimental_features:\n - cdc\n",
throw_on_error);
+ BOOST_CHECK_EQUAL(cfg.experimental_features(), features{ef::CDC});
+ BOOST_CHECK(cfg.check_experimental(ef::CDC));
+ BOOST_CHECK(!cfg.check_experimental(ef::LWT));
+ BOOST_CHECK(!cfg.check_experimental(ef::UDF));
+ return make_ready_future();
+}
+
+SEASTAR_TEST_CASE(test_parse_experimental_features_lwt) {
+ config cfg;
+ cfg.read_from_yaml("experimental_features:\n - lwt\n",
throw_on_error);
+ BOOST_CHECK_EQUAL(cfg.experimental_features(), features{ef::LWT});
+ BOOST_CHECK(!cfg.check_experimental(ef::CDC));
+ BOOST_CHECK(cfg.check_experimental(ef::LWT));
+ BOOST_CHECK(!cfg.check_experimental(ef::UDF));
+ return make_ready_future();
+}
+
+SEASTAR_TEST_CASE(test_parse_experimental_features_udf) {
+ config cfg;
+ cfg.read_from_yaml("experimental_features:\n - udf\n",
throw_on_error);
+ BOOST_CHECK_EQUAL(cfg.experimental_features(), features{ef::UDF});
+ BOOST_CHECK(!cfg.check_experimental(ef::CDC));
+ BOOST_CHECK(!cfg.check_experimental(ef::LWT));
+ BOOST_CHECK(cfg.check_experimental(ef::UDF));
+ return make_ready_future();
+}
+
+SEASTAR_TEST_CASE(test_parse_experimental_features_multiple) {
+ config cfg;
+ cfg.read_from_yaml("experimental_features:\n - cdc\n - lwt\n
- cdc\n", throw_on_error);
+ BOOST_CHECK_EQUAL(cfg.experimental_features(), (features{ef::CDC,
ef::LWT, ef::CDC}));
+ BOOST_CHECK(cfg.check_experimental(ef::CDC));
+ BOOST_CHECK(cfg.check_experimental(ef::LWT));
+ BOOST_CHECK(!cfg.check_experimental(ef::UDF));
+ return make_ready_future();
+}
+
+SEASTAR_TEST_CASE(test_parse_experimental_features_invalid) {
+ config cfg;
+ using value_status = utils::config_file::value_status;
+ cfg.read_from_yaml("experimental_features:\n -
invalidoptiontvaluedonotuse\n",
+ [&cfg] (const sstring& opt, const sstring& msg,
std::optional<value_status> status) {
+
BOOST_REQUIRE_EQUAL(opt, "experimental_features");
+ BOOST_REQUIRE_NE(msg.find("line 2, column 7"),
msg.npos);
+ BOOST_CHECK(!cfg.check_experimental(ef::CDC));
+ BOOST_CHECK(!cfg.check_experimental(ef::LWT));
+ BOOST_CHECK(!cfg.check_experimental(ef::UDF));
+ });
+ return make_ready_future();
+}
+
+SEASTAR_TEST_CASE(test_parse_experimental_true) {
+ config cfg;
+ cfg.read_from_yaml("experimental: true", throw_on_error);
+ BOOST_CHECK(cfg.check_experimental(ef::CDC));
+ BOOST_CHECK(cfg.check_experimental(ef::LWT));
+ BOOST_CHECK(cfg.check_experimental(ef::UDF));
+ return make_ready_future();
+}
+
+SEASTAR_TEST_CASE(test_parse_experimental_false) {
+ config cfg;
+ cfg.read_from_yaml("experimental: false", throw_on_error);
+ BOOST_CHECK(!cfg.check_experimental(ef::CDC));
+ BOOST_CHECK(!cfg.check_experimental(ef::LWT));
+ BOOST_CHECK(!cfg.check_experimental(ef::UDF));
+ return make_ready_future();
+}
diff --git a/tests/cql_test_env.cc b/tests/cql_test_env.cc
--- a/tests/cql_test_env.cc
+++ b/tests/cql_test_env.cc
@@ -360,7 +360,10 @@ class single_node_cql_env : public cql_test_env {
cfg->view_hints_directory.set(data_dir_path
+ "/view_hints.dir");
cfg->num_tokens.set(256);
cfg->ring_delay_ms.set(500);
- cfg->experimental.set(true);
+ auto features = cfg->experimental_features();
+ features.emplace_back(db::experimental_features_t::CDC);
+ features.emplace_back(db::experimental_features_t::LWT);
+ cfg->experimental_features(features);
cfg->shutdown_announce_in_ms.set(0);
cfg->broadcast_to_all_shards().get();
create_directories((data_dir_path + "/system").c_str());
diff --git a/tests/schema_change_test.cc b/tests/schema_change_test.cc
--- a/tests/schema_change_test.cc
+++ b/tests/schema_change_test.cc
@@ -589,6 +589,7 @@ future<>
test_schema_digest_does_not_change_with_disabled_features(sstring data_
auto db_cfg_ptr = make_shared<db::config>();
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},
db::config::config_source::CommandLine);
if (regenerate) {
db_cfg.data_file_directories({data_dir},
db::config::config_source::CommandLine);
} else {
diff --git a/tests/user_function_test.cc b/tests/user_function_test.cc
--- a/tests/user_function_test.cc
+++ b/tests/user_function_test.cc
@@ -45,9 +45,19 @@ static shared_ptr<cql_transport::event::schema_change>
get_schema_change(
}

SEASTAR_TEST_CASE(test_user_function_disabled) {
+ static const char* cql_using_udf =
+ "CREATE FUNCTION my_func(val int) CALLED ON NULL INPUT RETURNS
int LANGUAGE Lua AS 'return 2';";
return do_with_cql_env_thread([] (cql_test_env& e) {
- auto fut = e.execute_cql("CREATE FUNCTION my_func(val int) CALLED
ON NULL INPUT RETURNS int LANGUAGE Lua AS 'return 2';");
- BOOST_REQUIRE_EXCEPTION(fut.get(), ire, message_equals("User
defined functions are disabled. Set enable_user_defined_functions to enable
them"));
+ auto fut = e.execute_cql(cql_using_udf);
+ BOOST_REQUIRE_EXCEPTION(fut.get(), ire, message_contains("User
defined functions are disabled"));
+ }).then([] {
+ auto db_cfg_ptr = make_shared<db::config>();
+ auto& db_cfg = *db_cfg_ptr;
+ db_cfg.enable_user_defined_functions({true},
db::config::config_source::CommandLine);
+ return do_with_cql_env_thread([] (cql_test_env& e) {
+ auto fut = e.execute_cql(cql_using_udf);
+ BOOST_REQUIRE_EXCEPTION(fut.get(), ire, message_contains("User
defined functions are disabled"));
+ });
});
}

@@ -56,6 +66,7 @@ static future<> with_udf_enabled(Func&& func) {
auto db_cfg_ptr = make_shared<db::config>();
auto& db_cfg = *db_cfg_ptr;
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);
return do_with_cql_env_thread(std::forward<Func>(func), db_cfg_ptr);
}

@@ -950,6 +961,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);

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();

[Commit Bot]

Scylla with --experimental-features=lwt to enable.");

[Commit Bot]

From: Dejan Mircevski <de...@scylladb.com>
Committer: Dejan Mircevski <de...@scylladb.com>

Branch: master