diff --git a/cql3/statements/cf_prop_defs.cc b/cql3/statements/cf_prop_defs.cc index 33da04a7de..ef82a1dd55 100644 --- a/cql3/statements/cf_prop_defs.cc +++ b/cql3/statements/cf_prop_defs.cc @@ -206,9 +206,6 @@ void cf_prop_defs::validate(const data_dictionary::database db, sstring ks_name, if (!db.features().logstor) { throw exceptions::configuration_exception(format("The experimental feature 'logstor' must be enabled in order to use the 'logstor' storage engine.")); } - if (!db.get_config().enable_logstor()) { - throw exceptions::configuration_exception(format("The configuration option 'enable_logstor' must be set to true in the configuration in order to use the 'logstor' storage engine.")); - } } else { throw exceptions::configuration_exception(format("Illegal value for '{}'", KW_STORAGE_ENGINE)); } diff --git a/db/config.cc b/db/config.cc index 3ab564b3d4..15407650a8 100644 --- a/db/config.cc +++ b/db/config.cc @@ -1291,7 +1291,6 @@ db::config::config(std::shared_ptr exts) , enable_in_memory_data_store(this, "enable_in_memory_data_store", value_status::Used, false, "Enable in memory mode (system tables are always persisted).") , enable_cache(this, "enable_cache", value_status::Used, true, "Enable cache.") , enable_commitlog(this, "enable_commitlog", value_status::Used, true, "Enable commitlog.") - , enable_logstor(this, "enable_logstor", value_status::Used, false, "Enable the logstor storage engine.") , volatile_system_keyspace_for_testing(this, "volatile_system_keyspace_for_testing", value_status::Used, false, "Don't persist system keyspace - testing only!") , api_port(this, "api_port", value_status::Used, 10000, "Http Rest API port.") , api_address(this, "api_address", value_status::Used, "", "Http Rest API address.") diff --git a/db/config.hh b/db/config.hh index 24a572664c..4c4d5884af 100644 --- a/db/config.hh +++ b/db/config.hh @@ -370,7 +370,6 @@ public: named_value enable_in_memory_data_store; named_value enable_cache; named_value enable_commitlog; - named_value enable_logstor; named_value volatile_system_keyspace_for_testing; named_value api_port; named_value api_address; diff --git a/docs/dev/logstor.md b/docs/dev/logstor.md index b740cd03c0..931c8c0103 100644 --- a/docs/dev/logstor.md +++ b/docs/dev/logstor.md @@ -68,11 +68,9 @@ The `buffered_writer` manages multiple write buffers for user writes, an active ### Enabling Logstor -To use logstor, enable it in the configuration: +To use logstor, enable the experimental feature in the configuration: ```yaml -enable_logstor: true - experimental_features: - logstor ``` diff --git a/replica/database.cc b/replica/database.cc index 8db8ffb285..2472637eee 100644 --- a/replica/database.cc +++ b/replica/database.cc @@ -1199,11 +1199,8 @@ void database::add_column_family(keyspace& ks, schema_ptr schema, column_family: } if (schema->logstor_enabled()) { - if (!_cfg.enable_logstor()) { - throw std::runtime_error(fmt::format("The table {}.{} is using logstor storage but logstor is not enabled in the configuration", schema->ks_name(), schema->cf_name())); - } if (!_logstor) { - on_internal_error(dblog, "The table is using logstor but logstor is not initialized"); + throw std::runtime_error(fmt::format("The table {}.{} is using logstor storage but logstor is not initialized", schema->ks_name(), schema->cf_name())); } cf->init_logstor(_logstor.get()); dblog.info0("Table {}.{} is using logstor storage", schema->ks_name(), schema->cf_name()); @@ -2725,7 +2722,7 @@ future<> database::start(sharded& sl_controller, _compaction_manager.enable(); } co_await init_commitlog(); - if (_cfg.enable_logstor()) { + if (_cfg.check_experimental(db::experimental_features_t::feature::LOGSTOR)) { co_await init_logstor(); } } diff --git a/test/cluster/test_logstor.py b/test/cluster/test_logstor.py index 3c049363c9..4f3d48a748 100644 --- a/test/cluster/test_logstor.py +++ b/test/cluster/test_logstor.py @@ -20,7 +20,7 @@ logger = logging.getLogger(__name__) @pytest.mark.asyncio async def test_property(manager: ManagerClient): cmdline = ['--logger-log-level', 'logstor=debug'] - cfg = {'enable_logstor': True, 'experimental_features': ['logstor']} + cfg = {'experimental_features': ['logstor']} await manager.servers_add(1, cmdline=cmdline, config=cfg) cql = manager.get_cql() @@ -39,10 +39,27 @@ async def test_property(manager: ManagerClient): with pytest.raises(ConfigurationException, match="The 'logstor' storage engine cannot be used with tables that have clustering columns"): await cql.run_async(f"CREATE TABLE {ks}.t_enabled (pk int, ck int, v int, PRIMARY KEY (pk, ck)) WITH storage_engine = 'logstor'") +@pytest.mark.asyncio +async def test_config_option_consistency(manager: ManagerClient): + """ + Test that logstor storage engine requires the experimental 'logstor' feature to be enabled. + Without the feature flag, users cannot create logstor tables. + """ + cmdline = ['--logger-log-level', 'logstor=debug'] + # Logstor feature is NOT enabled + cfg = {'experimental_features': []} + await manager.servers_add(1, cmdline=cmdline, config=cfg) + cql = manager.get_cql() + + async with new_test_keyspace(manager, "") as ks: + # Should fail because logstor feature is not enabled + with pytest.raises(ConfigurationException, match="The experimental feature 'logstor' must be enabled"): + await cql.run_async(f"CREATE TABLE {ks}.t_logstor (pk int PRIMARY KEY, v int) WITH storage_engine = 'logstor'") + @pytest.mark.asyncio async def test_basic_write_and_read(manager: ManagerClient): cmdline = ['--logger-log-level', 'logstor=debug'] - cfg = {'enable_logstor': True, 'experimental_features': ['logstor']} + cfg = {'experimental_features': ['logstor']} await manager.servers_add(1, cmdline=cmdline, config=cfg) cql = manager.get_cql() @@ -86,7 +103,7 @@ async def test_basic_write_and_read(manager: ManagerClient): @pytest.mark.asyncio async def test_range_read(manager: ManagerClient): cmdline = ['--logger-log-level', 'logstor=debug'] - cfg = {'enable_logstor': True, 'experimental_features': ['logstor']} + cfg = {'experimental_features': ['logstor']} await manager.servers_add(1, cmdline=cmdline, config=cfg) cql = manager.get_cql() @@ -114,7 +131,7 @@ async def test_range_read(manager: ManagerClient): @pytest.mark.asyncio async def test_parallel_writes(manager: ManagerClient): cmdline = ['--logger-log-level', 'logstor=debug'] - cfg = {'enable_logstor': True, 'experimental_features': ['logstor']} + cfg = {'experimental_features': ['logstor']} await manager.servers_add(1, cmdline=cmdline, config=cfg) cql = manager.get_cql() @@ -133,7 +150,7 @@ async def test_parallel_writes(manager: ManagerClient): @pytest.mark.asyncio async def test_overwrites(manager: ManagerClient): cmdline = ['--logger-log-level', 'logstor=debug'] - cfg = {'enable_logstor': True, 'experimental_features': ['logstor']} + cfg = {'experimental_features': ['logstor']} await manager.servers_add(1, cmdline=cmdline, config=cfg) cql = manager.get_cql() @@ -156,7 +173,7 @@ async def test_parallel_big_writes(manager: ManagerClient): Perform multiple writes in parallel with large values and validate to test segment switching. """ cmdline = ['--logger-log-level', 'logstor=debug', '--smp=1'] - cfg = {'enable_logstor': True, 'experimental_features': ['logstor']} + cfg = {'experimental_features': ['logstor']} await manager.servers_add(1, cmdline=cmdline, config=cfg) cql = manager.get_cql() @@ -192,7 +209,7 @@ async def test_recovery_basic(manager: ManagerClient, fail_separator_flush: bool 6. Performs additional writes and reads to verify system continues functioning """ cmdline = ['--logger-log-level', 'logstor=trace'] - cfg = {'enable_logstor': True, 'experimental_features': ['logstor']} + cfg = {'experimental_features': ['logstor']} servers = await manager.servers_add(1, cmdline=cmdline, config=cfg) cql = manager.get_cql() @@ -282,7 +299,6 @@ async def test_recovery_with_segment_reuse(manager: ManagerClient): cmdline = ['--logger-log-level', 'logstor=trace', '--smp=1'] cfg = { - 'enable_logstor': True, 'logstor_disk_size_in_mb': disk_size_mb, 'logstor_file_size_in_mb': file_size_mb, 'experimental_features': ['logstor'] @@ -337,7 +353,7 @@ async def test_compaction(manager: ManagerClient): Test log compaction by creating dead data and verifying space reclamation. """ cmdline = ['--logger-log-level', 'logstor=trace', '--smp=1'] - cfg = {'enable_logstor': True, 'experimental_features': ['logstor']} + cfg = {'experimental_features': ['logstor']} servers = await manager.servers_add(1, cmdline=cmdline, config=cfg) cql = manager.get_cql() @@ -378,7 +394,7 @@ async def test_drop_table(manager: ManagerClient): Test that DROP TABLE works properly with logstor tables. """ cmdline = ['--logger-log-level', 'logstor=trace'] - cfg = {'enable_logstor': True, 'experimental_features': ['logstor']} + cfg = {'experimental_features': ['logstor']} servers = await manager.servers_add(1, cmdline=cmdline, config=cfg) cql = manager.get_cql() @@ -449,7 +465,6 @@ async def test_trigger_separator_flush(manager: ManagerClient): cmdline = ['--logger-log-level', 'logstor=debug', '--smp=1'] cfg = { - 'enable_logstor': True, 'logstor_disk_size_in_mb': disk_size_mb, 'logstor_file_size_in_mb': file_size_mb, 'experimental_features': ['logstor'] @@ -495,7 +510,6 @@ async def test_tablet_split_trigger_by_size(manager: ManagerClient): ] cfg = { 'tablet_load_stats_refresh_interval_in_seconds': 1, - 'enable_logstor': True, 'experimental_features': ['logstor'], } servers = [await manager.server_add(cmdline=cmdline, config=cfg)] @@ -541,7 +555,7 @@ async def test_tablet_split_and_merge(manager: ManagerClient): ] cfg = { 'tablet_load_stats_refresh_interval_in_seconds': 1, - 'enable_logstor': True, 'experimental_features': ['logstor'] + 'experimental_features': ['logstor'] } servers = [await manager.server_add(config=cfg, cmdline=cmdline)] @@ -598,7 +612,7 @@ async def test_tablet_migration(manager: ManagerClient): Test tablet migration """ cmdline = ['--logger-log-level', 'logstor=trace', '--logger-log-level', 'stream_blob=trace','--smp=1'] - cfg = {'enable_logstor': True, 'experimental_features': ['logstor']} + cfg = {'experimental_features': ['logstor']} s1 = await manager.server_add(cmdline=cmdline, config=cfg, property_file={"dc": "dc1", "rack": "rack1"}) servers = [s1] cql, hosts = await manager.get_ready_cql(servers) @@ -634,7 +648,7 @@ async def test_tablet_intranode_migration(manager: ManagerClient): Test tablet intranode migration """ cmdline = ['--logger-log-level', 'logstor=trace', '--logger-log-level', 'stream_blob=trace','--smp=2'] - cfg = {'enable_logstor': True, 'experimental_features': ['logstor']} + cfg = {'experimental_features': ['logstor']} s1 = await manager.server_add(cmdline=cmdline, config=cfg, property_file={"dc": "dc1", "rack": "rack1"}) servers = [s1] cql, _ = await manager.get_ready_cql(servers) @@ -686,7 +700,7 @@ async def test_tablet_migration_with_compaction(manager: ManagerClient): 8. Verifies all data is correct on the destination after migration """ cmdline = ['--logger-log-level', 'logstor=trace', '--logger-log-level', 'stream_blob=trace', '--smp=2'] - cfg = {'enable_logstor': True, 'experimental_features': ['logstor']} + cfg = {'experimental_features': ['logstor']} s1 = await manager.server_add(cmdline=cmdline, config=cfg, property_file={"dc": "dc1", "rack": "rack1"}) servers = [s1] cql, _ = await manager.get_ready_cql(servers)