[
https://issues.apache.org/jira/browse/CASSANDRA-13304?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16599314#comment-16599314
]
Jordan West edited comment on CASSANDRA-13304 at 9/1/18 3:21 PM:
-----------------------------------------------------------------
[~beobal] I'm still going over the changes but I wanted to post this bug now
before I finish. The issue is that the checksum over the lengths is only
calculated over the least-significant byte of the compressed and uncompressed
lengths. This means that if we introduce corruption in the three most
significant bytes we won't catch it, which leads to a host of different bugs
(index out of bounds exception , lz4 decompression issues, etc). I pushed a
patch
[here|https://github.com/jrwest/cassandra/commit/e57a2508c26f05efb826a7f4342964fa6d6691bd]
with a new test that catches the issue. I left in the fixed seed for now so
its easy for you to run and see the failing example in a debugger (its the
second example that fails). I've pasted a stack trace of the failure with that
seed below. The example generated has a single byte input and introduces
corruption into the 4th byte in the stream (the second most significant byte of
the first length in the stream). This leads to a case where the checksums match
but when we go to read the data we read past the total length of the buffer.
A couple other comments while I'm here. All minor:
* [~djoshi3] and I were reviewing the bug above before I posted and we were
thinking it would be nice to refactor
{{ChecksummingTransformer#transformInbound/transformOutbound}}. They are a bit
large/unwieldy right now. We can open a JIRA to address this later if you
prefer.
* Re: the {{roundTripZeroLength}} property. This is mostly covered by the
property I already added although this makes it more likely to generate a few
cases. If you want to keep it I would recommend setting {{withExamples}} and
using something small like 10 or 20 examples (since the state space is small).
* The {{System.out.println}} I added in {{roundTripSafetyProperty}} should be
removed
* In the test failure below we also hit the buffer resize condition,
{{ret.writableBytes() < (CHUNK_HEADER_OVERHEAD + toWrite)}} this is because we
don’t account for {{CHUNK_HEADER_OVERHEAD}} when allocating the buffer and by
the time we check, we have already written some parts of the chunk header.
While this test is an edge case (single byte input) it does lead to us
allocating a buffer 3.6x larger than we need.
Stack Trace:
{code:java}
java.lang.AssertionError: Property falsified after 2 example(s)
Smallest found falsifying value(s) :- \{(c,3), 0, null, Adler32}
Cause was :-
java.lang.IndexOutOfBoundsException: readerIndex(10) + length(16711681)
exceeds writerIndex(15): UnpooledHeapByteBuf(ridx: 10, widx: 15, cap: 54/54)
at
io.netty.buffer.AbstractByteBuf.checkReadableBytes0(AbstractByteBuf.java:1401)
at
io.netty.buffer.AbstractByteBuf.checkReadableBytes(AbstractByteBuf.java:1388)
at io.netty.buffer.AbstractByteBuf.readBytes(AbstractByteBuf.java:870)
at
org.apache.cassandra.transport.frame.checksum.ChecksummingTransformer.transformInbound(ChecksummingTransformer.java:289)
at
org.apache.cassandra.transport.frame.checksum.ChecksummingTransformerTest.roundTripWithCorruption(ChecksummingTransformerTest.java:106)
at
org.quicktheories.dsl.TheoryBuilder4.lambda$checkAssert$9(TheoryBuilder4.java:163)
at org.quicktheories.dsl.TheoryBuilder4.lambda$check$8(TheoryBuilder4.java:151)
at org.quicktheories.impl.Property.tryFalsification(Property.java:23)
at org.quicktheories.impl.Core.shrink(Core.java:111)
at org.quicktheories.impl.Core.run(Core.java:39)
at org.quicktheories.impl.TheoryRunner.check(TheoryRunner.java:35)
at org.quicktheories.dsl.TheoryBuilder4.check(TheoryBuilder4.java:150)
at org.quicktheories.dsl.TheoryBuilder4.checkAssert(TheoryBuilder4.java:162)
at
org.apache.cassandra.transport.frame.checksum.ChecksummingTransformerTest.corruptionCausesFailure(ChecksummingTransformerTest.java:87)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at
org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
at
org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at
org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
at
org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
at
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
at
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
at
org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
at
com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
at
com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:47)
at
com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:242)
at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)
Other found falsifying value(s) :-
{(c,3), 0, null, CRC32} \{(c,3), 1, null, CRC32} \{(c,3), 9, null, CRC32}
\{(c,3), 11, null, CRC32} \{(c,3), 36, null, CRC32} \{(c,3), 50, null, CRC32}
\{(c,3), 74, null, CRC32} \{(c,3), 99, null, CRC32}
Seed was 179207634899674
at org.quicktheories.core.ExceptionReporter.falsify(ExceptionReporter.java:43)
at
org.quicktheories.core.ExceptionReporter.falisification(ExceptionReporter.java:37)
at
org.quicktheories.impl.TheoryRunner.reportFalsification(TheoryRunner.java:48)
at org.quicktheories.impl.TheoryRunner.check(TheoryRunner.java:37)
at org.quicktheories.dsl.TheoryBuilder4.check(TheoryBuilder4.java:150)
at org.quicktheories.dsl.TheoryBuilder4.checkAssert(TheoryBuilder4.java:162)
at
org.apache.cassandra.transport.frame.checksum.ChecksummingTransformerTest.corruptionCausesFailure(ChecksummingTransformerTest.java:87)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at
org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
at
org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at
org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
at
org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
at
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
at
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
at
org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
at
com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
at
com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:47)
at
com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:242)
at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)
{code}
was (Author: jrwest):
[~beobal] I'm still going over the changes but I wanted to post this bug now
before I finish. The issue is that the checksum over the lengths is only
calculated over the least-significant byte of the compressed and uncompressed
lengths. This means that if we introduce corruption in the three most
significant bytes we won't catch it, which leads to a host of different bugs
(index out of bounds exception , lz4 decompression issues, etc). I pushed a
patch
[here|https://github.com/jrwest/cassandra/commit/e57a2508c26f05efb826a7f4342964fa6d6691bd]
with a new test that catches the issue. I left in the fixed seed for now so
its easy for you to run and see the failing example in a debugger (its the
second example that fails). I've pasted a stack trace of the failure with that
seed below. The example generated has a single byte input and introduces
corruption into the 4th byte in the stream (the second most significant byte of
the first length in the stream). This leads to a case where the checksums match
but when we go to read the data we read past the total length of the buffer.
A couple other comments while I'm here. All minor:
* [~djoshi3] and I were reviewing the bug above before I posted and we were
thinking it would be nice to refactor
{{ChecksummingTransformer#transformInbound/transformOutbound}}. They are a bit
large/unwieldy right now. We can open a JIRA to address this later if you
prefer.
* Re: the {{roundTripZeroLength}} property. This is mostly covered by the
property I already added although this makes it more likely to generate a few
cases. If you want to keep it I would recommend setting {{withExamples}} and
using something small like 10 or 20 examples (since the state space is small).
* The {{System.out.println}} I added in {{roundTripSafetyProperty}} should be
removed
* In the test failure below we also hit the buffer resize condition,
{{ret.writableBytes() < (CHUNK_HEADER_OVERHEAD + toWrite)}} this is because we
don’t account for {{CHUNK_HEADER_OVERHEAD}} when allocating the buffer and by
the time we check, we have already written some parts of the chunk header.
While this test is an edge case (single byte input) it does lead to us
allocating a buffer 3.8x larger than we need.
Stack Trace:
{code:java}
java.lang.AssertionError: Property falsified after 2 example(s)
Smallest found falsifying value(s) :- \{(c,3), 0, null, Adler32}
Cause was :-
java.lang.IndexOutOfBoundsException: readerIndex(10) + length(16711681)
exceeds writerIndex(15): UnpooledHeapByteBuf(ridx: 10, widx: 15, cap: 54/54)
at
io.netty.buffer.AbstractByteBuf.checkReadableBytes0(AbstractByteBuf.java:1401)
at
io.netty.buffer.AbstractByteBuf.checkReadableBytes(AbstractByteBuf.java:1388)
at io.netty.buffer.AbstractByteBuf.readBytes(AbstractByteBuf.java:870)
at
org.apache.cassandra.transport.frame.checksum.ChecksummingTransformer.transformInbound(ChecksummingTransformer.java:289)
at
org.apache.cassandra.transport.frame.checksum.ChecksummingTransformerTest.roundTripWithCorruption(ChecksummingTransformerTest.java:106)
at
org.quicktheories.dsl.TheoryBuilder4.lambda$checkAssert$9(TheoryBuilder4.java:163)
at org.quicktheories.dsl.TheoryBuilder4.lambda$check$8(TheoryBuilder4.java:151)
at org.quicktheories.impl.Property.tryFalsification(Property.java:23)
at org.quicktheories.impl.Core.shrink(Core.java:111)
at org.quicktheories.impl.Core.run(Core.java:39)
at org.quicktheories.impl.TheoryRunner.check(TheoryRunner.java:35)
at org.quicktheories.dsl.TheoryBuilder4.check(TheoryBuilder4.java:150)
at org.quicktheories.dsl.TheoryBuilder4.checkAssert(TheoryBuilder4.java:162)
at
org.apache.cassandra.transport.frame.checksum.ChecksummingTransformerTest.corruptionCausesFailure(ChecksummingTransformerTest.java:87)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at
org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
at
org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at
org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
at
org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
at
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
at
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
at
org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
at
com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
at
com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:47)
at
com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:242)
at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)
Other found falsifying value(s) :-
{(c,3), 0, null, CRC32} \{(c,3), 1, null, CRC32} \{(c,3), 9, null, CRC32}
\{(c,3), 11, null, CRC32} \{(c,3), 36, null, CRC32} \{(c,3), 50, null, CRC32}
\{(c,3), 74, null, CRC32} \{(c,3), 99, null, CRC32}
Seed was 179207634899674
at org.quicktheories.core.ExceptionReporter.falsify(ExceptionReporter.java:43)
at
org.quicktheories.core.ExceptionReporter.falisification(ExceptionReporter.java:37)
at
org.quicktheories.impl.TheoryRunner.reportFalsification(TheoryRunner.java:48)
at org.quicktheories.impl.TheoryRunner.check(TheoryRunner.java:37)
at org.quicktheories.dsl.TheoryBuilder4.check(TheoryBuilder4.java:150)
at org.quicktheories.dsl.TheoryBuilder4.checkAssert(TheoryBuilder4.java:162)
at
org.apache.cassandra.transport.frame.checksum.ChecksummingTransformerTest.corruptionCausesFailure(ChecksummingTransformerTest.java:87)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at
org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
at
org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at
org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
at
org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
at
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
at
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
at
org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
at
com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
at
com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:47)
at
com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:242)
at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)
{code}
> Add checksumming to the native protocol
> ---------------------------------------
>
> Key: CASSANDRA-13304
> URL: https://issues.apache.org/jira/browse/CASSANDRA-13304
> Project: Cassandra
> Issue Type: Improvement
> Components: Core
> Reporter: Michael Kjellman
> Assignee: Sam Tunnicliffe
> Priority: Blocker
> Labels: client-impacting
> Fix For: 4.x
>
> Attachments: 13304_v1.diff, boxplot-read-throughput.png,
> boxplot-write-throughput.png
>
>
> The native binary transport implementation doesn't include checksums. This
> makes it highly susceptible to silently inserting corrupted data either due
> to hardware issues causing bit flips on the sender/client side, C*/receiver
> side, or network in between.
> Attaching an implementation that makes checksum'ing mandatory (assuming both
> client and server know about a protocol version that supports checksums) --
> and also adds checksumming to clients that request compression.
> The serialized format looks something like this:
> {noformat}
> * 1 1 1 1 1 1 1 1 1 1 2 2 2 2 2 2 2 2 2 2 3 3
> * 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> * | Number of Compressed Chunks | Compressed Length (e1) /
> * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> * / Compressed Length cont. (e1) | Uncompressed Length (e1) /
> * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> * | Uncompressed Length cont. (e1)| CRC32 Checksum of Lengths (e1)|
> * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> * | Checksum of Lengths cont. (e1)| Compressed Bytes (e1) +//
> * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> * | CRC32 Checksum (e1) ||
> * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> * | Compressed Length (e2) |
> * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> * | Uncompressed Length (e2) |
> * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> * | CRC32 Checksum of Lengths (e2) |
> * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> * | Compressed Bytes (e2) +//
> * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> * | CRC32 Checksum (e2) ||
> * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> * | Compressed Length (en) |
> * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> * | Uncompressed Length (en) |
> * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> * | CRC32 Checksum of Lengths (en) |
> * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> * | Compressed Bytes (en) +//
> * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> * | CRC32 Checksum (en) ||
> * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> {noformat}
> The first pass here adds checksums only to the actual contents of the frame
> body itself (and doesn't actually checksum lengths and headers). While it
> would be great to fully add checksuming across the entire protocol, the
> proposed implementation will ensure we at least catch corrupted data and
> likely protect ourselves pretty well anyways.
> I didn't go to the trouble of implementing a Snappy Checksum'ed Compressor
> implementation as it's been deprecated for a while -- is really slow and
> crappy compared to LZ4 -- and we should do everything in our power to make
> sure no one in the community is still using it. I left it in (for obvious
> backwards compatibility aspects) old for clients that don't know about the
> new protocol.
> The current protocol has a 256MB (max) frame body -- where the serialized
> contents are simply written in to the frame body.
> If the client sends a compression option in the startup, we will install a
> FrameCompressor inline. Unfortunately, we went with a decision to treat the
> frame body separately from the header bits etc in a given message. So,
> instead we put a compressor implementation in the options and then if it's
> not null, we push the serialized bytes for the frame body *only* thru the
> given FrameCompressor implementation. The existing implementations simply
> provide all the bytes for the frame body in one go to the compressor
> implementation and then serialize it with the length of the compressed bytes
> up front.
> Unfortunately, this won't work for checksum'ing for obvious reasons as we
> can't naively just checksum the entire (potentially) 256MB frame body and
> slap it at the end... so,
> The best place to start with the changes is in {{ChecksumedCompressor}}. I
> implemented one single place to perform the checksuming (and to support
> checksuming) the actual required chunking logic. Implementations of
> ChecksumedCompressor only implement the actual calls to the given compression
> algorithm for the provided bytes.
> Although the interface takes a {{Checksum}}, right now the attached patch
> uses CRC32 everywhere. As of right now, given JDK8+ has support for doing the
> calculation with the Intel instruction set, CRC32 is about as fast as we can
> get right now.
> I went with a 32kb "default" for the chunk size -- meaning we will chunk the
> entire frame body into 32kb chunks, compress each one of those chunks, and
> checksum the chunk. Upon discussing with a bunch of people and researching
> how checksums actually work and how much data they will protect etc -- if we
> use 32kb chunks with CRC32 we can catch up to 32 bits flipped in a row (but
> more importantly catch the more likely corruption where a single bit is
> flipped) with pretty high certainty. 64kb seems to introduce too much of a
> probability of missing corruption.
> The maximum block size LZ4 operates on is a 64kb chunk -- so this combined
> with the need to make sure the CRC32 checksums are actually going to catch
> stuff -- chunking at 32kb seemed like a good reasonable value to use when
> weighing both checksums and compression (to ensure we don't kill our
> compression ratio etc).
> I'm not including client changes here -- I asked around and I'm not really
> sure what the policy there is -- do we update the python driver? java driver?
> how has the timing of this stuff been handled in the past?
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
