[
https://issues.apache.org/jira/browse/CASSANDRA-20190?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17935609#comment-17935609
]
Dmitry Konstantinov edited comment on CASSANDRA-20190 at 3/14/25 5:44 PM:
--------------------------------------------------------------------------
Before touching Memory class I have decided to trace careful the logic related
to it one more time to be sure that any changes in it would not cause some
compatibility issues.
First part: IndexSummary.offsets, serialization:
# IndexSummaryBuilder#maybeAddEntry - we start from (int) entries.length()
value
#
SafeMemoryWriter#writeInt - we write int value to SafeMemoryWriter with LE
order configured
#
new IndexSummary(partitioner, offsets.currentBuffer().sharedCopy() - we take
the underline buffer from SafeMemoryWriter as is for IndexSummary
# IndexSummary.IndexSummarySerializer#serialize - we read it back using int
offset = t.offsets.getInt(i * 4) + baseOffset;
# We do Integer.reverseBytes to switch to native order instead of a default BE
(for backward compatibility)
# java.io.DataOutput#writeInt(offset) - we write int value, writer uses BE
The table contains the tranformation during buidling + serialization to
{color:#172b4d}a file using 0x01_02_03_04 int value as an example.{color}
||Architecture||1. offset as int||2. SafeMemoryWriter (LE)||3.
IndexSummary.offsets ||4. int offsets.getInt||5. int reverseBytes||6.
out.writeInt (BE) -> disk||
|LE
Unaligned|{color:#00875a}0x01_02_03_04{color}|{color:#de350b}0x04_03_02_01{color}|{color:#de350b}0x04_03_02_01{color}|{color:#00875a}0x01_02_03_04{color}|{color:#de350b}0x04_03_02_01{color}|{color:#de350b}0x04_03_02_01
(LE){color}|
|BE Unaligned, after
CASSANDRA-17723|{color:#00875a}0x01_02_03_04{color}|{color:#de350b}0x04_03_02_01{color}|{color:#de350b}0x04_03_02_01{color}|{color:#00875a}0x01_02_03_04{color}|{color:#de350b}0x04_03_02_01{color}|{color:#de350b}0x04_03_02_01
(LE){color}|
|BE Unaligned, before
CASSANDRA-17723|{color:#00875a}0x01_02_03_04{color}|{color:#de350b}0x04_03_02_01{color}|{color:#de350b}0x04_03_02_01{color}|{color:#00875a}
{color}{color:#de350b}0x04_03_02_01{color}|
{color:#00875a}0x01_02_03_04{color}| {color:#00875a}0x01_02_03_04 (BE){color}|
|BE
Aligned|{color:#00875a}0x01_02_03_04{color}|{color:#de350b}0x04_03_02_01{color}|{color:#de350b}0x04_03_02_01{color}|{color:#00875a}0x04_03_02_01{color}|{color:#00875a}0x01_02_03_04{color}|{color:#00875a}0x01_02_03_04
(BE){color}|
If I have traced the code correctly then it looks like:
* before CASSANDRA-17723 we had native order for offsets (the same conclusion
is here -
[link|https://opensource.docs.scylladb.com/stable/architecture/sstable/sstable3/sstables-3-summary.html#summary-entries])
* after CASSANDRA-17723 (since 5.0) we have LE order for offsets
It means that we broke compatibility for offsets format in 5.0 for offsets in
index summary file format. From another side we have made it machine
independent :).
So, the question for this part - what do we plan to do with it.. - should we
preserve the new behaviour or return back to the old one?..
was (Author: dnk):
Before touching Memory class I have decided to trace careful the logic related
to it one more time to be sure that any changes in it would not cause some
compatibility issues.
First part: IndexSummary.offsets, serialization:
# IndexSummaryBuilder#maybeAddEntry - we start from (int) entries.length()
value
#
SafeMemoryWriter#writeInt - we write int value to SafeMemoryWriter with LE
order configured
#
new IndexSummary(partitioner, offsets.currentBuffer().sharedCopy() - we take
the underline buffer from SafeMemoryWriter as is for IndexSummary
# IndexSummary.IndexSummarySerializer#serialize - we read it back using int
offset = t.offsets.getInt(i * 4) + baseOffset;
# We do Integer.reverseBytes to switch to native order instead of a default BE
(for backward compatibility)
# java.io.DataOutput#writeInt(offset) - we write int value, writer uses BE
The table contains the tranformation during buidling + serialization to
{color:#172b4d}a file using 0x01_02_03_04 int value as an example.{color}
||Architecture||1. offset as int||2. SafeMemoryWriter (LE)||3.
IndexSummary.offsets ||4. int offsets.getInt||5. int reverseBytes||6.
out.writeInt (BE) -> disk||
|LE
Unaligned|{color:#00875a}0x01_02_03_04{color}|{color:#de350b}0x04_03_02_01{color}|{color:#de350b}0x04_03_02_01{color}|{color:#00875a}0x01_02_03_04{color}|{color:#de350b}0x04_03_02_01{color}|{color:#de350b}0x04_03_02_01
(LE){color}|
|BE Unaligned, after
CASSANDRA-17723|{color:#00875a}0x01_02_03_04{color}|{color:#de350b}0x04_03_02_01{color}|{color:#de350b}0x04_03_02_01{color}|{color:#00875a}0x01_02_03_04{color}|{color:#de350b}0x04_03_02_01{color}|{color:#de350b}0x04_03_02_01
(LE){color}|
|BE Unaligned, before
CASSANDRA-17723|{color:#00875a}0x01_02_03_04{color}|{color:#de350b}0x04_03_02_01{color}|{color:#de350b}0x04_03_02_01{color}|{color:#00875a}
{color}{color:#de350b}0x04_03_02_01{color}|
{color:#00875a}0x01_02_03_04{color}| {color:#00875a}0x01_02_03_04 (BE){color}|
|BE
Aligned|{color:#00875a}0x01_02_03_04{color}|{color:#de350b}0x04_03_02_01{color}|{color:#de350b}0x04_03_02_01{color}|{color:#00875a}{color:#de350b}0x04_03_02_01{color}{color}|{color:#00875a}0x01_02_03_04{color}|{color:#00875a}0x01_02_03_04
(BE){color}|
If I have traced the code correctly then it looks like:
* before CASSANDRA-17723 we had native order for offsets (the same conclusion
is here -
[link|https://opensource.docs.scylladb.com/stable/architecture/sstable/sstable3/sstables-3-summary.html#summary-entries])
* after CASSANDRA-17723 (since 5.0) we have LE order for offsets
It means that we broke compatibility for offsets format in 5.0 for offsets in
index summary file format. From another side we have made it machine
independent :).
So, the question for this part - what do we plan to do with it.. - should we
preserve the new behaviour or return back to the old one?..
> MemoryUtil.setInt/getInt and similar use the wrong endianness
> -------------------------------------------------------------
>
> Key: CASSANDRA-20190
> URL: https://issues.apache.org/jira/browse/CASSANDRA-20190
> Project: Apache Cassandra
> Issue Type: Bug
> Components: Local/Other
> Reporter: Branimir Lambov
> Assignee: Dmitry Konstantinov
> Priority: Normal
> Time Spent: 1h
> Remaining Estimate: 0h
>
> `NativeCell`, `NativeClustering` and `NativeDecoratedKey` use the above
> methods from `MemoryUtil` to write and read data from native memory. As far
> as I can see they are meant to write data in big endian. They do not (they
> always correct to little endian).
> Moreover, they disagree with their `ByByte` versions on big-endian machines
> (which is only likely an issue on aligned-access architectures (x86 and arm
> should be fine)).
> The same is true for the methods in `Memory`, used by compression metadata as
> well as index summaries.
> We need to verify that this does not cause any problems, and to change the
> methods to behave as expected and document the behaviour by explicitly using
> `ByteOrder.LITTLE_ENDIAN` for any data that may have been persisted on disk
> with the wrong endianness.
>
> The current MemoryUtil behaviour (before the fix):
> ||Native
> order||MemoryUtil.setX||MemoryUtil.setXByByte||MemoryUtil.getX||MemoryUtil.getXByByte||
> |BE|LE|BE|LE|BE|
> |LE|LE|LE|LE|LE|
> shortly: MemoryUtil.setX/getX is LE, MemoryUtil.setXByByte/getXByByte is
> Native
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
