diff --git a/src/java/org/apache/cassandra/schema/MemtableParams.java b/src/java/org/apache/cassandra/schema/MemtableParams.java index bd1f1b3f4645..145d9a5d5988 100644 --- a/src/java/org/apache/cassandra/schema/MemtableParams.java +++ b/src/java/org/apache/cassandra/schema/MemtableParams.java @@ -20,6 +20,7 @@ import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.nio.ByteBuffer; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; @@ -30,14 +31,21 @@ import com.google.common.base.Objects; import com.google.common.collect.ImmutableMap; +import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.apache.cassandra.config.DatabaseDescriptor; import org.apache.cassandra.config.InheritingClass; import org.apache.cassandra.config.ParameterizedClass; +import org.apache.cassandra.cql3.UntypedResultSet; +import org.apache.cassandra.db.marshal.MapType; +import org.apache.cassandra.db.marshal.UTF8Type; import org.apache.cassandra.db.memtable.Memtable; import org.apache.cassandra.db.memtable.TrieMemtableFactory; import org.apache.cassandra.exceptions.ConfigurationException; +import org.apache.cassandra.serializers.MarshalException; +import org.apache.cassandra.utils.CassandraVersion; +import org.apache.cassandra.utils.StorageCompatibilityMode; /** * Memtable types and options are specified with these parameters. Memtable classes must either contain a static @@ -50,6 +58,7 @@ */ public final class MemtableParams { + private static final Logger logger = LoggerFactory.getLogger(MemtableParams.class); public final Memtable.Factory factory; private final String configurationKey; @@ -165,10 +174,10 @@ public static MemtableParams getWithFallback(String key) } catch (ConfigurationException e) { - LoggerFactory.getLogger(MemtableParams.class).error("Invalid memtable configuration \"" + key + "\" in schema. " + - "Falling back to default to avoid schema mismatch.\n" + - "Please ensure the correct definition is given in cassandra.yaml.", - e); + logger.error("Invalid memtable configuration \"" + key + "\" in schema. " + + "Falling back to default to avoid schema mismatch.\n" + + "Please ensure the correct definition is given in cassandra.yaml.", + e); return new MemtableParams(DEFAULT.factory(), key); } } @@ -304,4 +313,243 @@ private static Memtable.Factory getMemtableFactory(ParameterizedClass options) throw new ConfigurationException("Could not create memtable factory for class " + options, e); } } + + /** + * Attempts to read memtable configuration, with fallback for CC4 upgrade compatibility. + * + * CC4 stored memtable as {@code frozen>}, while CC5 uses text. + * During upgrades or in mixed clusters, the column may contain either format. + * + * This method uses byte-sniffing (detecting null bytes) to determine the actual + * format of the stored data. The storage_compatibility_mode determines the format + * to write, but doesn't guarantee the format of existing stored data. + * + * This defensive approach handles: + * + * + * @param row The row containing the memtable column + * @param columnName The name of the memtable column + * @return MemtableParams instance, or DEFAULT if column is missing or invalid + */ + public static MemtableParams getWithCC4Fallback(UntypedResultSet.Row row, String columnName) + { + if (!row.has(columnName)) + return DEFAULT; + + String stringValue; + try + { + stringValue = row.getString(columnName); + } + catch (MarshalException e) + { + // CC4 map data may not be valid UTF-8, fall back to map parsing + return parseCC4MapFormat(row, columnName); + } + + // Check if this looks like binary data (contains null bytes from CC4's map serialization) + if (stringValue != null && stringValue.indexOf('\0') >= 0) + { + return parseCC4MapFormat(row, columnName); + } + + // Normal CC5 string value + return getWithFallback(stringValue); + } + + private static MemtableParams parseCC4MapFormat(UntypedResultSet.Row row, String columnName) + { + // This is likely CC4's frozen> serialization + // Try to read it as a map instead + try + { + ByteBuffer raw = row.getBytes(columnName); + Map cc4Map = MapType.getInstance(UTF8Type.instance, UTF8Type.instance, false).compose(raw); + + if (cc4Map == null || cc4Map.isEmpty()) + { + // Empty map in CC4 means "default" + logger.info("Detected CC4 empty memtable map for upgrade compatibility, using default"); + return DEFAULT; + } + + // Convert CC4 map format to CC5 configuration key + String className = cc4Map.get("class"); + if (className != null) + { + // CC4 used class names like "SkipListMemtable" or "TrieMemtable" + // Try to map to CC5 configuration keys + String configKey = mapCC4ClassNameToCC5Key(className); + logger.info("Detected CC4 memtable configuration '{}', mapped to CC5 key '{}'", + className, configKey); + return getWithFallback(configKey); + } + else + { + // CC4 map exists but has no "class" key - likely corrupted data + logger.warn("Detected CC4 memtable map without 'class' key, falling back to default"); + return DEFAULT; + } + } + catch (Exception e) + { + logger.warn("Failed to parse memtable column as CC4 map format, falling back to default", e); + return DEFAULT; + } + } + + private static String mapCC4ClassNameToCC5Key(String cc4ClassName) + { + // Handle both short names and fully qualified names + String shortName = cc4ClassName.contains(".") + ? cc4ClassName.substring(cc4ClassName.lastIndexOf('.') + 1) + : cc4ClassName; + + // Map common CC4 class names to CC5 configuration keys + switch (shortName) + { + case "SkipListMemtable": + return "skiplist"; + case "TrieMemtable": + return "trie"; + default: + // For unknown types, try the short name as-is + logger.warn("Unknown CC4 memtable class '{}', attempting to use as configuration key", shortName); + return shortName.toLowerCase(); + } + } + + /** + * Returns the memtable value as a map for CC4 compatibility mode. + * Used when storage_compatibility_mode is CC_4 or CASSANDRA_4. + * + * @return Map representation for CC4 schema (frozen<map<text,text>>) + * @throws ConfigurationException if the memtable type is not compatible with CC4 + */ + public Map asSchemaValueMap() + { + return asSchemaValueMap(DatabaseDescriptor.getStorageCompatibilityMode()); + } + + /** + * Returns the memtable value as a map for CC4 compatibility mode. + * This overload exists for testing purposes. + * + * @param mode The storage compatibility mode to use + * @return Map representation for CC4 schema (frozen<map<text,text>>) + * @throws ConfigurationException if the memtable type is not compatible with CC4 + */ + @VisibleForTesting + Map asSchemaValueMap(StorageCompatibilityMode mode) + { + if (!mode.isBefore(CassandraVersion.CASSANDRA_5_0.major)) + throw new IllegalStateException("Cannot get map value in CC5 mode. Use asSchemaValueText() instead."); + + // CC4 writes empty map {} for "default" configuration + if ("default".equals(configurationKey)) + return ImmutableMap.of(); + + // Validate and map the configuration key to CC4 class name + // This also validates CC4 compatibility (rejects sharded types, unknown configs) + String className = mapCC5KeyToCC4ClassName(configurationKey); + + // Get the configuration definition to access parameters + ParameterizedClass definition = CONFIGURATION_DEFINITIONS.get(configurationKey); + + // Build the map with class name and any additional parameters + Map map = new HashMap<>(); + map.put("class", className); + if (definition != null && definition.parameters != null) + map.putAll(definition.parameters); + + return map; + } + + /** + * Returns the memtable value as text for CC5 mode. + * Used when storage_compatibility_mode is NONE. + * + * @return String representation for CC5 schema (text) + */ + public String asSchemaValueText() + { + return asSchemaValueText(DatabaseDescriptor.getStorageCompatibilityMode()); + } + + /** + * Returns the memtable value as text for CC5 mode. + * This overload exists for testing purposes. + * + * @param mode The storage compatibility mode to use + * @return String representation for CC5 schema (text) + * @throws IllegalStateException if called in CC4 compatibility mode + */ + @VisibleForTesting + String asSchemaValueText(StorageCompatibilityMode mode) + { + if (mode.isBefore(CassandraVersion.CASSANDRA_5_0.major)) + throw new IllegalStateException("Cannot get text value in CC4 compatibility mode. Use asSchemaValueMap() instead."); + + return configurationKey; + } + + /** + * Maps CC5 configuration key to CC4 class name, validating CC4 compatibility. + * This method combines validation and mapping for use in CC4 compatibility mode. + * + * @param configKey The CC5 configuration key (e.g., "trie", "skiplist") + * @return The corresponding CC4 class name (e.g., "TrieMemtable") + * @throws ConfigurationException if the configuration is not compatible with CC4 + */ + private static String mapCC5KeyToCC4ClassName(String configKey) + { + if (configKey == null || configKey.isEmpty()) + throw new ConfigurationException("Configuration key cannot be null or empty"); + + // Check if this is a CC5-only memtable type + // ShardedSkipListMemtable and related sharded types don't exist in CC4 + String lowerKey = configKey.toLowerCase(); + if (lowerKey.contains("sharded")) + { + throw new ConfigurationException( + String.format("Memtable configuration '%s' is not compatible with CC4. " + + "Sharded memtable types were introduced in CC5. " + + "Please use 'skiplist' or 'trie' when storage_compatibility_mode is CC_4 or CASSANDRA_4.", + configKey)); + } + + // Check if the configuration key exists in CONFIGURATION_DEFINITIONS + // This ensures we're not trying to write an unknown/invalid configuration + ParameterizedClass definition = CONFIGURATION_DEFINITIONS.get(configKey); + if (definition == null) + { + throw new ConfigurationException( + String.format("Memtable configuration '%s' not found in cassandra.yaml. " + + "Cannot write to schema in CC4 compatibility mode.", + configKey)); + } + + // Get the class name from the definition and strip the package prefix + // CC4 accepts both short names (e.g., 'TrieMemtable') and fully qualified names, + // but we use short names for standard Cassandra memtables + String className = definition.class_name; + if (className == null || className.isEmpty()) + { + throw new ConfigurationException( + String.format("Memtable configuration '%s' has no class name defined.", + configKey)); + } + + // Strip the standard Cassandra memtable package prefix + if (className.startsWith("org.apache.cassandra.db.memtable.")) + { + className = className.substring("org.apache.cassandra.db.memtable.".length()); + } + + return className; + } } diff --git a/src/java/org/apache/cassandra/schema/SchemaKeyspace.java b/src/java/org/apache/cassandra/schema/SchemaKeyspace.java index b897bf52b6c2..9528da54f0bf 100644 --- a/src/java/org/apache/cassandra/schema/SchemaKeyspace.java +++ b/src/java/org/apache/cassandra/schema/SchemaKeyspace.java @@ -59,6 +59,7 @@ import org.apache.cassandra.schema.Keyspaces.KeyspacesDiff; import org.apache.cassandra.service.reads.repair.ReadRepairStrategy; import org.apache.cassandra.utils.ByteBufferUtil; +import org.apache.cassandra.utils.CassandraVersion; import org.apache.cassandra.utils.FBUtilities; import org.apache.cassandra.utils.Simulate; @@ -117,6 +118,42 @@ private SchemaKeyspace() + "graph_engine text," + "PRIMARY KEY ((keyspace_name)))"); + // CC4-compatible schema with memtable as frozen> + // Used when storage_compatibility_mode is CC_4 to support downgrade to CC4 + private static final TableMetadata TablesLegacy = + parse(TABLES, + "table definitions", + "CREATE TABLE %s (" + + "keyspace_name text," + + "table_name text," + + "allow_auto_snapshot boolean," + + "bloom_filter_fp_chance double," + + "caching frozen>," + + "comment text," + + "compaction frozen>," + + "compression frozen>," + + "memtable frozen>," // CC4 format for downgrade compatibility + + "crc_check_chance double," + + "dclocal_read_repair_chance double," // no longer used, left for drivers' sake + + "default_time_to_live int," + + "extensions frozen>," + + "flags frozen>," // SUPER, COUNTER, DENSE, COMPOUND + + "gc_grace_seconds int," + + "incremental_backups boolean," + + "id uuid," + + "max_index_interval int," + + "memtable_flush_period_in_ms int," + + "min_index_interval int," + + "nodesync frozen>," + + "read_repair_chance double," // no longer used, left for drivers' sake + + "speculative_retry text," + + "additional_write_policy text," + + "cdc boolean," + + "read_repair text," + + "PRIMARY KEY ((keyspace_name), table_name))"); + + // CC5 schema with memtable as text + // Used when storage_compatibility_mode is NONE (no downgrade support) private static final TableMetadata Tables = parse(TABLES, "table definitions", @@ -129,7 +166,7 @@ private SchemaKeyspace() + "comment text," + "compaction frozen>," + "compression frozen>," - + "memtable text," + + "memtable text," // CC5 format + "crc_check_chance double," + "dclocal_read_repair_chance double," // no longer used, left for drivers' sake + "default_time_to_live int," @@ -200,6 +237,44 @@ private SchemaKeyspace() + "options frozen>," + "PRIMARY KEY ((keyspace_name), table_name, trigger_name))"); + // CC4-compatible schema with memtable as frozen> + private static final TableMetadata ViewsLegacy = + parse(VIEWS, + "view definitions", + "CREATE TABLE %s (" + + "keyspace_name text," + + "view_name text," + + "base_table_id uuid," + + "base_table_name text," + + "where_clause text," + + "allow_auto_snapshot boolean," + + "bloom_filter_fp_chance double," + + "caching frozen>," + + "comment text," + + "compaction frozen>," + + "compression frozen>," + + "memtable frozen>," // CC4 format for downgrade compatibility + + "crc_check_chance double," + + "dclocal_read_repair_chance double," // no longer used, left for drivers' sake + + "default_time_to_live int," + + "extensions frozen>," + + "gc_grace_seconds int," + + "incremental_backups boolean," + + "id uuid," + + "include_all_columns boolean," + + "max_index_interval int," + + "memtable_flush_period_in_ms int," + + "min_index_interval int," + + "nodesync frozen>," + + "read_repair_chance double," // no longer used, left for drivers' sake + + "speculative_retry text," + + "additional_write_policy text," + + "cdc boolean," + + "version int," + + "read_repair text," + + "PRIMARY KEY ((keyspace_name), view_name))"); + + // CC5 schema with memtable as text private static final TableMetadata Views = parse(VIEWS, "view definitions", @@ -215,7 +290,7 @@ private SchemaKeyspace() + "comment text," + "compaction frozen>," + "compression frozen>," - + "memtable text," + + "memtable text," // CC5 format + "crc_check_chance double," + "dclocal_read_repair_chance double," // no longer used, left for drivers' sake + "default_time_to_live int," @@ -289,17 +364,46 @@ private SchemaKeyspace() + "deterministic boolean," + "PRIMARY KEY ((keyspace_name), aggregate_name, argument_types))"); - private static final List ALL_TABLE_METADATA = ImmutableList.of(Keyspaces, - Tables, - Columns, - ColumnMasks, - Triggers, - DroppedColumns, - Views, - Types, - Functions, - Aggregates, - Indexes); + /** + * Returns the list of schema table metadata based on current storage compatibility mode. + */ + private static List allTableMetadata() + { + return ImmutableList.of(Keyspaces, + // Use legacy schema (frozen) when in CC_4 compatibility mode to support downgrade + tablesTableMetadata(), + Columns, + ColumnMasks, + Triggers, + DroppedColumns, + viewsTableMetadata(), + Types, + Functions, + Aggregates, + Indexes); + } + + /** + * Returns the appropriate Tables schema table metadata based on current storage compatibility mode. + * Uses TablesLegacy ({@code frozen} for memtable) in CC4 mode, Tables (text for memtable) otherwise. + */ + private static TableMetadata tablesTableMetadata() + { + return DatabaseDescriptor.getStorageCompatibilityMode().isBefore(CassandraVersion.CASSANDRA_5_0.major) + ? TablesLegacy + : Tables; + } + + /** + * Returns the appropriate Views schema table metadata based on current storage compatibility mode. + * Uses ViewsLegacy ({@code frozen} for memtable) in CC4 mode, Views (text for memtable) otherwise. + */ + private static TableMetadata viewsTableMetadata() + { + return DatabaseDescriptor.getStorageCompatibilityMode().isBefore(CassandraVersion.CASSANDRA_5_0.major) + ? ViewsLegacy + : Views; + } private static TableMetadata parse(String name, String description, String cql) { @@ -313,7 +417,7 @@ private static TableMetadata parse(String name, String description, String cql) public static KeyspaceMetadata metadata() { - return KeyspaceMetadata.create(SchemaConstants.SCHEMA_KEYSPACE_NAME, KeyspaceParams.local(), org.apache.cassandra.schema.Tables.of(ALL_TABLE_METADATA)); + return KeyspaceMetadata.create(SchemaConstants.SCHEMA_KEYSPACE_NAME, KeyspaceParams.local(), org.apache.cassandra.schema.Tables.of(allTableMetadata())); } static Collection convertSchemaDiffToMutations(KeyspacesDiff diff, long timestamp) @@ -538,7 +642,7 @@ private static Mutation.SimpleBuilder makeDropKeyspaceMutation(KeyspaceMetadata Mutation.SimpleBuilder builder = Mutation.simpleBuilder(SchemaConstants.SCHEMA_KEYSPACE_NAME, decorate(Keyspaces, keyspace.name)) .timestamp(timestamp); - for (TableMetadata schemaTable : ALL_TABLE_METADATA) + for (TableMetadata schemaTable : allTableMetadata()) builder.update(schemaTable).delete(); return builder; @@ -568,7 +672,7 @@ static Mutation.SimpleBuilder makeCreateTableMutation(KeyspaceMetadata keyspace, private static void addTableToSchemaMutation(TableMetadata table, boolean withColumnsAndTriggers, Mutation.SimpleBuilder builder) { - Row.SimpleBuilder rowBuilder = builder.update(Tables) + Row.SimpleBuilder rowBuilder = builder.update(tablesTableMetadata()) .row(table.name) .deletePrevious() .add("id", table.id.asUUID()) @@ -619,8 +723,14 @@ private static void addTableParamsToRowBuilder(TableParams params, Row.SimpleBui // As above, only add the memtable column if the table uses a non-default memtable configuration to avoid RTE // in mixed operation with pre-4.1 versioned node during upgrades. + // Write in CC4 format (map) or CC5 format (text) based on storage compatibility mode if (params.memtable != MemtableParams.DEFAULT) - builder.add("memtable", params.memtable.configurationKey()); + { + if (DatabaseDescriptor.getStorageCompatibilityMode().isBefore(CassandraVersion.CASSANDRA_5_0.major)) + builder.add("memtable", params.memtable.asSchemaValueMap()); + else + builder.add("memtable", params.memtable.asSchemaValueText()); + } // As above, only add the allow_auto_snapshot column if the value is not default (true) and // auto-snapshotting is enabled, to avoid RTE in pre-4.2 versioned node during upgrades @@ -723,7 +833,7 @@ private static MapDifference triggersDiff(Triggers befo private static void addDropTableToSchemaMutation(TableMetadata table, Mutation.SimpleBuilder builder) { - builder.update(Tables).row(table.name).delete(); + builder.update(tablesTableMetadata()).row(table.name).delete(); for (ColumnMetadata column : table.columns()) dropColumnFromSchemaMutation(table, column, builder); @@ -832,7 +942,7 @@ private static void dropTriggerFromSchemaMutation(TableMetadata table, TriggerMe private static void addViewToSchemaMutation(ViewMetadata view, boolean includeColumns, Mutation.SimpleBuilder builder) { TableMetadata table = view.metadata; - Row.SimpleBuilder rowBuilder = builder.update(Views) + Row.SimpleBuilder rowBuilder = builder.update(viewsTableMetadata()) .row(view.name()) .deletePrevious() .add("include_all_columns", view.includeAllColumns) @@ -855,7 +965,7 @@ private static void addViewToSchemaMutation(ViewMetadata view, boolean includeCo private static void addDropViewToSchemaMutation(ViewMetadata view, Mutation.SimpleBuilder builder) { - builder.update(Views).row(view.name()).delete(); + builder.update(viewsTableMetadata()).row(view.name()).delete(); TableMetadata table = view.metadata; for (ColumnMetadata column : table.columns()) @@ -1077,9 +1187,8 @@ static TableParams createTableParamsFromRow(UntypedResultSet.Row row) .comment(row.getString("comment")) .compaction(CompactionParams.fromMap(row.getFrozenTextMap("compaction"))) .compression(CompressionParams.fromMap(row.getFrozenTextMap("compression"))) - .memtable(MemtableParams.getWithFallback(row.has("memtable") - ? row.getString("memtable") - : null)) // memtable column was introduced in 4.1 + // Handles CC4 upgrade compatibility + .memtable(MemtableParams.getWithCC4Fallback(row, "memtable")) .defaultTimeToLive(row.getInt("default_time_to_live")) .extensions(row.getFrozenMap("extensions", UTF8Type.instance, BytesType.instance)) .gcGraceSeconds(row.getInt("gc_grace_seconds")) diff --git a/src/java/org/apache/cassandra/utils/StorageCompatibilityMode.java b/src/java/org/apache/cassandra/utils/StorageCompatibilityMode.java index c99b423b8f95..45b7f00e5b8e 100644 --- a/src/java/org/apache/cassandra/utils/StorageCompatibilityMode.java +++ b/src/java/org/apache/cassandra/utils/StorageCompatibilityMode.java @@ -42,7 +42,13 @@ public enum StorageCompatibilityMode CASSANDRA_4(4), /** - * Same as {@link #CASSANDRA_4}, but allows the use of BTI format in {@link #validateSstableFormat}. + * Same major version as {@link #CASSANDRA_4}, but with additional CC_4-specific behaviors: + *
    + *
  • Schema storage: The memtable column in system_schema.tables and system_schema.views is stored + * as {@code frozen>} (CC4 format) instead of {@code text} (CC5 format). + * This ensures safe downgrades to CC4.
    • + *
  • SSTable format: Allows BTI format in {@link #validateSstableFormat}.
    • + *
*/ CC_4(4), diff --git a/test/unit/org/apache/cassandra/schema/MemtableParamsTest.java b/test/unit/org/apache/cassandra/schema/MemtableParamsTest.java index d3fc8befc22a..0d42b054d785 100644 --- a/test/unit/org/apache/cassandra/schema/MemtableParamsTest.java +++ b/test/unit/org/apache/cassandra/schema/MemtableParamsTest.java @@ -18,6 +18,8 @@ package org.apache.cassandra.schema; +import java.nio.ByteBuffer; +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.Map; @@ -26,10 +28,15 @@ import org.apache.cassandra.config.InheritingClass; import org.apache.cassandra.config.ParameterizedClass; +import org.apache.cassandra.cql3.UntypedResultSet; +import org.apache.cassandra.db.marshal.MapType; +import org.apache.cassandra.db.marshal.UTF8Type; import org.apache.cassandra.db.memtable.TrieMemtableFactory; import org.apache.cassandra.exceptions.ConfigurationException; +import org.apache.cassandra.utils.StorageCompatibilityMode; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -278,4 +285,258 @@ public void testInvalidLoops() // expected } } + + // ======================================================================== + // CC4 to CC5 Upgrade Compatibility Tests + // ======================================================================== + + /** + * Helper method to create a row with CC4 binary map data. + */ + private UntypedResultSet.Row createCC4MapRow(Map cc4Map) + { + ByteBuffer serialized = MapType.getInstance(UTF8Type.instance, UTF8Type.instance, false) + .decompose(cc4Map); + Map data = new HashMap<>(); + data.put("memtable", serialized); + return new UntypedResultSet.Row(data); + } + + /** + * Helper method to create a row with CC5 text data. + */ + private UntypedResultSet.Row createCC5TextRow(String textValue) + { + ByteBuffer serialized = UTF8Type.instance.decompose(textValue); + Map data = new HashMap<>(); + data.put("memtable", serialized); + return new UntypedResultSet.Row(data); + } + + /** + * Helper method to assert memtable configuration key. + */ + private void assertMemtableConfigKey(UntypedResultSet.Row row, String expectedKey) + { + MemtableParams params = MemtableParams.getWithCC4Fallback(row, "memtable"); + assertNotNull(params); + assertEquals(expectedKey, params.configurationKey()); + } + + /** + * Test CC4 empty map upgrade. + * CC4 stored empty map as 4 null bytes (32-bit integer = 0 entries). + */ + @Test + public void testCC4EmptyMapUpgrade() + { + ByteBuffer emptyMap = ByteBuffer.wrap(new byte[]{ 0x00, 0x00, 0x00, 0x00}); + Map data = new HashMap<>(); + data.put("memtable", emptyMap); + UntypedResultSet.Row row = new UntypedResultSet.Row(data); + + MemtableParams params = MemtableParams.getWithCC4Fallback(row, "memtable"); + assertEquals(MemtableParams.DEFAULT, params); + } + + /** + * Test CC4 TrieMemtable configuration upgrade. + * CC4 stored: {"class": "TrieMemtable"} → Should map to CC5 "trie" + */ + @Test + public void testCC4TrieMemtableUpgrade() + { + UntypedResultSet.Row row = createCC4MapRow(ImmutableMap.of("class", "TrieMemtable")); + assertMemtableConfigKey(row, "trie"); + } + + /** + * Test CC4 SkipListMemtable configuration upgrade. + * CC4 stored: {"class": "SkipListMemtable"} → Should map to CC5 "skiplist" + */ + @Test + public void testCC4SkipListMemtableUpgrade() + { + UntypedResultSet.Row row = createCC4MapRow(ImmutableMap.of("class", "SkipListMemtable")); + assertMemtableConfigKey(row, "skiplist"); + } + + /** + * Test CC4 fully qualified class name upgrade. + * CC4 stored: {"class": "org.apache.cassandra.db.memtable.TrieMemtable"} + * Should extract short name and map to CC5 "trie" + */ + @Test + public void testCC4FullyQualifiedClassNameUpgrade() + { + UntypedResultSet.Row row = createCC4MapRow( + ImmutableMap.of("class", "org.apache.cassandra.db.memtable.TrieMemtable") + ); + assertMemtableConfigKey(row, "trie"); + } + + /** + * Test CC4 map with additional parameters (should still work). + * CC4 stored: {"class": "TrieMemtable", "extra_param": "value"} + * Should extract class name and map to CC5 "trie" + */ + @Test + public void testCC4MapWithExtraParametersUpgrade() + { + UntypedResultSet.Row row = createCC4MapRow( + ImmutableMap.of("class", "TrieMemtable", "extra_param", "some_value") + ); + assertMemtableConfigKey(row, "trie"); + } + + /** + * Test CC4 map without "class" key (should fall back to default). + * CC4 corrupted data: {"other_key": "value"} → Should fall back to DEFAULT + */ + @Test + public void testCC4MapWithoutClassKeyUpgrade() + { + UntypedResultSet.Row row = createCC4MapRow(ImmutableMap.of("other_key", "value")); + MemtableParams params = MemtableParams.getWithCC4Fallback(row, "memtable"); + assertEquals(MemtableParams.DEFAULT, params); + } + + /** + * Test CC5 text values (normal operation, should work unchanged). + */ + @Test + public void testCC5TextValueTrie() + { + assertMemtableConfigKey(createCC5TextRow("trie"), "trie"); + } + + @Test + public void testCC5TextValueSkiplist() + { + assertMemtableConfigKey(createCC5TextRow("skiplist"), "skiplist"); + } + + @Test + public void testCC5TextValueDefault() + { + assertMemtableConfigKey(createCC5TextRow("default"), "default"); + } + + /** + * Test missing memtable column (should return DEFAULT). + * This happens when reading old schema that predates the memtable column. + */ + @Test + public void testMissingMemtableColumn() + { + UntypedResultSet.Row row = new UntypedResultSet.Row(new HashMap<>()); + MemtableParams params = MemtableParams.getWithCC4Fallback(row, "memtable"); + assertEquals(MemtableParams.DEFAULT, params); + } + + /** + * Test null memtable value (should return DEFAULT). + */ + @Test + public void testNullMemtableValue() + { + Map data = new HashMap<>(); + data.put("memtable", null); + UntypedResultSet.Row row = new UntypedResultSet.Row(data); + + MemtableParams params = MemtableParams.getWithCC4Fallback(row, "memtable"); + assertEquals(MemtableParams.DEFAULT, params); + } + + // ======================================================================== + // StorageCompatibilityMode Writing Tests + // ======================================================================== + + /** + * Test that asSchemaValueMap() returns a Map in CC_4 and CASSANDRA_4 compatibility modes. + * Both modes should write memtable as {@code frozen>} for CC4 compatibility. + * This ensures downgrade to CC4 is safe. + */ + @Test + public void testAsSchemaValueMapInCC4CompatibilityModes() + { + // Test both CC_4 and CASSANDRA_4 modes (they should behave identically) + for (StorageCompatibilityMode mode : new StorageCompatibilityMode[]{StorageCompatibilityMode.CC_4, + StorageCompatibilityMode.CASSANDRA_4}) + { + // Test DEFAULT memtable - CC4 writes empty map {} for "default" configuration + Map defaultMap = MemtableParams.DEFAULT.asSchemaValueMap(mode); + assertNotNull("Should return Map in " + mode + " mode", defaultMap); + assertTrue("Default should be empty map in " + mode + " mode", defaultMap.isEmpty()); + } + } + + /** + * Test that asSchemaValueMap() throws exception when called in CC5 mode. + */ + @Test(expected = IllegalStateException.class) + public void testAsSchemaValueMapThrowsInCC5Mode() + { + MemtableParams.DEFAULT.asSchemaValueMap(StorageCompatibilityMode.NONE); + } + + /** + * Test that asSchemaValueMap() rejects incompatible configurations in CC_4 mode. + * Tests both CC5-only types (sharded) and unknown configurations. + */ + @Test + public void testAsSchemaValueMapRejectsIncompatibleConfigurations() + { + // Test 1: Sharded memtables (CC5-only, don't exist in CC4) + MemtableParams shardedParams = MemtableParams.forTesting(MemtableParams.DEFAULT.factory(), "sharded-skiplist"); + try + { + shardedParams.asSchemaValueMap(StorageCompatibilityMode.CC_4); + fail("Should have thrown ConfigurationException for sharded memtable in CC_4 mode"); + } + catch (ConfigurationException e) + { + assertTrue("Error message should mention CC4 incompatibility", + e.getMessage().contains("not compatible with CC4")); + assertTrue("Error message should mention sharded types", + e.getMessage().contains("Sharded memtable types")); + } + + // Test 2: Unknown configurations (might not exist in CC4) + MemtableParams unknownParams = MemtableParams.forTesting(MemtableParams.DEFAULT.factory(), "unknown-memtable-type"); + try + { + unknownParams.asSchemaValueMap(StorageCompatibilityMode.CC_4); + fail("Should have thrown ConfigurationException for unknown configuration in CC_4 mode"); + } + catch (ConfigurationException e) + { + assertTrue("Error message should mention configuration not found", + e.getMessage().contains("not found in cassandra.yaml")); + assertTrue("Error message should mention CC4 compatibility mode", + e.getMessage().contains("CC4 compatibility mode")); + } + } + + /** + * Test that asSchemaValueText() returns a String in CC5 mode (NONE). + * This is the normal CC5 operation. + */ + @Test + public void testAsSchemaValueTextInCC5Mode() + { + // Test DEFAULT memtable in CC5 mode + String defaultValue = MemtableParams.DEFAULT.asSchemaValueText(StorageCompatibilityMode.NONE); + assertEquals("default", defaultValue); + } + + /** + * Test that asSchemaValueText() throws exception when called in CC4 compatibility mode. + */ + @Test(expected = IllegalStateException.class) + public void testAsSchemaValueTextThrowsInCC4Mode() + { + MemtableParams.DEFAULT.asSchemaValueText(StorageCompatibilityMode.CC_4); + } + } diff --git a/test/unit/org/apache/cassandra/schema/SchemaKeyspaceTest.java b/test/unit/org/apache/cassandra/schema/SchemaKeyspaceTest.java index 7551fb3af418..2ba7be43100a 100644 --- a/test/unit/org/apache/cassandra/schema/SchemaKeyspaceTest.java +++ b/test/unit/org/apache/cassandra/schema/SchemaKeyspaceTest.java @@ -18,7 +18,6 @@ */ package org.apache.cassandra.schema; -import java.io.IOException; import java.nio.ByteBuffer; import java.util.Collection; import java.util.Collections; @@ -42,6 +41,7 @@ import org.apache.cassandra.SchemaLoader; import org.apache.cassandra.config.CassandraRelevantProperties; import org.apache.cassandra.config.DatabaseDescriptor; +import org.apache.cassandra.cql3.ColumnIdentifier; import org.apache.cassandra.cql3.QueryProcessor; import org.apache.cassandra.cql3.UntypedResultSet; import org.apache.cassandra.cql3.statements.schema.CreateTableStatement; @@ -60,6 +60,7 @@ import org.apache.cassandra.net.Verb; import org.apache.cassandra.service.reads.repair.ReadRepairStrategy; import org.apache.cassandra.utils.FBUtilities; +import org.apache.cassandra.utils.StorageCompatibilityMode; import org.apache.cassandra.utils.concurrent.AsyncPromise; import org.jboss.byteman.contrib.bmunit.BMRule; import org.jboss.byteman.contrib.bmunit.BMUnitRunner; @@ -87,6 +88,80 @@ public static void defineSchema() throws ConfigurationException MessagingService.instance().listen(); } + /** + * Helper method to run a test with different storage compatibility modes. + * This ensures schema mutations work correctly in both CC5 and CC4 modes. + * + * @param test The test logic to run for each mode + * @param validateMetadata If true, validates that schema table metadata matches the expected format for each mode + */ + private void runWithAllCompatibilityModes(TestWithMode test, boolean validateMetadata) throws Exception + { + StorageCompatibilityMode[] modes = { + StorageCompatibilityMode.NONE, // CC5 mode + StorageCompatibilityMode.CC_4, // CC4 compatibility mode + StorageCompatibilityMode.CASSANDRA_4 // Cassandra 4 compatibility mode + }; + + for (StorageCompatibilityMode mode : modes) + { + StorageCompatibilityMode original = DatabaseDescriptor.getStorageCompatibilityMode(); + try + { + DatabaseDescriptor.setStorageCompatibilityMode(mode); + + if (validateMetadata) + { + validateSchemaTableMetadata(mode); + } + + test.run(mode); + } + finally + { + DatabaseDescriptor.setStorageCompatibilityMode(original); + } + } + } + + /** + * Validates that the schema table metadata (Tables and Views) matches the expected format + * for the given storage compatibility mode. + */ + private void validateSchemaTableMetadata(StorageCompatibilityMode mode) + { + KeyspaceMetadata ksm = SchemaKeyspace.metadata(); + + // Validate Tables table + TableMetadata tablesTable = ksm.tables.getNullable(SchemaKeyspaceTables.TABLES); + assertNotNull("Tables table should exist in mode " + mode, tablesTable); + + ColumnMetadata memtableColumn = tablesTable.getColumn(new ColumnIdentifier("memtable", true)); + assertNotNull("Memtable column should exist in mode " + mode, memtableColumn); + + String expectedType = mode.isBefore(5) ? "frozen>" : "text"; + assertEquals("Memtable column type should match mode " + mode, + expectedType, + memtableColumn.type.asCQL3Type().toString()); + + // Validate Views table + TableMetadata viewsTable = ksm.tables.getNullable(SchemaKeyspaceTables.VIEWS); + assertNotNull("Views table should exist in mode " + mode, viewsTable); + + ColumnMetadata viewMemtableColumn = viewsTable.getColumn(new ColumnIdentifier("memtable", true)); + assertNotNull("View memtable column should exist in mode " + mode, viewMemtableColumn); + + assertEquals("View memtable column type should match mode " + mode, + expectedType, + viewMemtableColumn.type.asCQL3Type().toString()); + } + + @FunctionalInterface + private interface TestWithMode + { + void run(StorageCompatibilityMode mode) throws Exception; + } + /** See CASSANDRA-16856/16996. Make sure schema pulls are synchronized to prevent concurrent schema pull/writes */ @Test @BMRule(name = "delay partition updates to schema tables", @@ -146,38 +221,42 @@ private Collection getSchemaMutations() @Test public void testConversionsInverses() throws Exception { - for (String keyspaceName : Schema.instance.distributedKeyspaces().names()) - { - for (ColumnFamilyStore cfs : Keyspace.open(keyspaceName).getColumnFamilyStores()) + runWithAllCompatibilityModes(mode -> { + for (String keyspaceName : Schema.instance.distributedKeyspaces().names()) { - checkInverses(cfs.metadata()); - - // Testing with compression to catch #3558 - TableMetadata withCompression = cfs.metadata().unbuild().compression(CompressionParams.snappy(32768)).build(); - checkInverses(withCompression); + for (ColumnFamilyStore cfs : Keyspace.open(keyspaceName).getColumnFamilyStores()) + { + checkInverses(cfs.metadata()); + + // Testing with compression to catch #3558 + TableMetadata withCompression = cfs.metadata().unbuild().compression(CompressionParams.snappy(32768)).build(); + checkInverses(withCompression); + } } - } + }, true); // Validate metadata for each mode } @Test - public void testExtensions() throws IOException + public void testExtensions() throws Exception { - String keyspace = "SandBox"; + runWithAllCompatibilityModes(mode -> { + String keyspace = "SandBox"; - createTable(keyspace, "CREATE TABLE test (a text primary key, b int, c int)"); + createTable(keyspace, "CREATE TABLE test (a text primary key, b int, c int)"); - TableMetadata metadata = Schema.instance.getTableMetadata(keyspace, "test"); - assertTrue("extensions should be empty", metadata.params.extensions.isEmpty()); + TableMetadata metadata = Schema.instance.getTableMetadata(keyspace, "test"); + assertTrue("extensions should be empty", metadata.params.extensions.isEmpty()); - ImmutableMap extensions = ImmutableMap.of("From ... with Love", - ByteBuffer.wrap(new byte[]{0, 0, 7})); + ImmutableMap extensions = ImmutableMap.of("From ... with Love", + ByteBuffer.wrap(new byte[]{0, 0, 7})); - TableMetadata copy = metadata.unbuild().extensions(extensions).build(); + TableMetadata copy = metadata.unbuild().extensions(extensions).build(); - updateTable(keyspace, metadata, copy); + updateTable(keyspace, metadata, copy); - metadata = Schema.instance.getTableMetadata(keyspace, "test"); - assertEquals(extensions, metadata.params.extensions); + metadata = Schema.instance.getTableMetadata(keyspace, "test"); + assertEquals(extensions, metadata.params.extensions); + }, true); // Validate metadata for each mode } @Test @@ -218,12 +297,13 @@ public void testMetricsExtensions() } @Test - public void testReadRepair() + public void testReadRepair() throws Exception { - createTable("ks", "CREATE TABLE tbl (a text primary key, b int, c int) WITH read_repair='none'"); - TableMetadata metadata = Schema.instance.getTableMetadata("ks", "tbl"); - Assert.assertEquals(ReadRepairStrategy.NONE, metadata.params.readRepair); - + runWithAllCompatibilityModes(mode -> { + createTable("ks", "CREATE TABLE tbl (a text primary key, b int, c int) WITH read_repair='none'"); + TableMetadata metadata = Schema.instance.getTableMetadata("ks", "tbl"); + Assert.assertEquals(ReadRepairStrategy.NONE, metadata.params.readRepair); + }, true); // Validate metadata for each mode } @Test