From abdbcf19a7fd2a08e41b539a90cd0bc232cb9a34 Mon Sep 17 00:00:00 2001 From: Daniel Jatnieks Date: Fri, 6 Mar 2026 17:33:19 -0800 Subject: [PATCH 01/11] =?UTF-8?q?CNDB-17010:=20Fix=20CC4=E2=86=92CC5=20mem?= =?UTF-8?q?table=20configuration=20loss=20during=20upgrade?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CC4 stored the memtable column in system_schema.tables as frozen>, while CC5 uses text. During upgrades, binary-serialized map data is misinterpreted as UTF-8 text, causing memtable configurations to fall back to defaults. Changes: - Add MemtableParams.getWithCC4Fallback() to detect and parse binary map data using null-byte heuristic - Add mapCC4ClassNameToCC5Key() to map CC4 class names (TrieMemtable, SkipListMemtable) to CC5 config keys - Update SchemaKeyspace.createTableParamsFromRow() to use new compatibility method --- .../cassandra/schema/MemtableParams.java | 93 +++++++++- .../cassandra/schema/SchemaKeyspace.java | 5 +- .../cassandra/schema/MemtableParamsTest.java | 168 ++++++++++++++++++ 3 files changed, 259 insertions(+), 7 deletions(-) diff --git a/src/java/org/apache/cassandra/schema/MemtableParams.java b/src/java/org/apache/cassandra/schema/MemtableParams.java index bd1f1b3f4645..ce1994727c6e 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,11 +31,15 @@ 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; @@ -50,6 +55,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 +171,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 +310,83 @@ 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 frozen>, while CC5 uses text. + * This method detects binary-serialized map data (containing null bytes) and converts it. + */ + public static MemtableParams getWithCC4Fallback(UntypedResultSet.Row row, String columnName) + { + if (!row.has(columnName)) + return DEFAULT; + + // Try to get as string first + String stringValue = row.getString(columnName); + + // Check if this looks like binary data (contains null bytes from CC4's map serialization) + if (stringValue != null && stringValue.indexOf('\0') >= 0) + { + // 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; + } + } + + // Normal CC5 string value + return getWithFallback(stringValue); + } + + 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(); + } + } } diff --git a/src/java/org/apache/cassandra/schema/SchemaKeyspace.java b/src/java/org/apache/cassandra/schema/SchemaKeyspace.java index b897bf52b6c2..eff096fdda1f 100644 --- a/src/java/org/apache/cassandra/schema/SchemaKeyspace.java +++ b/src/java/org/apache/cassandra/schema/SchemaKeyspace.java @@ -1077,9 +1077,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/test/unit/org/apache/cassandra/schema/MemtableParamsTest.java b/test/unit/org/apache/cassandra/schema/MemtableParamsTest.java index d3fc8befc22a..4fa69ad56338 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,14 @@ 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 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 +284,166 @@ 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); + } } From 68b5e114e51eb0781e0145c0bd47248b2f3021c8 Mon Sep 17 00:00:00 2001 From: Daniel Jatnieks Date: Tue, 10 Mar 2026 15:41:39 -0700 Subject: [PATCH 02/11] CNDB-17010: Add CC4 downgrade support for memtable schema changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CC5 changed the memtable column in system_schema.tables/views from frozen> (CC4) to text (CC5). This prevents safe downgrades from CC5 to CC4 when storage_compatibility_mode is set. This commit adds bidirectional compatibility: - Reading: Previous commit (abdbcf19a7) handles CC4→CC5 upgrades - Writing: This commit handles CC5→CC4 downgrades --- .../cassandra/schema/MemtableParams.java | 98 +++++++++++++++- .../cassandra/schema/SchemaKeyspace.java | 107 +++++++++++++++--- .../cassandra/schema/MemtableParamsTest.java | 78 +++++++++++++ 3 files changed, 268 insertions(+), 15 deletions(-) diff --git a/src/java/org/apache/cassandra/schema/MemtableParams.java b/src/java/org/apache/cassandra/schema/MemtableParams.java index ce1994727c6e..dc8ad12992e9 100644 --- a/src/java/org/apache/cassandra/schema/MemtableParams.java +++ b/src/java/org/apache/cassandra/schema/MemtableParams.java @@ -43,6 +43,7 @@ import org.apache.cassandra.db.memtable.Memtable; import org.apache.cassandra.db.memtable.TrieMemtableFactory; import org.apache.cassandra.exceptions.ConfigurationException; +import org.apache.cassandra.utils.CassandraVersion; /** * Memtable types and options are specified with these parameters. Memtable classes must either contain a static @@ -312,7 +313,7 @@ private static Memtable.Factory getMemtableFactory(ParameterizedClass options) } /** * Attempts to read memtable configuration, with fallback for CC4 upgrade compatibility. - * CC4 stored memtable as frozen>, while CC5 uses text. + * CC4 stored memtable as {@code frozen>}, while CC5 uses text. * This method detects binary-serialized map data (containing null bytes) and converts it. */ public static MemtableParams getWithCC4Fallback(UntypedResultSet.Row row, String columnName) @@ -389,4 +390,99 @@ private static String mapCC4ClassNameToCC5Key(String cc4ClassName) return shortName.toLowerCase(); } } + + + + /** + * Returns the memtable value to write to schema based on storage compatibility mode. + * - In CC_4 mode: Returns a map {"class": "TrieMemtable"} for CC4 compatibility + * - In CC5 mode: Returns the configuration key as text ("trie") + * + * @return Object to write (either {@code Map} or String) + */ + public Object asSchemaValue() + { + return asSchemaValue(DatabaseDescriptor.getStorageCompatibilityMode()); + } + + /** + * Returns the memtable value to write to schema based on the provided storage compatibility mode. + * This overload exists for testing purposes. + * + * @param mode The storage compatibility mode to use + * @return Object to write (either {@code Map} or String) + * @throws ConfigurationException if the memtable type is not compatible with the requested mode + */ + @VisibleForTesting + Object asSchemaValue(org.apache.cassandra.utils.StorageCompatibilityMode mode) + { + if (mode.isBefore(CassandraVersion.CASSANDRA_5_0.major)) + { + // CC4 compatibility mode: write as map + // CC4 writes empty map {} for "default" configuration + if ("default".equals(configurationKey)) + return ImmutableMap.of(); + + // Validate and map the configuration key to CC4 class name + String className = mapCC5KeyToCC4ClassName(configurationKey); + return ImmutableMap.of("class", className); + } + else + { + // CC5 mode: write as text + 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 + if (!CONFIGURATION_DEFINITIONS.containsKey(configKey)) + { + throw new ConfigurationException( + String.format("Memtable configuration '%s' not found in cassandra.yaml. " + + "Cannot write to schema in CC4 compatibility mode.", + configKey)); + } + + // Map to CC4 class name + switch (lowerKey) + { + case "skiplist": + return "SkipListMemtable"; + case "trie": + return "TrieMemtable"; + case "triememtablestage1": + return "TrieMemtableStage1"; + case "persistentmemorymemtable": + return "PersistentMemoryMemtable"; + default: + // For unknown types, capitalize first letter + return configKey.substring(0, 1).toUpperCase() + configKey.substring(1) + "Memtable"; + } + } } diff --git a/src/java/org/apache/cassandra/schema/SchemaKeyspace.java b/src/java/org/apache/cassandra/schema/SchemaKeyspace.java index eff096fdda1f..6fe749888d0e 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,20 @@ 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); + private static final List ALL_TABLE_METADATA = ImmutableList.of( + Keyspaces, + // Use legacy schema (frozen) when in CC_4 compatibility mode to support downgrade + DatabaseDescriptor.getStorageCompatibilityMode().isBefore(CassandraVersion.CASSANDRA_5_0.major) ? TablesLegacy : Tables, + Columns, + ColumnMasks, + Triggers, + DroppedColumns, + // Use legacy schema (frozen) when in CC_4 compatibility mode to support downgrade + DatabaseDescriptor.getStorageCompatibilityMode().isBefore(CassandraVersion.CASSANDRA_5_0.major) ? ViewsLegacy : Views, + Types, + Functions, + Aggregates, + Indexes); private static TableMetadata parse(String name, String description, String cql) { @@ -619,8 +697,9 @@ 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()); + builder.add("memtable", params.memtable.asSchemaValue()); // 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 diff --git a/test/unit/org/apache/cassandra/schema/MemtableParamsTest.java b/test/unit/org/apache/cassandra/schema/MemtableParamsTest.java index 4fa69ad56338..18d58caafe01 100644 --- a/test/unit/org/apache/cassandra/schema/MemtableParamsTest.java +++ b/test/unit/org/apache/cassandra/schema/MemtableParamsTest.java @@ -33,6 +33,7 @@ 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; @@ -446,4 +447,81 @@ public void testNullMemtableValue() MemtableParams params = MemtableParams.getWithCC4Fallback(row, "memtable"); assertEquals(MemtableParams.DEFAULT, params); } + + // ======================================================================== + // StorageCompatibilityMode Writing Tests + // ======================================================================== + + /** + * Test that asSchemaValue() 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 testAsSchemaValueInCC4CompatibilityModes() + { + // 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 + Object defaultValue = MemtableParams.DEFAULT.asSchemaValue(mode); + assertTrue("Should return Map in " + mode + " mode", defaultValue instanceof Map); + @SuppressWarnings("unchecked") + Map defaultMap = (Map) defaultValue; + assertTrue("Default should be empty map in " + mode + " mode", defaultMap.isEmpty()); + } + } + + /** + * Test that asSchemaValue() rejects incompatible configurations in CC_4 mode. + * Tests both CC5-only types (sharded) and unknown configurations. + */ + @Test + public void testAsSchemaValueInCC4ModeRejectsIncompatibleConfigurations() + { + // Test 1: Sharded memtables (CC5-only, don't exist in CC4) + MemtableParams shardedParams = MemtableParams.forTesting(MemtableParams.DEFAULT.factory(), "sharded-skiplist"); + try + { + shardedParams.asSchemaValue(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.asSchemaValue(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 asSchemaValue() returns a String in CC5 mode (NONE). + * This is the normal CC5 operation. + */ + @Test + public void testAsSchemaValueInCC5Mode() + { + // Test DEFAULT memtable in CC5 mode + Object defaultValue = MemtableParams.DEFAULT.asSchemaValue(StorageCompatibilityMode.NONE); + assertTrue("Should return String in CC5 mode", defaultValue instanceof String); + assertEquals("default", defaultValue); + } + } From 74be7bf52334e7d68d6e5ffb33590bc7c8c26ea8 Mon Sep 17 00:00:00 2001 From: Daniel Jatnieks Date: Wed, 11 Mar 2026 15:06:56 -0700 Subject: [PATCH 03/11] Add strong typing for memtable schema value methods --- .../cassandra/schema/MemtableParams.java | 89 +++++++++++++------ .../cassandra/schema/SchemaKeyspace.java | 7 +- .../cassandra/schema/MemtableParamsTest.java | 43 ++++++--- 3 files changed, 98 insertions(+), 41 deletions(-) diff --git a/src/java/org/apache/cassandra/schema/MemtableParams.java b/src/java/org/apache/cassandra/schema/MemtableParams.java index dc8ad12992e9..80f14210905a 100644 --- a/src/java/org/apache/cassandra/schema/MemtableParams.java +++ b/src/java/org/apache/cassandra/schema/MemtableParams.java @@ -311,10 +311,27 @@ 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. - * This method detects binary-serialized map data (containing null bytes) and converts it. + * 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: + *
    + *
  • Upgrading from CC4 to CC5 (reads CC4 format, writes CC5 format)
    • + *
  • Running CC5 in CC_4 mode (reads either format, writes CC4 format)
    • + *
  • Mixed clusters during rolling upgrades (reads both formats)
    • + *
+ * + * @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) { @@ -391,47 +408,67 @@ private static String mapCC4ClassNameToCC5Key(String cc4ClassName) } } + /** + * 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(org.apache.cassandra.utils.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 + String className = mapCC5KeyToCC4ClassName(configurationKey); + return ImmutableMap.of("class", className); + } /** - * Returns the memtable value to write to schema based on storage compatibility mode. - * - In CC_4 mode: Returns a map {"class": "TrieMemtable"} for CC4 compatibility - * - In CC5 mode: Returns the configuration key as text ("trie") + * Returns the memtable value as text for CC5 mode. + * Used when storage_compatibility_mode is NONE. * - * @return Object to write (either {@code Map} or String) + * @return String representation for CC5 schema (text) */ - public Object asSchemaValue() + public String asSchemaValueText() { - return asSchemaValue(DatabaseDescriptor.getStorageCompatibilityMode()); + return asSchemaValueText(DatabaseDescriptor.getStorageCompatibilityMode()); } /** - * Returns the memtable value to write to schema based on the provided storage compatibility mode. + * Returns the memtable value as text for CC5 mode. * This overload exists for testing purposes. * * @param mode The storage compatibility mode to use - * @return Object to write (either {@code Map} or String) - * @throws ConfigurationException if the memtable type is not compatible with the requested mode + * @return String representation for CC5 schema (text) + * @throws IllegalStateException if called in CC4 compatibility mode */ @VisibleForTesting - Object asSchemaValue(org.apache.cassandra.utils.StorageCompatibilityMode mode) + String asSchemaValueText(org.apache.cassandra.utils.StorageCompatibilityMode mode) { if (mode.isBefore(CassandraVersion.CASSANDRA_5_0.major)) - { - // CC4 compatibility mode: write as map - // CC4 writes empty map {} for "default" configuration - if ("default".equals(configurationKey)) - return ImmutableMap.of(); - - // Validate and map the configuration key to CC4 class name - String className = mapCC5KeyToCC4ClassName(configurationKey); - return ImmutableMap.of("class", className); - } - else - { - // CC5 mode: write as text - return configurationKey; - } + throw new IllegalStateException("Cannot get text value in CC4 compatibility mode. Use asSchemaValueMap() instead."); + + return configurationKey; } /** diff --git a/src/java/org/apache/cassandra/schema/SchemaKeyspace.java b/src/java/org/apache/cassandra/schema/SchemaKeyspace.java index 6fe749888d0e..84c22bdc1f58 100644 --- a/src/java/org/apache/cassandra/schema/SchemaKeyspace.java +++ b/src/java/org/apache/cassandra/schema/SchemaKeyspace.java @@ -699,7 +699,12 @@ private static void addTableParamsToRowBuilder(TableParams params, Row.SimpleBui // 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.asSchemaValue()); + { + 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 diff --git a/test/unit/org/apache/cassandra/schema/MemtableParamsTest.java b/test/unit/org/apache/cassandra/schema/MemtableParamsTest.java index 18d58caafe01..0d42b054d785 100644 --- a/test/unit/org/apache/cassandra/schema/MemtableParamsTest.java +++ b/test/unit/org/apache/cassandra/schema/MemtableParamsTest.java @@ -453,38 +453,45 @@ public void testNullMemtableValue() // ======================================================================== /** - * Test that asSchemaValue() returns a Map in CC_4 and CASSANDRA_4 compatibility modes. + * 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 testAsSchemaValueInCC4CompatibilityModes() + 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 - Object defaultValue = MemtableParams.DEFAULT.asSchemaValue(mode); - assertTrue("Should return Map in " + mode + " mode", defaultValue instanceof Map); - @SuppressWarnings("unchecked") - Map defaultMap = (Map) defaultValue; + 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 asSchemaValue() rejects incompatible configurations in CC_4 mode. + * 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 testAsSchemaValueInCC4ModeRejectsIncompatibleConfigurations() + 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.asSchemaValue(StorageCompatibilityMode.CC_4); + shardedParams.asSchemaValueMap(StorageCompatibilityMode.CC_4); fail("Should have thrown ConfigurationException for sharded memtable in CC_4 mode"); } catch (ConfigurationException e) @@ -499,7 +506,7 @@ public void testAsSchemaValueInCC4ModeRejectsIncompatibleConfigurations() MemtableParams unknownParams = MemtableParams.forTesting(MemtableParams.DEFAULT.factory(), "unknown-memtable-type"); try { - unknownParams.asSchemaValue(StorageCompatibilityMode.CC_4); + unknownParams.asSchemaValueMap(StorageCompatibilityMode.CC_4); fail("Should have thrown ConfigurationException for unknown configuration in CC_4 mode"); } catch (ConfigurationException e) @@ -512,16 +519,24 @@ public void testAsSchemaValueInCC4ModeRejectsIncompatibleConfigurations() } /** - * Test that asSchemaValue() returns a String in CC5 mode (NONE). + * Test that asSchemaValueText() returns a String in CC5 mode (NONE). * This is the normal CC5 operation. */ @Test - public void testAsSchemaValueInCC5Mode() + public void testAsSchemaValueTextInCC5Mode() { // Test DEFAULT memtable in CC5 mode - Object defaultValue = MemtableParams.DEFAULT.asSchemaValue(StorageCompatibilityMode.NONE); - assertTrue("Should return String in CC5 mode", defaultValue instanceof String); + 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); + } + } From 9b16fd38d7ab58eca882ad55deadd90c75f90da4 Mon Sep 17 00:00:00 2001 From: Daniel Jatnieks Date: Thu, 12 Mar 2026 18:51:34 -0700 Subject: [PATCH 04/11] Refactor memtable CC4 mapping to use definition class names Replace hardcoded switch statement with dynamic lookup from CONFIGURATION_DEFINITIONS. Get actual class name from definition.class_name instead of synthesizing it. --- .../cassandra/schema/MemtableParams.java | 50 ++++++++++++------- .../utils/StorageCompatibilityMode.java | 8 ++- 2 files changed, 40 insertions(+), 18 deletions(-) diff --git a/src/java/org/apache/cassandra/schema/MemtableParams.java b/src/java/org/apache/cassandra/schema/MemtableParams.java index 80f14210905a..d0cd2f64e777 100644 --- a/src/java/org/apache/cassandra/schema/MemtableParams.java +++ b/src/java/org/apache/cassandra/schema/MemtableParams.java @@ -44,6 +44,7 @@ import org.apache.cassandra.db.memtable.TrieMemtableFactory; import org.apache.cassandra.exceptions.ConfigurationException; 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 @@ -429,7 +430,7 @@ public Map asSchemaValueMap() * @throws ConfigurationException if the memtable type is not compatible with CC4 */ @VisibleForTesting - Map asSchemaValueMap(org.apache.cassandra.utils.StorageCompatibilityMode mode) + 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."); @@ -439,8 +440,19 @@ Map asSchemaValueMap(org.apache.cassandra.utils.StorageCompatibi 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); - return ImmutableMap.of("class", className); + + // 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; } /** @@ -463,7 +475,7 @@ public String asSchemaValueText() * @throws IllegalStateException if called in CC4 compatibility mode */ @VisibleForTesting - String asSchemaValueText(org.apache.cassandra.utils.StorageCompatibilityMode mode) + 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."); @@ -498,7 +510,8 @@ private static String mapCC5KeyToCC4ClassName(String configKey) // Check if the configuration key exists in CONFIGURATION_DEFINITIONS // This ensures we're not trying to write an unknown/invalid configuration - if (!CONFIGURATION_DEFINITIONS.containsKey(configKey)) + ParameterizedClass definition = CONFIGURATION_DEFINITIONS.get(configKey); + if (definition == null) { throw new ConfigurationException( String.format("Memtable configuration '%s' not found in cassandra.yaml. " + @@ -506,20 +519,23 @@ private static String mapCC5KeyToCC4ClassName(String configKey) configKey)); } - // Map to CC4 class name - switch (lowerKey) + // 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()) { - case "skiplist": - return "SkipListMemtable"; - case "trie": - return "TrieMemtable"; - case "triememtablestage1": - return "TrieMemtableStage1"; - case "persistentmemorymemtable": - return "PersistentMemoryMemtable"; - default: - // For unknown types, capitalize first letter - return configKey.substring(0, 1).toUpperCase() + configKey.substring(1) + "Memtable"; + 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/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), From 533b5240ced1e29b140f407463751a89e65967fc Mon Sep 17 00:00:00 2001 From: Daniel Jatnieks Date: Fri, 13 Mar 2026 09:53:10 -0700 Subject: [PATCH 05/11] Replace ALL_TABLE_METADATA static variable with allTableMetadata method. --- .../cassandra/schema/SchemaKeyspace.java | 41 +++++++++++-------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/src/java/org/apache/cassandra/schema/SchemaKeyspace.java b/src/java/org/apache/cassandra/schema/SchemaKeyspace.java index 84c22bdc1f58..233fd3d0839f 100644 --- a/src/java/org/apache/cassandra/schema/SchemaKeyspace.java +++ b/src/java/org/apache/cassandra/schema/SchemaKeyspace.java @@ -364,20 +364,29 @@ private SchemaKeyspace() + "deterministic boolean," + "PRIMARY KEY ((keyspace_name), aggregate_name, argument_types))"); - private static final List ALL_TABLE_METADATA = ImmutableList.of( - Keyspaces, - // Use legacy schema (frozen) when in CC_4 compatibility mode to support downgrade - DatabaseDescriptor.getStorageCompatibilityMode().isBefore(CassandraVersion.CASSANDRA_5_0.major) ? TablesLegacy : Tables, - Columns, - ColumnMasks, - Triggers, - DroppedColumns, - // Use legacy schema (frozen) when in CC_4 compatibility mode to support downgrade - DatabaseDescriptor.getStorageCompatibilityMode().isBefore(CassandraVersion.CASSANDRA_5_0.major) ? ViewsLegacy : Views, - Types, - Functions, - Aggregates, - Indexes); + /** + * Returns the list of schema table metadata based on current storage compatibility mode. + * This must be dynamic (not static final) because storage_compatibility_mode can change + * via rolling restart, and we need to use the correct schema table definitions (legacy + * vs current) based on the mode at the time of access. + */ + private static List allTableMetadata() + { + boolean useLegacySchema = DatabaseDescriptor.getStorageCompatibilityMode().isBefore(CassandraVersion.CASSANDRA_5_0.major); + return ImmutableList.of(Keyspaces, + // Use legacy schema (frozen) when in CC_4 compatibility mode to support downgrade + useLegacySchema ? TablesLegacy : Tables, + Columns, + ColumnMasks, + Triggers, + DroppedColumns, + // Use legacy schema (frozen) when in CC_4 compatibility mode to support downgrade + useLegacySchema ? ViewsLegacy : Views, + Types, + Functions, + Aggregates, + Indexes); + } private static TableMetadata parse(String name, String description, String cql) { @@ -391,7 +400,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) @@ -616,7 +625,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; From ae46f7fa600f39efa299fcb89a26cd71b2e78cdd Mon Sep 17 00:00:00 2001 From: Daniel Jatnieks Date: Fri, 13 Mar 2026 09:56:26 -0700 Subject: [PATCH 06/11] Shorten comment --- src/java/org/apache/cassandra/schema/SchemaKeyspace.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/java/org/apache/cassandra/schema/SchemaKeyspace.java b/src/java/org/apache/cassandra/schema/SchemaKeyspace.java index 233fd3d0839f..df3da2c6dda5 100644 --- a/src/java/org/apache/cassandra/schema/SchemaKeyspace.java +++ b/src/java/org/apache/cassandra/schema/SchemaKeyspace.java @@ -366,9 +366,6 @@ private SchemaKeyspace() /** * Returns the list of schema table metadata based on current storage compatibility mode. - * This must be dynamic (not static final) because storage_compatibility_mode can change - * via rolling restart, and we need to use the correct schema table definitions (legacy - * vs current) based on the mode at the time of access. */ private static List allTableMetadata() { From e880e9bbe298abbbb55c90d2e596824e3a97bf6b Mon Sep 17 00:00:00 2001 From: Daniel Jatnieks Date: Wed, 18 Mar 2026 11:02:52 -0700 Subject: [PATCH 07/11] Replace direct Tables and Views refs with method tablesTableMetadata and viewTablesMetadata that use SCM to determine the correct table/view metadata to use. --- .../cassandra/schema/SchemaKeyspace.java | 36 ++++++++++++++----- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/src/java/org/apache/cassandra/schema/SchemaKeyspace.java b/src/java/org/apache/cassandra/schema/SchemaKeyspace.java index df3da2c6dda5..7cc4e83869e6 100644 --- a/src/java/org/apache/cassandra/schema/SchemaKeyspace.java +++ b/src/java/org/apache/cassandra/schema/SchemaKeyspace.java @@ -369,22 +369,42 @@ private SchemaKeyspace() */ private static List allTableMetadata() { - boolean useLegacySchema = DatabaseDescriptor.getStorageCompatibilityMode().isBefore(CassandraVersion.CASSANDRA_5_0.major); return ImmutableList.of(Keyspaces, // Use legacy schema (frozen) when in CC_4 compatibility mode to support downgrade - useLegacySchema ? TablesLegacy : Tables, + tablesTableMetadata(), Columns, ColumnMasks, Triggers, DroppedColumns, - // Use legacy schema (frozen) when in CC_4 compatibility mode to support downgrade - useLegacySchema ? ViewsLegacy : Views, + viewsTableMetadata(), Types, Functions, Aggregates, Indexes); } + /** + * Returns the appropriate Tables schema table metadata based on current storage compatibility mode. + * Uses TablesLegacy (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 (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) { return CreateTableStatement.parse(format(cql, name), SchemaConstants.SCHEMA_KEYSPACE_NAME) @@ -652,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()) @@ -813,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); @@ -922,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) @@ -945,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()) From 99f6a04fb8efa0f4ac2a0bcc3673029ada33c206 Mon Sep 17 00:00:00 2001 From: Daniel Jatnieks Date: Wed, 18 Mar 2026 12:16:53 -0700 Subject: [PATCH 08/11] Wrap javadoc text with @code tag to avoid checkstyle complaints --- src/java/org/apache/cassandra/schema/SchemaKeyspace.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/java/org/apache/cassandra/schema/SchemaKeyspace.java b/src/java/org/apache/cassandra/schema/SchemaKeyspace.java index 7cc4e83869e6..9528da54f0bf 100644 --- a/src/java/org/apache/cassandra/schema/SchemaKeyspace.java +++ b/src/java/org/apache/cassandra/schema/SchemaKeyspace.java @@ -385,7 +385,7 @@ private static List allTableMetadata() /** * Returns the appropriate Tables schema table metadata based on current storage compatibility mode. - * Uses TablesLegacy (frozen for memtable) in CC4 mode, Tables (text for memtable) otherwise. + * Uses TablesLegacy ({@code frozen} for memtable) in CC4 mode, Tables (text for memtable) otherwise. */ private static TableMetadata tablesTableMetadata() { @@ -396,7 +396,7 @@ private static TableMetadata tablesTableMetadata() /** * Returns the appropriate Views schema table metadata based on current storage compatibility mode. - * Uses ViewsLegacy (frozen for memtable) in CC4 mode, Views (text for memtable) otherwise. + * Uses ViewsLegacy ({@code frozen} for memtable) in CC4 mode, Views (text for memtable) otherwise. */ private static TableMetadata viewsTableMetadata() { From 38c3f030ad077dcf13a6c3a9e0611c091b175df4 Mon Sep 17 00:00:00 2001 From: Daniel Jatnieks Date: Thu, 19 Mar 2026 10:38:20 -0700 Subject: [PATCH 09/11] Add SCM-aware testing for schema table selection Tests now run in all compatibility modes (NONE, CC_4, CASSANDRA_4) and validate correct schema table metadata selection. --- .../cassandra/schema/SchemaKeyspaceTest.java | 132 ++++++++++++++---- 1 file changed, 106 insertions(+), 26 deletions(-) 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 From a498d2e47a036e74e0e171edc9c43b8e79c9e02f Mon Sep 17 00:00:00 2001 From: Daniel Jatnieks Date: Thu, 19 Mar 2026 10:38:37 -0700 Subject: [PATCH 10/11] Remove comment --- src/java/org/apache/cassandra/schema/MemtableParams.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/java/org/apache/cassandra/schema/MemtableParams.java b/src/java/org/apache/cassandra/schema/MemtableParams.java index d0cd2f64e777..08eb062f223f 100644 --- a/src/java/org/apache/cassandra/schema/MemtableParams.java +++ b/src/java/org/apache/cassandra/schema/MemtableParams.java @@ -339,7 +339,6 @@ public static MemtableParams getWithCC4Fallback(UntypedResultSet.Row row, String if (!row.has(columnName)) return DEFAULT; - // Try to get as string first String stringValue = row.getString(columnName); // Check if this looks like binary data (contains null bytes from CC4's map serialization) From 0c2d7efac4dccf061a27c225216d99f50b58558c Mon Sep 17 00:00:00 2001 From: Daniel Jatnieks Date: Thu, 19 Mar 2026 12:37:01 -0700 Subject: [PATCH 11/11] Wrap row.getString in a try/catch in case CC4 Map value binary data is not valid UTF-8 and raises an exception. --- .../cassandra/schema/MemtableParams.java | 85 +++++++++++-------- 1 file changed, 50 insertions(+), 35 deletions(-) diff --git a/src/java/org/apache/cassandra/schema/MemtableParams.java b/src/java/org/apache/cassandra/schema/MemtableParams.java index 08eb062f223f..145d9a5d5988 100644 --- a/src/java/org/apache/cassandra/schema/MemtableParams.java +++ b/src/java/org/apache/cassandra/schema/MemtableParams.java @@ -43,6 +43,7 @@ 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; @@ -339,52 +340,66 @@ public static MemtableParams getWithCC4Fallback(UntypedResultSet.Row row, String if (!row.has(columnName)) return DEFAULT; - String stringValue = row.getString(columnName); + 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) { - // 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); + return parseCC4MapFormat(row, columnName); + } - 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; - } + // Normal CC5 string value + return getWithFallback(stringValue); + } - // 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; - } + 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; } - catch (Exception e) + + // Convert CC4 map format to CC5 configuration key + String className = cc4Map.get("class"); + if (className != null) { - logger.warn("Failed to parse memtable column as CC4 map format, falling back to default", e); + // 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; } } - - // Normal CC5 string value - return getWithFallback(stringValue); + 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)