CNDB-15995 Preserve "default" configuration class name when serializing memtable params for CC4 compatibility. #2138
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ng memtable params for CC4 compatibility.
Checklist before you submit for review
|
…mtable configuration in CC4 compatibility mode.
Author
|
@driftx I will have to wait for the artifactory license issue to be resolved before I can get build and test results, but I added you as reviewer in the meantime anyway. |
|
driftx
approved these changes
Nov 21, 2025
djatnieks
added a commit
that referenced
this pull request
Nov 24, 2025
…hould be an empty map (#2139) ### What is the issue Another follow up fix for CNDB-15995. ### What does this PR fix and why was it fixed The previous follow up to CNDB-15995 > CNDB-15995 Preserve "default" configuration class name when serializing memtable params for CC4 compatibility. (#2138) ef37cba Daniel Jatnieks <jatnieks@pobox.com> Nov 21, 2025 at 12:32 PM fixed CNDB tests `SchemaJsonSerDeTest` and `SchemaTrackerTest.smokeTest` that failed in the CNDB side PR related to this issue (CNDB-15995). The issue was around round-trip serialization/deserialization of schema's being the same. If the original schema does not include any `memtable =` option, the default configuration is used, which in CC5 is `trie`. When CNDB runs with `cndb.storage_compatibility_mode = CC_4` then this was serialized back as `memtable = {'class' : 'trie'}`, but then it doesn't match the original schema. Commit `ef37cba4b8` addressed by recognizing that `trie` is the default configuration and instead serializing as `memtable = {'class' : 'default'}` which the CC5 memtable processing of CC4 style map options is able to process, and fixed that as far as that goes. However, this created a new problem because when CNDB migration tests, such as `CDCMigrationTest#testMixedVersionWritersWithCurrentVersionServices` execute, they will send the CC4 version schema to an actual CC4 C* instance. The problem is that CC4 does not recognize `memtable = {'class' : 'default'}` and throws an exception. This new change changes the serialization of `CC_4` compatibility mode to still recognize that a `trie` configuration is the default, but now serializes back as `memtable = {}`. The intent is to preserve the *default* meaning. e.g. `trie`, or some other config, is being used because it was the "default". Serialization should reflect that the memtable configuration is the "default" too and whichever C* version consumes that schema can interpret what default means.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is the issue
Follow up fix for CNDB-15995 so that a CC4 schema using memtable param
{'class' : 'default'}will be de-serialized in the same way, instead of whichever memtable configuration class actually represents "default".E.g. CNDB, uses
StorageCompatibilityMode.CC_4. When it uses a CC4 style memtable param such as{'class' : 'default'}, CC 5.0 knows how to handle this map representation anddefaultwill use the memtable configuration that is defined asdefault, which happens to beTrieMemtableortrie. Later, when this schema is deserialized, the original implementation of CNDB-15995 inMemtableParams.toMapForCC4handled this by producing{'class' : 'trie'}, but the CNDB tests are expecting that this should be consistent with the original{'class' : 'default'}that was used when the table was created.What does this PR fix and why was it fixed
The fix here is to special case the handling of the
defaultmemtable configuration inMemtableParams.toMapForCC4to recognize thedefaultconfiguration and deserialize it as{'class' : 'default'}.