diff --git a/CHANGES.txt b/CHANGES.txt index 88e940c280..672711f697 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 5.1 + * Provide clearer exception message on failing commitlog_disk_access_mode combinations (CASSANDRA-19812) * Add total space used for a keyspace to nodetool tablestats (CASSANDRA-19671) * Ensure Relation#toRestriction() handles ReversedType properly (CASSANDRA-19950) * Add JSON and YAML output option to nodetool gcstats (CASSANDRA-19771) diff --git a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java index 66e0fd0532..7009da0aba 100644 --- a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java +++ b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java @@ -1562,7 +1562,14 @@ public class DatabaseDescriptor } else if (compressOrEncrypt && accessModeDirectIoPair.left != DiskAccessMode.standard) { - throw new ConfigurationException("commitlog_disk_access_mode = " + accessModeDirectIoPair.left + " is not supported with compression or encryption. Please use 'auto' when unsure.", false); + String with; + if (null != getCommitLogCompression() && (null != getEncryptionContext() && getEncryptionContext().isEnabled())) + with = "compression or encryption"; + else if (null != getCommitLogCompression()) + with = "compression"; + else + with = "encryption"; + throw new ConfigurationException("commitlog_disk_access_mode = " + accessModeDirectIoPair.left + " is not supported with " + with + ". Please use 'auto' when unsure.", false); } else if (!compressOrEncrypt && accessModeDirectIoPair.left != DiskAccessMode.mmap && accessModeDirectIoPair.left != DiskAccessMode.direct) { diff --git a/test/unit/org/apache/cassandra/cql3/CQLTester.java b/test/unit/org/apache/cassandra/cql3/CQLTester.java index 6d67c05ced..624b069e9c 100644 --- a/test/unit/org/apache/cassandra/cql3/CQLTester.java +++ b/test/unit/org/apache/cassandra/cql3/CQLTester.java @@ -3070,10 +3070,10 @@ public abstract class CQLTester Map config = CONFIG_GEN.next(RANDOM); CONFIG = YamlConfigurationLoader.toYaml(config); - Config c = ConfigGenBuilder.sanitize(DatabaseDescriptor.loadConfig()); + Config c = ConfigGenBuilder.prepare(DatabaseDescriptor.loadConfig()); YamlConfigurationLoader.updateFromMap(config, true, c); - DatabaseDescriptor.unsafeDaemonInitialization(() -> c); + DatabaseDescriptor.unsafeDaemonInitialization(() -> ConfigGenBuilder.sanitize(c)); } public static class FailureWatcher extends TestWatcher diff --git a/test/unit/org/apache/cassandra/utils/ConfigGenBuilder.java b/test/unit/org/apache/cassandra/utils/ConfigGenBuilder.java index f339e42522..c103a8dc6d 100644 --- a/test/unit/org/apache/cassandra/utils/ConfigGenBuilder.java +++ b/test/unit/org/apache/cassandra/utils/ConfigGenBuilder.java @@ -37,12 +37,23 @@ public class ConfigGenBuilder public enum Memtable {SkipListMemtable, TrieMemtable, ShardedSkipListMemtable} + private static boolean validCommitLogDiskAccessMode(Config c) + { + Config.DiskAccessMode m = c.commitlog_disk_access_mode; + + return null != c.commitlog_compression + ? Config.DiskAccessMode.standard == m + : Config.DiskAccessMode.mmap == m || Config.DiskAccessMode.direct == m; + } + @Nullable Gen partitionerGen = Generators.toGen(CassandraGenerators.nonLocalPartitioners()); + Gen commitLogDiskAccessModeGen = Gens.enums().all(Config.DiskAccessMode.class) .filter(m -> m != Config.DiskAccessMode.standard && m != Config.DiskAccessMode.mmap_index_only && m != Config.DiskAccessMode.direct); // don't allow direct as not every filesystem supports it, making the config environment specific + Gen diskAccessModeGen = Gens.enums().all(Config.DiskAccessMode.class).filter(m -> m != Config.DiskAccessMode.direct); Gen sstableFormatGen = Generators.toGen(CassandraGenerators.sstableFormatNames()); Gen memtableAllocationTypeGen = Gens.enums().all(Config.MemtableAllocationType.class); @@ -71,7 +82,7 @@ public class ConfigGenBuilder /** * When loading the {@link Config} from a yaml its possible that some configs set will conflict with the configs that get generated here, to avoid that set them to a good default */ - public static Config sanitize(Config config) + public static Config prepare(Config config) { Config defaults = new Config(); config.commitlog_sync = defaults.commitlog_sync; @@ -80,6 +91,18 @@ public class ConfigGenBuilder return config; } + /** + * After loading from yaml, sanitize the resulting config to avoid fast-failures on illegal combinations + */ + public static Config sanitize(Config config) + { + // if commitlog_disk_access_mode has been generated to something other than standard, make sure compression is disabled + if (null != config.commitlog_compression && Config.DiskAccessMode.standard != config.commitlog_disk_access_mode) + config.commitlog_compression = null; + + return config; + } + public ConfigGenBuilder withPartitioner(IPartitioner instance) { this.partitionerGen = ignore -> instance; diff --git a/test/unit/org/apache/cassandra/utils/ConfigGenBuilderTest.java b/test/unit/org/apache/cassandra/utils/ConfigGenBuilderTest.java index 0c2aaf29ad..6bae12c319 100644 --- a/test/unit/org/apache/cassandra/utils/ConfigGenBuilderTest.java +++ b/test/unit/org/apache/cassandra/utils/ConfigGenBuilderTest.java @@ -57,12 +57,12 @@ public class ConfigGenBuilderTest private static void validate(Map config, Config c) { YamlConfigurationLoader.updateFromMap(config, true, c); - DatabaseDescriptor.unsafeDaemonInitialization(() -> c); + DatabaseDescriptor.unsafeDaemonInitialization(() -> ConfigGenBuilder.sanitize(c)); } private static Config defaultConfig() { - return ConfigGenBuilder.sanitize(DatabaseDescriptor.loadConfig()); + return ConfigGenBuilder.prepare(DatabaseDescriptor.loadConfig()); } private static Config simpleConfig()