Prevent CQLTester fuzz testing from using illegal commitlog_disk_access_mode combinations
Also provide a clearer message from DatabaseDescriptor about the failing combination
patch by Mick Semb Wever; reviewed by David Capwell, Štefan Miklošovič for CASSANDRA-19812
diff --git a/CHANGES.txt b/CHANGES.txt
index 88e940c..672711f 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 66e0fd0..7009da0 100644
--- a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java
+++ b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java
@@ -1562,7 +1562,14 @@
}
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 6d67c05..624b069 100644
--- a/test/unit/org/apache/cassandra/cql3/CQLTester.java
+++ b/test/unit/org/apache/cassandra/cql3/CQLTester.java
@@ -3070,10 +3070,10 @@
Map<String, Object> 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 f339e42..c103a8d 100644
--- a/test/unit/org/apache/cassandra/utils/ConfigGenBuilder.java
+++ b/test/unit/org/apache/cassandra/utils/ConfigGenBuilder.java
@@ -37,12 +37,23 @@
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<IPartitioner> partitionerGen = Generators.toGen(CassandraGenerators.nonLocalPartitioners());
+
Gen<Config.DiskAccessMode> 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<Config.DiskAccessMode> diskAccessModeGen = Gens.enums().all(Config.DiskAccessMode.class).filter(m -> m != Config.DiskAccessMode.direct);
Gen<String> sstableFormatGen = Generators.toGen(CassandraGenerators.sstableFormatNames());
Gen<Config.MemtableAllocationType> memtableAllocationTypeGen = Gens.enums().all(Config.MemtableAllocationType.class);
@@ -71,7 +82,7 @@
/**
* 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 @@
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 0c2aaf2..6bae12c 100644
--- a/test/unit/org/apache/cassandra/utils/ConfigGenBuilderTest.java
+++ b/test/unit/org/apache/cassandra/utils/ConfigGenBuilderTest.java
@@ -57,12 +57,12 @@
private static void validate(Map<String, Object> 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()
