bahman/scylla
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

db/config: Deprecate sstable_compression_dictionaries_allow_in_ddl

Browse Source 
The option is a knob that allows to reject dictionary-aware compressors
in the validation stage of CREATE/ALTER statements, and in the
validation of `sstable_compression_user_table_options`. It was
introduced in 7d26d3c7cb to allow the admins of Scylla Cloud to
selectively enable it in certain clusters. For more details, check:
https://github.com/scylladb/scylla-enterprise/issues/5435

As of this series, we want to start offering dictionary compression as
the default option in all clusters, i.e., treat it as a generally
available feature. This makes the knob redundant.

Additionally, making dictionary compression the default choice in
`sstable_compression_user_table_options` creates an awkward dependency
with the knob (disabling the knob should cause
`sstable_compression_user_table_options` to fall back to a non-dict
compressor as default). That may not be very clear to the end user.

For these reasons, mark the option as "Deprecated", remove all relevant
tests, and adjust the business logic as if dictionary compression is
always available.

Signed-off-by: Nikos Dragazis <nikolaos.dragazis@scylladb.com>
(cherry picked from commit 96e727d7b9)
This commit is contained in:
Nikos Dragazis
2025-10-29 19:26:04 +02:00
parent 260c9972b0
commit bafe2bbbbc
7 changed files with 6 additions and 119 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
cql3/statements/cf_prop_defs.cc
View File

@@ -136,9 +136,7 @@ void cf_prop_defs::validate(const data_dictionary::database db, sstring ks_name,
throw exceptions::configuration_exception(sstring("Missing sub-option '") + compression_parameters::SSTABLE_COMPRESSION + "' for the '" + KW_COMPRESSION + "' option.");
}
compression_parameters cp(*compression_options);
cp.validate(
compression_parameters::dicts_feature_enabled(bool(db.features().sstable_compression_dicts)),
compression_parameters::dicts_usage_allowed(db.get_config().sstable_compression_dictionaries_allow_in_ddl()));
cp.validate(compression_parameters::dicts_feature_enabled(bool(db.features().sstable_compression_dicts)));
}
auto per_partition_rate_limit_options = get_per_partition_rate_limit_options(schema_extensions);

2
db/config.cc
View File

@@ -1326,7 +1326,7 @@ db::config::config(std::shared_ptr<db::extensions> exts)
"* chunk_length_in_kb: (Default: 4) The size of chunks to compress in kilobytes. Allowed values are powers of two between 1 and 128.\n"
"* crc_check_chance: (Default: 1.0) Not implemented (option value is ignored).\n"
"* compression_level: (Default: 3) Compression level for ZstdCompressor and ZstdWithDictsCompressor. Higher levels provide better compression ratios at the cost of speed. Allowed values are integers between 1 and 22.")
, sstable_compression_dictionaries_allow_in_ddl(this, "sstable_compression_dictionaries_allow_in_ddl", liveness::LiveUpdate, value_status::Used, true,
, sstable_compression_dictionaries_allow_in_ddl(this, "sstable_compression_dictionaries_allow_in_ddl", liveness::LiveUpdate, value_status::Deprecated, true,
"Allows for configuring tables to use SSTable compression with shared dictionaries. "
"If the option is disabled, Scylla will reject CREATE and ALTER statements which try to set dictionary-based sstable compressors.\n"
"This is only enforced when this node validates a new DDL statement; disabling the option won't disable dictionary-based compression "

4
main.cc
View File

@@ -2218,10 +2218,8 @@ sharded<locator::shared_token_metadata> token_metadata;
// required SSTABLE_COMPRESSION_DICTS cluster feature has been negotiated.
try {
const auto& dicts_feature_enabled = feature_service.local().sstable_compression_dicts;
const auto& dicts_usage_allowed = cfg->sstable_compression_dictionaries_allow_in_ddl();
cfg->sstable_compression_user_table_options().validate(
compression_parameters::dicts_feature_enabled(bool(dicts_feature_enabled)),
compression_parameters::dicts_usage_allowed(dicts_usage_allowed));
compression_parameters::dicts_feature_enabled(bool(dicts_feature_enabled)));
} catch (const std::exception& e) {
startlog.error("Invalid sstable_compression_user_table_options: {}", e.what());
throw bad_configuration_error();

6
sstables/compressor.cc
View File

@@ -539,17 +539,13 @@ compression_parameters::compression_parameters(const std::map<sstring, sstring>&
}
}
void compression_parameters::validate(dicts_feature_enabled dicts_enabled, dicts_usage_allowed dicts_allowed) const {
void compression_parameters::validate(dicts_feature_enabled dicts_enabled) const {
if (_algorithm == algorithm::zstd_with_dicts || _algorithm == algorithm::lz4_with_dicts) {
if (!dicts_enabled) {
throw std::runtime_error(std::format("sstable_compression {} can't be used before "
"all nodes are upgraded to a versions which supports it",
algorithm_to_name(_algorithm)));
}
if (!dicts_allowed) {
throw std::runtime_error(std::format("sstable_compression {} has been disabled by `sstable_compression_dictionaries_allow_in_ddl: false`",
algorithm_to_name(_algorithm)));
}
}
if (_chunk_length) {
auto chunk_length = _chunk_length.value();

3
sstables/compressor.hh
View File

@@ -106,8 +106,7 @@ public:
std::optional<int> zstd_compression_level() const { return _zstd_compression_level; }
using dicts_feature_enabled = bool_class<struct dicts_feature_enabled_tag>;
using dicts_usage_allowed = bool_class<struct dicts_usage_allowed_tag>;
void validate(dicts_feature_enabled, dicts_usage_allowed) const;
void validate(dicts_feature_enabled) const;
std::map<sstring, sstring> get_options() const;

19
test/cluster/test_sstable_compression_config.py
View File

@@ -23,25 +23,6 @@ def yaml_to_cmdline(config):
return cmdline
@pytest.mark.asyncio
@pytest.mark.parametrize('cfg_source', ['yaml', 'cmdline'])
async def test_dict_compression_not_allowed(manager: ManagerClient, cfg_source: str):
config = {
'sstable_compression_dictionaries_allow_in_ddl': False,
'sstable_compression_user_table_options': {
'sstable_compression': 'ZstdWithDictsCompressor',
'chunk_length_in_kb': 4,
'compression_level': 10
}
}
expected_error = 'Invalid sstable_compression_user_table_options: sstable_compression ZstdWithDictsCompressor has been disabled by `sstable_compression_dictionaries_allow_in_ddl: false`'
if cfg_source == 'yaml':
await manager.server_add(config=config, expected_error=expected_error)
else:
await manager.server_add(cmdline=yaml_to_cmdline(config), expected_error=expected_error)
@pytest.mark.asyncio
@pytest.mark.parametrize('cfg_source', ['yaml', 'cmdline'])
async def test_chunk_size_negative(manager: ManagerClient, cfg_source: str):

87
test/cluster/test_sstable_compression_dictionaries_basic.py
View File

@@ -8,14 +8,10 @@ import logging
import pytest
import itertools
import time
import contextlib
import typing
from test.pylib.manager_client import ManagerClient, ServerInfo
from test.pylib.rest_client import read_barrier, ScyllaMetrics, HTTPError
from test.pylib.rest_client import read_barrier, ScyllaMetrics
from cassandra.cluster import ConsistencyLevel, Session as CassandraSession
from cassandra.policies import FallthroughRetryPolicy, ConstantReconnectionPolicy
from cassandra.protocol import ServerError
from cassandra.query import SimpleStatement
from test.pylib.util import wait_for_cql_and_get_hosts
logger = logging.getLogger(__name__)
@@ -496,84 +492,3 @@ async def test_sstable_compression_dictionaries_enable_writing(manager: ManagerC
for algo in nondict_algorithms:
assert (await get_compressor_names(algo)) == {name_prefix + f"{algo}Compressor"}
assert (await get_compressor_names(no_compression)) == set()
async def test_sstable_compression_dictionaries_allow_in_ddl(manager: ManagerClient):
"""
Tests the sstable_compression_dictionaries_allow_in_ddl option.
When it's disabled, ALTER and CREATE statements should not be allowed
to configure tables to use compression dictionaries for sstables.
"""
# Bootstrap cluster and configure server
logger.info("Bootstrapping cluster")
servers = (await manager.servers_add(1, cmdline=[
*common_debug_cli_options,
"--sstable-compression-dictionaries-allow-in-ddl=false",
], auto_rack_dc="dc1"))
@contextlib.asynccontextmanager
async def with_expect_server_error(msg):
try:
yield
except ServerError as e:
if e.message != msg:
raise
else:
raise Exception('Expected a ServerError, got no exceptions')
cql = manager.get_cql()
hosts = await wait_for_cql_and_get_hosts(cql, servers, time.time() + 60)
await cql.run_async("""
CREATE KEYSPACE test
WITH replication = {'class': 'NetworkTopologyStrategy', 'replication_factor': 1}
""")
for new_algo in ['LZ4WithDicts', 'ZstdWithDicts']:
logger.info(f"Tested algorithm: {new_algo}")
table_name = f"test.{new_algo}"
logger.info("Check that disabled sstable_compression_dictionaries_allow_in_ddl prevents CREATE with dict compression")
async with with_expect_server_error(f"sstable_compression {new_algo}Compressor has been disabled by `sstable_compression_dictionaries_allow_in_ddl: false`"):
await cql.run_async(SimpleStatement(f'''
CREATE TABLE {table_name} (pk int PRIMARY KEY, c blob)
WITH COMPRESSION = {{'sstable_compression': '{new_algo}Compressor'}};
''', retry_policy=FallthroughRetryPolicy()), host=hosts[0])
logger.info("Enable the config option")
await live_update_config(manager, servers, 'sstable_compression_dictionaries_allow_in_ddl', "true")
logger.info("CREATE the table with dict compression")
await cql.run_async(SimpleStatement(f'''
CREATE TABLE {table_name} (pk int PRIMARY KEY, c blob)
WITH COMPRESSION = {{'sstable_compression': '{new_algo}Compressor'}};
''', retry_policy=FallthroughRetryPolicy()), host=hosts[0])
logger.info("Disable compression on the table")
await cql.run_async(SimpleStatement(f'''
ALTER TABLE {table_name}
WITH COMPRESSION = {{'sstable_compression': ''}};
''', retry_policy=FallthroughRetryPolicy()), host=hosts[0])
logger.info("Disable the config option again")
await live_update_config(manager, servers, 'sstable_compression_dictionaries_allow_in_ddl', "false")
logger.info("Check that disabled sstable_compression_dictionaries_allow_in_ddl prevents ALTER with dict compression")
async with with_expect_server_error(f"sstable_compression {new_algo}Compressor has been disabled by `sstable_compression_dictionaries_allow_in_ddl: false`"):
await cql.run_async(SimpleStatement(f'''
ALTER TABLE {table_name}
WITH COMPRESSION = {{'sstable_compression': '{new_algo}Compressor'}};
''', retry_policy=FallthroughRetryPolicy()), host=hosts[0])
logger.info("Enable the config option again")
await live_update_config(manager, servers, 'sstable_compression_dictionaries_allow_in_ddl', "true")
logger.info("ALTER the table with dict compression")
await cql.run_async(SimpleStatement(f'''
ALTER TABLE {table_name}
WITH COMPRESSION = {{'sstable_compression': '{new_algo}Compressor'}};
''', retry_policy=FallthroughRetryPolicy()), host=hosts[0])
logger.info("Enable the config option again")
logger.info("Disable the config option for the next test")
await live_update_config(manager, servers, 'sstable_compression_dictionaries_allow_in_ddl', "false")