Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feature] protocol v3 #57

Closed
wants to merge 6 commits into from

Conversation

thibaultcha
Copy link
Contributor

Implements #22. Discussions about it already started at thibaultcha#5 so just copy pasting what I've written there so far:

About this PR

Drop v2 support for v3 only with:

  • update constants' version_codes
  • stream id is now 2 bytes long (a [short] value), so the header is now 1 byte longer (9 bytes total).
  • BATCH messages now have <flags> (like QUERY and EXECUTE) and a corresponding optional <serial_consistency> parameters (see Section 4.1.7).
  • User Defined Types and tuple types have been added to ResultSet metadata (see 4.2.5.2) and a new section on the serialization format of UDT and tuple values has been added to the documentation (Section 7).
    • UDT
    • Tuple
  • The serialization format for collection has changed (both the collection size and the length of each argument is now 4 bytes long). See Section 6.
  • The format of "Schema_change" results (Section 4.2.5.5) and "SCHEMA_CHANGE" events (Section 4.2.6) has been modified, and now includes changes related to user types.

  • Some refactor (moving batch representation to protocol.lua for example, which makes more sense since the query representation is already there.
  • Add support for the serial_consistency flag
  • Expose errors, as they are needed externally, like consistency and batch_types
  • Add some test cases
  • Some code linting

About dropping v2

In regards of the v3: I wanted to support both v2 and v3 (and we still can) but I was struggling with the current structure and I wasn't sure how. I just came up with an idea: we could have a Protocol class as an attribute of a session, and a subclass for v2 and v3 implementations, overriding what needs to be.

Dropping v2 means losing support for cassandra 2.0:

Cassandra Version Protocol Versions
1.2 1
2.0 1, 2
2.1 1, 2, 3

So let me know how you feel about it and my suggested solution because I'm pretty sure we still want to support 2.0.

About the file structure

In any case I think a refactor of the structure should be done to have a cleaner code base. I would like to have such a structure (I'm just making this up, haven't given it much thought yet):

cassandra
├── cassandra.lua
├── utils.lua -- we kinda need this for methods such as `shuffle`, `split`, bit operations, big endian etc...
├── constants.lua -- those could have more than just the values. For ex it could specify how the `paging_size` flag should be encoded (as an `[int]`), and thus reduce the quantity of code needed everywhere it's used (just an example)
├── marshallers -- only deal with converting lua types to cql types
│   ├── marshall.lua
│   └── unmarshall.lua
└── protocol -- those could be classes for v2, v3...
    ├── reader.lua
    └── writer.lua

We could breakup the project into smaller, more maintainable and documentable files.

Types could also be classes. That way it gets easy and elegant to serialize/deserialize them, do bit operations on them etc. A simple example:

local flags_repr = 0
flags_repr = setbit(flags_repr, constants.query_flags.VALUES)

would become

local flags = Byte.new() -- a [byte]
flags:setbit(constants.query_flags.VALUES)

- move query/execute/batch representations to separate functions in
  protocol.lua and add some documentation as to what they are supposed
  to represent
- notify of some eventual errors in specs
- necessary changes to switch to v3
  - 9 bytes long headers
  - 2 bytes ([short]) stream ids
  - <flag> for BATCH statements for <serial_consistency> and <timestamp> (none is yest supported)
  - new serialization format for collections
  - new serialization for SCHEMA_CHANGE results
- add tests for SCHEMA_CHANGE results
- some linting (shadowed variables and redefinitions)
@thibaultcha thibaultcha force-pushed the feature/protocol-v3 branch from ce16eeb to 3d1ad98 Compare May 12, 2015 00:55
@thibaultcha thibaultcha force-pushed the feature/protocol-v3 branch from 3d1ad98 to 195e02c Compare May 12, 2015 01:00
@thibaultcha
Copy link
Contributor Author

Damn, travis-ci installs Cassandra 2.0.9, which apparently does not support the v3. This is how we install Cassandra for Kong, so let me know what you want to do about that.

@thibaultcha
Copy link
Contributor Author

Updated the PR with infos about cassandra versions/protocol versions infos.

@subnetmarco
Copy link
Contributor

+1

@thibaultcha
Copy link
Contributor Author

Let me know how you feel about dropping v2 (I'm pretty sure we don't want to do that) and my suggestion to support both protocols @jbochi 😄

@sonicaghi
Copy link

+1

@alubbe
Copy link

alubbe commented Dec 12, 2015

just a quick check - what's the status of this PR?

@thibaultcha
Copy link
Contributor Author

The project was forked to https://github.com/thibaultCha/lua-cassandra, with added features and bugfixes, and has recently been entirely re-written with many new features thibaultcha/lua-cassandra#15.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants