config: Remove experimental TABLETS feature

... and replace it with boolean enable_tablets option. All the places in the code are patched to check the latter option instead of the former feature. The option is OFF by default, but the default scylla.yaml file sets this to true, so that newly installed clusters turn tablets ON. Signed-off-by: Pavel Emelyanov <xemul@scylladb.com> Closes scylladb/scylladb#18898
2024-05-27 16:00:16 +03:00
parent dc588d1eef
commit 83d491af02
27 changed files with 41 additions and 51 deletions
--- a/alternator/executor.cc
+++ b/alternator/executor.cc
@@ -4576,7 +4576,7 @@ static lw_shared_ptr<keyspace_metadata> create_keyspace_metadata(std::string_vie
    // used by default on new Alternator tables. Change this initialization
    // to 0 enable tablets by default, with automatic number of tablets.
    std::optional<unsigned> initial_tablets;
-    if (sp.get_db().local().get_config().check_experimental(db::experimental_features_t::feature::TABLETS)) {
+    if (sp.get_db().local().get_config().enable_tablets()) {
        auto it = tags_map.find(INITIAL_TABLETS_TAG_KEY);
        if (it != tags_map.end()) {
            // Tag set. If it's a valid number, use it. If not - e.g., it's
--- a/conf/scylla.yaml
+++ b/conf/scylla.yaml
@@ -618,3 +618,6 @@ maintenance_socket: ignore
 # replication_strategy_warn_list:
 #  - SimpleStrategy
 # replication_strategy_fail_list:
+
+# This enables tablets on newly created keyspaces
+enable_tablets: true
--- a/db/config.cc
+++ b/db/config.cc
@@ -1157,6 +1157,7 @@ db::config::config(std::shared_ptr<db::extensions> exts)
    , service_levels_interval(this, "service_levels_interval_ms", liveness::LiveUpdate, value_status::Used, 10000, "Controls how often service levels module polls configuration table")
    , error_injections_at_startup(this, "error_injections_at_startup", error_injection_value_status, {}, "List of error injections that should be enabled on startup.")
    , topology_barrier_stall_detector_threshold_seconds(this, "topology_barrier_stall_detector_threshold_seconds", value_status::Used, 2, "Report sites blocking topology barrier if it takes longer than this.")
+    , enable_tablets(this, "enable_tablets", value_status::Used, false, "Enable tablets for newly created keyspaces")
    , default_log_level(this, "default_log_level", value_status::Used)
    , logger_log_level(this, "logger_log_level", value_status::Used)
    , log_to_stdout(this, "log_to_stdout", value_status::Used)
@@ -1342,7 +1343,6 @@ std::map<sstring, db::experimental_features_t::feature> db::experimental_feature
        {"consistent-topology-changes", feature::UNUSED},
        {"broadcast-tables", feature::BROADCAST_TABLES},
        {"keyspace-storage-options", feature::KEYSPACE_STORAGE_OPTIONS},
-        {"tablets", feature::TABLETS},
    };
 }

--- a/db/config.hh
+++ b/db/config.hh
@@ -109,7 +109,6 @@ struct experimental_features_t {
        ALTERNATOR_STREAMS,
        BROADCAST_TABLES,
        KEYSPACE_STORAGE_OPTIONS,
-        TABLETS,
    };
    static std::map<sstring, feature> map(); // See enum_option.
    static std::vector<enum_option<experimental_features_t>> all();
@@ -493,6 +492,7 @@ public:

    named_value<std::vector<error_injection_at_startup>> error_injections_at_startup;
    named_value<double> topology_barrier_stall_detector_threshold_seconds;
+    named_value<bool> enable_tablets;

    static const sstring default_tls_priority;
 private:
--- a/db/system_keyspace.cc
+++ b/db/system_keyspace.cc
@@ -2262,7 +2262,7 @@ std::vector<schema_ptr> system_keyspace::all_tables(const db::config& cfg) {
        r.insert(r.end(), {broadcast_kv_store()});
    }

-    if (cfg.check_experimental(db::experimental_features_t::feature::TABLETS)) {
+    if (cfg.enable_tablets()) {
        r.insert(r.end(), {tablets()});
    }

--- a/gms/feature_service.cc
+++ b/gms/feature_service.cc
@@ -78,7 +78,7 @@ feature_config feature_config_from_db_config(const db::config& cfg, std::set<sst
    if (!cfg.check_experimental(db::experimental_features_t::feature::KEYSPACE_STORAGE_OPTIONS)) {
        fcfg._disabled_features.insert("KEYSPACE_STORAGE_OPTIONS"s);
    }
-    if (!cfg.check_experimental(db::experimental_features_t::feature::TABLETS)) {
+    if (!cfg.enable_tablets()) {
        fcfg._disabled_features.insert("TABLETS"s);
    }
    if (!cfg.uuid_sstable_identifiers_enabled()) {
--- a/service/migration_manager.cc
+++ b/service/migration_manager.cc
@@ -159,7 +159,7 @@ void migration_manager::init_messaging_service()
            auto cm = co_await db::schema_tables::convert_schema_to_mutations(proxy, features);
            if (options->group0_snapshot_transfer) {
                cm.emplace_back(co_await db::system_keyspace::get_group0_history(db));
-                if (proxy.local().local_db().get_config().check_experimental(db::experimental_features_t::feature::TABLETS)) {
+                if (proxy.local().local_db().get_config().enable_tablets()) {
                    for (auto&& m: co_await replica::read_tablet_mutations(db)) {
                        cm.emplace_back(std::move(m));
                    }
--- a/service/storage_service.cc
+++ b/service/storage_service.cc
@@ -632,7 +632,7 @@ future<> storage_service::topology_state_load() {
    std::unordered_set<raft::server_id> prev_normal = boost::copy_range<std::unordered_set<raft::server_id>>(_topology_state_machine._topology.normal_nodes | boost::adaptors::map_keys);

    std::unordered_set<locator::host_id> tablet_hosts;
-    if (_db.local().get_config().check_experimental(db::experimental_features_t::feature::TABLETS)) {
+    if (_db.local().get_config().enable_tablets()) {
        tablet_hosts = co_await replica::read_required_hosts(_qp);
    }

@@ -706,7 +706,7 @@ future<> storage_service::topology_state_load() {

        auto nodes_to_notify = co_await sync_raft_topology_nodes(tmptr, std::nullopt, std::move(prev_normal));

-        if (_db.local().get_config().check_experimental(db::experimental_features_t::feature::TABLETS)) {
+        if (_db.local().get_config().enable_tablets()) {
            tmptr->set_tablets(co_await replica::read_tablet_metadata(_qp));
            tmptr->tablets().set_balancing_enabled(_topology_state_machine._topology.tablet_balancing_enabled);
        }
@@ -5152,7 +5152,7 @@ void storage_service::on_update_tablet_metadata() {
 }

 future<> storage_service::load_tablet_metadata() {
-    if (!_db.local().get_config().check_experimental(db::experimental_features_t::feature::TABLETS)) {
+    if (!_db.local().get_config().enable_tablets()) {
        return make_ready_future<>();
    }
    return mutate_token_metadata([this] (mutable_token_metadata_ptr tmptr) -> future<> {
@@ -5237,7 +5237,7 @@ void storage_service::start_tablet_split_monitor() {
    if (this_shard_id() != 0) {
        return;
    }
-    if (!_db.local().get_config().check_experimental(db::experimental_features_t::feature::TABLETS)) {
+    if (!_db.local().get_config().enable_tablets()) {
        return;
    }
    slogger.info("Starting the tablet split monitor...");
--- a/service/tablet_allocator.cc
+++ b/service/tablet_allocator.cc
@@ -1331,7 +1331,7 @@ public:
        if (_config.initial_tablets_scale == 0) {
            throw std::runtime_error("Initial tablets scale must be positive");
        }
-        if (db.get_config().check_experimental(db::experimental_features_t::feature::TABLETS)) {
+        if (db.get_config().enable_tablets()) {
            _migration_notifier.register_listener(this);
        }
    }
--- a/test/alternator/run
+++ b/test/alternator/run
@@ -32,7 +32,7 @@ remove_scylla_options = []
 # temporary usefulness, and should eventually be removed.
 if '--vnodes' in sys.argv:
    sys.argv.remove('--vnodes')
-    remove_scylla_options.append('--experimental-features=tablets')
+    remove_scylla_options.append('--enable-tablets=true')

 if "-h" in sys.argv or "--help" in sys.argv:
    run.run_pytest(sys.path[0], sys.argv)
--- a/test/alternator/suite.yaml
+++ b/test/alternator/suite.yaml
@@ -10,8 +10,7 @@ extra_scylla_config_options:
    experimental_features: [
                             udf,
                             alternator-streams,
-                             keyspace-storage-options,
-                             tablets
+                             keyspace-storage-options
    ],
    alternator_port: 8000,
    query_tombstone_page_limit: 1000,
@@ -19,5 +18,6 @@ extra_scylla_config_options:
    alternator_enforce_authorization: True,
    alternator_timeout_in_ms: 30000,
    alternator_ttl_period_in_seconds: 0.5,
-    alternator_streams_time_window_s: 0
+    alternator_streams_time_window_s: 0,
+    enable_tablets: True
  }
--- a/test/boost/commitlog_cleanup_test.cc
+++ b/test/boost/commitlog_cleanup_test.cc
@@ -115,7 +115,7 @@ SEASTAR_TEST_CASE(test_commitlog_cleanups) {
    auto cfg = cql_test_config();
    cfg.db_config->auto_snapshot.set(false);
    cfg.db_config->commitlog_sync.set("batch");
-    cfg.db_config->experimental_features.set({db::experimental_features_t::feature::TABLETS});
+    cfg.db_config->enable_tablets.set(true);
    cfg.initial_tablets = 1;

    return do_with_cql_env_thread([](cql_test_env& e) {
@@ -167,7 +167,7 @@ SEASTAR_TEST_CASE(test_commitlog_cleanup_record_gc) {
    auto cfg = cql_test_config();
    cfg.db_config->auto_snapshot.set(false);
    cfg.db_config->commitlog_sync.set("batch");
-    cfg.db_config->experimental_features.set({db::experimental_features_t::feature::TABLETS});
+    cfg.db_config->enable_tablets.set(true);
    cfg.initial_tablets = 1;

    return do_with_cql_env_thread([](cql_test_env& e) {
--- a/test/boost/cql_query_test.cc
+++ b/test/boost/cql_query_test.cc
@@ -5697,9 +5697,7 @@ SEASTAR_TEST_CASE(test_setting_synchronous_updates_property) {
 static
 cql_test_config tablet_cql_test_config() {
    cql_test_config c;
-    c.db_config->experimental_features({
-            db::experimental_features_t::feature::TABLETS,
-        }, db::config::config_source::CommandLine);
+    c.db_config->enable_tablets.set(true);
    return c;
 }

--- a/test/boost/tablets_test.cc
+++ b/test/boost/tablets_test.cc
@@ -60,9 +60,7 @@ void verify_tablet_metadata_persistence(cql_test_env& env, const tablet_metadata
 static
 cql_test_config tablet_cql_test_config() {
    cql_test_config c;
-    c.db_config->experimental_features({
-            db::experimental_features_t::feature::TABLETS,
-        }, db::config::config_source::CommandLine);
+    c.db_config->enable_tablets(true);
    c.initial_tablets = 2;
    return c;
 }
--- a/test/cql-pytest/run
+++ b/test/cql-pytest/run
@@ -22,7 +22,7 @@ if '--vnodes' in sys.argv:
    sys.argv.remove('--vnodes')
    def run_without_tablets(pid, dir):
        (c, e) = run_without_tablets.orig_cmd(pid, dir)
-        c.remove('--experimental-features=tablets')
+        c.remove('--enable-tablets=true')
        return (c, e)
    run_without_tablets.orig_cmd = cmd
    cmd = run_without_tablets
--- a/test/cql-pytest/run.py
+++ b/test/cql-pytest/run.py
@@ -312,7 +312,7 @@ def run_scylla_cmd(pid, dir):
        # test/alternator/run.
        '--experimental-features=udf',
        '--experimental-features=keyspace-storage-options',
-        '--experimental-features=tablets',
+        '--enable-tablets=true',
        '--enable-user-defined-functions', '1',
        # Set up authentication in order to allow testing this module
        # and other modules dependent on it: e.g. service levels
--- a/test/cql-pytest/suite.yaml
+++ b/test/cql-pytest/suite.yaml
@@ -5,4 +5,4 @@ dirties_cluster:
 extra_scylla_cmdline_options:
  - '--experimental-features=udf'
  - '--experimental-features=keyspace-storage-options'
-  - '--experimental-features=tablets'
+  - '--enable-tablets=true'
--- a/test/perf/perf_simple_query.cc
+++ b/test/perf/perf_simple_query.cc
@@ -563,8 +563,7 @@ int scylla_simple_query_main(int argc, char** argv) {
            db_cfg->enable_cache(enable_cache);
            cql_test_config cfg(db_cfg);
            if (app.configuration().contains("tablets")) {
-                cfg.db_config->experimental_features({db::experimental_features_t::feature::TABLETS},
-                                                      db::config::config_source::CommandLine);
+                cfg.db_config->enable_tablets.set(true);
                cfg.initial_tablets = app.configuration()["initial-tablets"].as<unsigned>();
            }
          return do_with_cql_env_thread([&app] (auto&& env) {
--- a/test/perf/perf_tablets.cc
+++ b/test/perf/perf_tablets.cc
@@ -36,9 +36,7 @@ static const size_t MiB = 1 << 20;
 static
 cql_test_config tablet_cql_test_config() {
    cql_test_config c;
-    c.db_config->experimental_features({
-               db::experimental_features_t::feature::TABLETS,
-       }, db::config::config_source::CommandLine);
+    c.db_config->enable_tablets.set(true);
    return c;
 }

--- a/test/rest_api/run
+++ b/test/rest_api/run
@@ -24,7 +24,7 @@ if '--vnodes' in sys.argv:
    sys.argv.remove('--vnodes')
    def run_without_tablets(pid, dir):
        (c, e) = run_without_tablets.orig_cmd(pid, dir)
-        c.remove('--experimental-features=tablets')
+        c.remove('--enable-tablets=true')
        return (c, e)
    run_without_tablets.orig_cmd = cmd
    cmd = run_without_tablets
--- a/test/rest_api/suite.yaml
+++ b/test/rest_api/suite.yaml
@@ -7,4 +7,4 @@ skip_in_release:

 extra_scylla_cmdline_options:
  - '--experimental-features=udf'
-  - '--experimental-features=tablets'
+  - '--enable-tablets=true'
--- a/test/topology_custom/test_tablets.py
+++ b/test/topology_custom/test_tablets.py
@@ -23,8 +23,7 @@ logger = logging.getLogger(__name__)

@pytest.mark.asyncio
 async def test_tablet_replication_factor_enough_nodes(manager: ManagerClient):
-    cfg = {'enable_user_defined_functions': False,
-           'experimental_features': ['tablets']}
+    cfg = {'enable_user_defined_functions': False, 'enable_tablets': True}
    servers = await manager.servers_add(2, config=cfg)

    cql = manager.get_cql()
@@ -42,7 +41,7 @@ async def test_tablet_replication_factor_enough_nodes(manager: ManagerClient):
@pytest.mark.asyncio
 async def test_tablet_cannot_decommision_below_replication_factor(manager: ManagerClient):
    logger.info("Bootstrapping cluster")
-    cfg = {'enable_user_defined_functions': False, 'experimental_features': ['tablets']}
+    cfg = {'enable_user_defined_functions': False, 'enable_tablets': True}
    servers = await manager.servers_add(4, config=cfg)

    logger.info("Creating table")
@@ -71,7 +70,7 @@ async def test_tablet_cannot_decommision_below_replication_factor(manager: Manag

 async def test_reshape_with_tablets(manager: ManagerClient):
    logger.info("Bootstrapping cluster")
-    cfg = {'enable_user_defined_functions': False, 'experimental_features': ['tablets']}
+    cfg = {'enable_user_defined_functions': False, 'enable_tablets': True}
    server = (await manager.servers_add(1, config=cfg, cmdline=['--smp', '1']))[0]

    logger.info("Creating table")
@@ -107,8 +106,7 @@ async def test_reshape_with_tablets(manager: ManagerClient):
@pytest.mark.parametrize("direction", ["up", "down", "none"])
@pytest.mark.asyncio
 async def test_tablet_rf_change(manager: ManagerClient, direction):
-    cfg = {'enable_user_defined_functions': False,
-           'experimental_features': ['tablets']}
+    cfg = {'enable_user_defined_functions': False, 'enable_tablets': True}
    servers = await manager.servers_add(3, config=cfg)
    for s in servers:
        await manager.api.disable_tablet_balancing(s.ip_addr)
@@ -173,10 +171,7 @@ async def test_tablet_rf_change(manager: ManagerClient, direction):
 # from is migrated away.
@pytest.mark.asyncio
 async def test_saved_readers_tablet_migration(manager: ManagerClient, mode):
-    cfg = {
-            'enable_user_defined_functions': False,
-            'experimental_features': ['tablets'],
-    }
+    cfg = {'enable_user_defined_functions': False, 'enable_tablets': True}

    if mode != "release":
        cfg['error_injections_at_startup'] = [{'name': 'querier-cache-ttl-seconds', 'value': 999999999}]
--- a/test/topology_custom/test_tablets_migration.py
+++ b/test/topology_custom/test_tablets_migration.py
@@ -22,7 +22,7 @@ logger = logging.getLogger(__name__)
@pytest.mark.asyncio
 async def test_tablet_transition_sanity(manager: ManagerClient, action):
    logger.info("Bootstrapping cluster")
-    cfg = {'enable_user_defined_functions': False, 'experimental_features': ['tablets']}
+    cfg = {'enable_user_defined_functions': False, 'enable_tablets': True}
    host_ids = []
    servers = []

@@ -104,7 +104,7 @@ async def test_node_failure_during_tablet_migration(manager: ManagerClient, fail
        pytest.skip('Failing source during target cleanup is pointless')

    logger.info("Bootstrapping cluster")
-    cfg = {'enable_user_defined_functions': False, 'experimental_features': ['tablets']}
+    cfg = {'enable_user_defined_functions': False, 'enable_tablets': True}
    host_ids = []
    servers = []

@@ -246,7 +246,7 @@ async def test_node_failure_during_tablet_migration(manager: ManagerClient, fail
@pytest.mark.asyncio
 async def test_tablet_back_and_forth_migration(manager: ManagerClient):
    logger.info("Bootstrapping cluster")
-    cfg = {'enable_user_defined_functions': False, 'experimental_features': ['tablets', 'consistent-topology-changes']}
+    cfg = {'enable_user_defined_functions': False, 'enable_tablets': True}
    host_ids = []
    servers = []

--- a/test/topology_custom/test_topology_failure_recovery.py
+++ b/test/topology_custom/test_topology_failure_recovery.py
@@ -20,8 +20,7 @@ async def inject_error_on(manager, error_name, servers):
@pytest.mark.asyncio
@skip_mode('release', 'error injections are not supported in release mode')
 async def test_tablet_drain_failure_during_decommission(manager: ManagerClient):
-    cfg = {'enable_user_defined_functions': False,
-           'experimental_features': ['tablets']}
+    cfg = {'enable_user_defined_functions': False, 'enable_tablets': True}
    servers = [await manager.server_add(config=cfg) for _ in range(3)]

    logs = [await manager.server_open_log(srv.server_id) for srv in servers]
--- a/test/topology_experimental_raft/suite.yaml
+++ b/test/topology_experimental_raft/suite.yaml
@@ -6,7 +6,7 @@ extra_scylla_config_options:
    authenticator: AllowAllAuthenticator
    authorizer: AllowAllAuthorizer
    enable_user_defined_functions: False
-    experimental_features: ['tablets']
+    enable_tablets: True
 run_first:
  - test_raft_cluster_features
  - test_raft_ignore_nodes
--- a/test/topology_experimental_raft/test_tablets.py
+++ b/test/topology_experimental_raft/test_tablets.py
@@ -652,7 +652,7 @@ async def test_tablet_cleanup_failure(manager: ManagerClient):
@pytest.mark.asyncio
 async def test_tablet_resharding(manager: ManagerClient):
    cmdline = ['--smp=3']
-    config = {'experimental_features': ['tablets']}
+    config = {'enable_tablets': True}
    servers = await manager.servers_add(1, cmdline=cmdline)
    server = servers[0]

--- a/test/topology_experimental_raft/test_topology_ops.py
+++ b/test/topology_experimental_raft/test_topology_ops.py
@@ -24,7 +24,7 @@ logger = logging.getLogger(__name__)
@pytest.mark.parametrize("tablets_enabled", [True, False])
 async def test_topology_ops(request, manager: ManagerClient, tablets_enabled: bool):
    """Test basic topology operations using the topology coordinator."""
-    cfg = {'experimental_features': ['tablets'] if tablets_enabled else []}
+    cfg = {'enable_tablets' : tablets_enabled}

    logger.info("Bootstrapping first node")
    servers = [await manager.server_add(config=cfg)]