CASSANDRA-20349 Accord: Serialization Improvements#3910
Merged
belliottsmith merged 1 commit intoapache:cep-15-accordfrom Feb 22, 2025
Merged
CASSANDRA-20349 Accord: Serialization Improvements#3910belliottsmith merged 1 commit intoapache:cep-15-accordfrom
belliottsmith merged 1 commit intoapache:cep-15-accordfrom
Conversation
belliottsmith
force-pushed
the
cleanup-14feb25
branch
3 times, most recently
from
ef15c8e to
a79e262
Compare
ifesdjeen
approved these changes
Feb 20, 2025
| if (buffer.remaining() < 8) | ||
| return super.readLeastSignificantBytes(bytes); | ||
|
|
||
| long retval = buffer.getLong(buffer.position()); |
Contributor
There was a problem hiding this comment.
nit: should we add an assert it is exactly 8 bytes only?
| serialize(flags, out, userVersion); | ||
| } | ||
|
|
||
| private void serialize(int flags, DataOutputPlus out, int userVersion) throws IOException |
Contributor
There was a problem hiding this comment.
Should we maybe use this one instead of serialize(Command command, int flags, DataOutputPlus out, int userVersion) too so that we do not have this logic twice? Essentially apply command to builder, and then serialize.
| storeId = accessor.getStoreId(partitionKeyComponents); | ||
| partitionKey = accessor.getKey(partitionKeyComponents); | ||
| ByteBuffer key = partition.partitionKey().getKey(); | ||
| storeId = accessor.getStoreId(key); |
Contributor
There was a problem hiding this comment.
nit: this is a static method now
| else | ||
| { | ||
| int pos = dst.position() + offset; | ||
| dst.putLong(pos, register << (64 - (bytes * 8))); |
Contributor
There was a problem hiding this comment.
nit: add invariant for number of bytes?
| else | ||
| { | ||
| int pos = dst.position() + offset; | ||
| return dst.getLong(pos) >>> (64 - (bytes * 8)); |
Contributor
There was a problem hiding this comment.
nit: add invariant for a maximum number of bytes?
| case 2: return accessor.getShort(dst, offset); | ||
| case 3: | ||
| return ((long)accessor.getShort(dst, offset) << 8) | ||
| | (long)accessor.getByte(dst, offset + 1); |
Contributor
There was a problem hiding this comment.
I think we should have + 2 here.
| AccordRoutingKey end = end(); | ||
| Token left = start instanceof SentinelKey ? partitioner.getMinimumToken() : start.token(); | ||
| Token right = end instanceof SentinelKey ? partitioner.getMinimumToken() : end.token(); | ||
| TokenKey start = start(); |
Contributor
There was a problem hiding this comment.
Oh, nice. This has bothered me quite a bit, but I was not sure what SentinelKey was at that moment.
| @Test | ||
| public void serde() | ||
| { | ||
| qt().withSeed(0L).forAll(fromQT(partitioners().assuming(IPartitioner::accordSupported)).flatMap(partitioner -> routingKeyGen(fromQT(CassandraGenerators.TABLE_ID_GEN), fromQT(token(partitioner)), partitioner))) |
| } | ||
|
|
||
| public abstract static class AbstractRangesSerializer<RS extends AbstractRanges> implements IVersionedSerializer<RS> | ||
| // this serializer is designed to permits using the collection in its serialized form with minimal in-memory state. |
| final IntFunction<K[]> allocate; | ||
|
|
||
| public AbstractKeysSerializer(IVersionedSerializer<K> keySerializer, IntFunction<K[]> allocate) | ||
| public AbstractKeysSerializer(AccordKeySerializer<K> keySerializer, IntFunction<K[]> allocate) |
Contributor
There was a problem hiding this comment.
nit: on L553, we can use skip instead of deserialize now that skip is available
belliottsmith
force-pushed
the
cleanup-14feb25
branch
4 times, most recently
from
aa95eee to
568a578
Compare
Improve: - Introduce pre/accept fast execution flags - Introduce searchable Deps serialization - Flatten AccordRoutingKey(s) into single type, using sentinel bits - Introduce new fast byte-comparable serialization methods for Token and TableId to support above Fix: - Fix journal re-serialization logic - Enable RandomPartitioner for Accord by supporting fixed-width serialization for RouteIndex
belliottsmith
force-pushed
the
cleanup-14feb25
branch
from
568a578 to
579e0d8
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Thanks for sending a pull request! Here are some tips if you're new here:
Commit messages should follow the following format:
The Cassandra Jira