[ 
https://issues.apache.org/jira/browse/CASSANDRA-15299?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17230079#comment-17230079
 ] 

Sam Tunnicliffe commented on CASSANDRA-15299:
---------------------------------------------

Something we'll need to do in order to merge this is to update the python 
dtests to use a driver which supports the new protocol. Initial support has 
been committed, but not yet released, so I propose updating dtests to pull a 
specific commit from the driver repo. I thought it should be possible to do 
this just by updating {{requirements.txt}} in the dtest repo, but unfortunately 
not. 

While this does cause the container to pull an updated version, but it only 
wipes the source files, not the compiled {{.c}} and {{.so}} objects from the 
installation and there are incompatibilities which cause {{ImportError}} when 
trying to run the dtests. I spoke to [~eduard.tudenhoefner] about this and 
published new docker images with the updated driver installed 
([e1fc528a0|https://github.com/datastax/python-driver/commit/e1fc528a0deec67f725f066204e8c1ea8b26bbde]).

Docker images:
* 
[beobal/cassandra-testing-ubuntu1910-java11:20201111|https://hub.docker.com/layers/beobal/cassandra-testing-ubuntu1910-java11/20201111/images/sha256-6c4bdcb82c6b25ec30495f3a49188206fbad99d5c8083217d576a8a84a610bf4?context=repo]
* 
[beobal/cassandra-testing-ubuntu1910-java11-w-dependencies:20201111|https://hub.docker.com/layers/beobal/cassandra-testing-ubuntu1910-java11-w-dependencies/20201111/images/sha256-beda323a6fbba94dc6c2953f5cd5f95d71dfc3dafbd605470ecb0d4282fb0fa8?context=repo]

This works as expected, but there is one failing dtest, which looks to be 
related so I'd like to open a follow up JIRA to fix that. One thing that's a 
bit concerning is that the {{cassandra-test}} branch of the driver, which is 
what dtests are currently using, is currently 693 commits behind the master 
branch. But, if there are no objections to updating in this way, I'll open PRs 
to {{cassandra-builds}} and {{cassandra-dtest}} before going any further here.

cc: [~mck] [~aboudreault] [~brandon.williams]


> CASSANDRA-13304 follow-up: improve checksumming and compression in protocol 
> v5-beta
> -----------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-15299
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-15299
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Messaging/Client
>            Reporter: Aleksey Yeschenko
>            Assignee: Sam Tunnicliffe
>            Priority: Normal
>              Labels: protocolv5
>             Fix For: 4.0-alpha
>
>         Attachments: Process CQL Frame.png, V5 Flow Chart.png
>
>
> CASSANDRA-13304 made an important improvement to our native protocol: it 
> introduced checksumming/CRC32 to request and response bodies. It’s an 
> important step forward, but it doesn’t cover the entire stream. In 
> particular, the message header is not covered by a checksum or a crc, which 
> poses a correctness issue if, for example, {{streamId}} gets corrupted.
> Additionally, we aren’t quite using CRC32 correctly, in two ways:
> 1. We are calculating the CRC32 of the *decompressed* value instead of 
> computing the CRC32 on the bytes written on the wire - losing the properties 
> of the CRC32. In some cases, due to this sequencing, attempting to decompress 
> a corrupt stream can cause a segfault by LZ4.
> 2. When using CRC32, the CRC32 value is written in the incorrect byte order, 
> also losing some of the protections.
> See https://users.ece.cmu.edu/~koopman/pubs/KoopmanCRCWebinar9May2012.pdf for 
> explanation for the two points above.
> Separately, there are some long-standing issues with the protocol - since 
> *way* before CASSANDRA-13304. Importantly, both checksumming and compression 
> operate on individual message bodies rather than frames of multiple complete 
> messages. In reality, this has several important additional downsides. To 
> name a couple:
> # For compression, we are getting poor compression ratios for smaller 
> messages - when operating on tiny sequences of bytes. In reality, for most 
> small requests and responses we are discarding the compressed value as it’d 
> be smaller than the uncompressed one - incurring both redundant allocations 
> and compressions.
> # For checksumming and CRC32 we pay a high overhead price for small messages. 
> 4 bytes extra is *a lot* for an empty write response, for example.
> To address the correctness issue of {{streamId}} not being covered by the 
> checksum/CRC32 and the inefficiency in compression and checksumming/CRC32, we 
> should switch to a framing protocol with multiple messages in a single frame.
> I suggest we reuse the framing protocol recently implemented for internode 
> messaging in CASSANDRA-15066 to the extent that its logic can be borrowed, 
> and that we do it before native protocol v5 graduates from beta. See 
> https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/net/FrameDecoderCrc.java
>  and 
> https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/net/FrameDecoderLZ4.java.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

Reply via email to