cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Stefania (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-12838) Extend native protocol flags and add supported versions to the SUPPORTED response
Date Fri, 28 Oct 2016 06:45:58 GMT

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

Stefania commented on CASSANDRA-12838:
--------------------------------------

Thank you for the review. I've replaced the {{compareTo}} calls with more descriptive methods,
and I've added a couple of unit tests (the utility method for making a page state is a cut
and paste). It's all in one [commit|https://github.com/stef1927/cassandra/commit/0fe9a51d41eb9366970705831f6453877a9ea0c7],
the 3.X patch applies to trunk without conflicts.

In terms of renaming {{ProtocolVersion}}, I wouldn't mind renaming it if there was an equivalent
name but I couldn't find any. I don't think we should add the protocol type because the class
lives in the native protocol package, so I disregarded something like {{NativeProtocolVersion}}.
I consider the fact that we need to use the java driver in our unit tests a limitation, so
I don't want to pick a not-so-good name because of it, but if someone has a name that it is
truly equivalent to {{ProtocolVersion}}, then I don't mind renaming it.

||3.X||trunk||
|[patch|https://github.com/stef1927/cassandra/tree/12838-3.X]|[patch|https://github.com/stef1927/cassandra/tree/12838]|
|[testall|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-12838-3.X-testall/]|[testall|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-12838-testall/]|
|[dtest|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-12838-3.X-dtest/]|[dtest|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-12838-dtest/]|

CI is still pending, there were unrelated problems with the dtests this morning, I've pinged
the TEs and relaunched. 

The pull request for the driver is [here|https://github.com/datastax/python-driver/pull/674].
[~aholmber] can we arrange a time slot next week, morning your time, where we can commit this
ticket and simultaneously merge the pull request into cassandra-test, so that we minimize
dtest failures for people, there are about 160 dtests using version 5, and they will fail
without the driver changes.

> Extend native protocol flags and add supported versions to the SUPPORTED response
> ---------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-12838
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-12838
>             Project: Cassandra
>          Issue Type: Sub-task
>          Components: CQL
>            Reporter: Stefania
>            Assignee: Stefania
>             Fix For: 3.x
>
>
> We already use 7 bits for the flags of the QUERY message, and since they are encoded
with a fixed size byte, we may be forced to change the structure of the message soon, and
I'd like to do this in version 5 but without wasting bytes on the wire. Therefore, I propose
to convert fixed flag's bytes to unsigned vints, as defined in CASSANDRA-9499. The only exception
would be the flags in the frame, which should stay as fixed size.
> Up to 7 bits, vints are encoded the same as bytes are, so no immediate change would be
required in the drivers, although they should plan to support vint flags if supporting version
5. Moving forward, when a new flag is required for the QUERY message, and eventually when
other flags reach 8 bits in other messages too, the flag's bitmaps would be automatically
encoded with a size that is big enough to accommodate all flags, but no bigger than required.
We can currently support up to 8 bytes with unsigned vints.
> The downside is that drivers need to implement unsigned vint encoding for version 5,
but this is already required by CASSANDRA-11873, and will most likely be required by CASSANDRA-11622
as well.
> I would also like to add the list of versions to the SUPPORTED message, in order to simplify
the handshake for drivers that prefer to send an OPTION message, rather than rely on receiving
an error for an unsupported version in the STARTUP message. Said error should also contain
the full list of supported versions, not just the min and max, for clarity, and because the
latest version is now a beta version.
> Finally, we currently store versions as integer constants in {{Server.java}}, and we
still have a fair bit of hard-coded numbers in the code, especially in tests. I plan to clean
this up by introducing a {{ProtocolVersion}} enum.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message