db: Remove redundant enable_logstor config option

The enable_logstor configuration option is redundant with the 'logstor'
experimental feature flag. Consolidate to a single gate: use the
experimental feature to control both whether logstor is available for
table creation and whether it is initialized at database startup.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>

Closes scylladb/scylladb#29427

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

db/config.cc

@@ -1291,7 +1291,6 @@ db::config::config(std::shared_ptr<db::extensions> 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.")

db/config.hh

@@ -370,7 +370,6 @@ public:
     named_value<bool> enable_in_memory_data_store;
     named_value<bool> enable_cache;
     named_value<bool> enable_commitlog;
-    named_value<bool> enable_logstor;
     named_value<bool> volatile_system_keyspace_for_testing;
     named_value<uint16_t> api_port;
     named_value<sstring> api_address;

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
 ```

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<qos::service_level_controller>& 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();
     }
 }

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)