[
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)
