CNDB-17010: Fix CC4→CC5 memtable configuration loss during upgrade#2261
CNDB-17010: Fix CC4→CC5 memtable configuration loss during upgrade#2261
Conversation
CC4 stored the memtable column in system_schema.tables as frozen<map<text, text>>, 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
Checklist before you submit for review
|
|
I think that tables created with CC5 and the |
|
I think we could do this with SCM at the schema level rather than at read time, such that we maintain the CC4 map approach when SCM is < 5 and then upgrade it away from the map when SCM is changed. This would be similar (but more involved) to the way we delay other schema changes with SCM now, and allows us to avoid the byte sniffing to detect the correct format. WDYT? |
Yes, that would be much better - and resolve the downgrade concern. I overlooked existing use of SCM to delay schema changes initially, but see them there now and give that at try. |
CC5 changed the memtable column in system_schema.tables/views from frozen<map<text, text>> (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 (abdbcf1) handles CC4→CC5 upgrades - Writing: This commit handles CC5→CC4 downgrades
|
❌ Build ds-cassandra-pr-gate/PR-2261 rejected by Butler7 regressions found Found 7 new test failures
Found 3 known test failures |
|
|
||
| // Validate and map the configuration key to CC4 class name | ||
| String className = mapCC5KeyToCC4ClassName(configurationKey); | ||
| return ImmutableMap.of("class", className); |
There was a problem hiding this comment.
Aren't we losing the memtable parameters if we just return the class?
| private static final List<TableMetadata> ALL_TABLE_METADATA = ImmutableList.of( | ||
| Keyspaces, | ||
| // Use legacy schema (frozen<map>) when in CC_4 compatibility mode to support downgrade | ||
| DatabaseDescriptor.getStorageCompatibilityMode().isBefore(CassandraVersion.CASSANDRA_5_0.major) ? TablesLegacy : Tables, |
There was a problem hiding this comment.
If we do this in static init I don't think we'll be able to change SCM later. Maybe this won't get accessed again but I'm not certain.
| return "PersistentMemoryMemtable"; | ||
| default: | ||
| // For unknown types, capitalize first letter | ||
| return configKey.substring(0, 1).toUpperCase() + configKey.substring(1) + "Memtable"; |
There was a problem hiding this comment.
Considering 'TrieMemtableStage1' doesn't end in 'Memtable', maybe we should pull the definition from CONFIGURATION_DEFINITIONS and get the class_name from that instead of synthesizing?
Replace hardcoded switch statement with dynamic lookup from CONFIGURATION_DEFINITIONS. Get actual class name from definition.class_name instead of synthesizing it.
3 participants
What is the issue
CNDB-17010
What does this PR fix and why was it fixed
CC4 stored the memtable column in system_schema.tables as frozen<map<text, text>>, 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: