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

Cassandra Websocket support #1215

Merged
merged 11 commits into from
Jun 30, 2023
Merged

Cassandra Websocket support #1215

merged 11 commits into from
Jun 30, 2023

Conversation

conorbros
Copy link
Member

@conorbros conorbros commented Jun 13, 2023

The objective of this PR is to add support for CQL over Websockets through Shotover. That means having a Websocket listener on Shotover that a compatible driver can send CQL messages to and have Shotover pass these messages to Cassandra through the traditional TCP socket, receive the responses and return it over the Websocket protocol to the client.

At the moment the only instance of this in the wild is an experiment from last year by the Scylla team that is not in production: https://www.scylladb.com/2022/08/11/scylladb-student-projects-cql-over-websocket/.

I am using the tokio-tungstenite library which handles all the websocket internals and lets us just feed the data of a binary Websocket message (which will be raw Cassandra protocol bytes) into our codecs. I also make use of the Sec-WebSocket-Protocol header to specify "cql" as the sub-protocol being used. Some examples of other database technologies using this: https://www.iana.org/assignments/websocket/websocket.xml.

To test this I began work on a simple Cassandra driver: https://github.com/conorbros/cql-ws

The design for configuring the Websocket listener in the topology.yaml is as follows, open to other suggestions as I just went with the simplest one to start with.

---
sources:
  cassandra_prod:
    Cassandra:
      listen_addr: "127.0.0.1:9042"
      websocket: true
chain_config:
  main_chain:
    - CassandraSinkSingle:
        remote_address: "127.0.0.1:9043"
        connect_timeout_ms: 3000
source_to_chain_mapping:
  cassandra_prod: main_chain

Moving forward:

  • make it robust, handle the shutdown logic properly
  • flesh out the test driver more and get more coverage
  • investigate performance and what can be done to improve it

@benbromhead
Copy link
Member

Good start! We can flesh out test coverage and perf in a different PR - Getting the shutdown logic sorted will be good to land this patch

@conorbros
Copy link
Member Author

Alright, got the shutdown flow finished, just a couple error handling todos

@conorbros
Copy link
Member Author

I think now the shutdown flow is handled this can merged as is, I've started modifying the test driver to let us send different types of Websocket messages that will let us test the error handling for that. But as it stands this PR has the MVP functionality.

@conorbros conorbros requested a review from rukai June 23, 2023 05:12
@rukai rukai marked this pull request as ready for review June 25, 2023 22:09
shotover/src/server.rs Outdated Show resolved Hide resolved
@rukai
Copy link
Member

rukai commented Jun 25, 2023

in terms of API, we could make it a field like transport: WebSocket.
that would let us extend to more types of transports in the future, e.g. transport: Quic

shotover/src/server.rs Outdated Show resolved Hide resolved
shotover/src/server.rs Outdated Show resolved Hide resolved
shotover/src/server.rs Outdated Show resolved Hide resolved
shotover/src/server.rs Outdated Show resolved Hide resolved
shotover/src/server.rs Outdated Show resolved Hide resolved
shotover/src/server.rs Outdated Show resolved Hide resolved
@rukai
Copy link
Member

rukai commented Jun 28, 2023

This is pretty cool work!
Just fix the remaining comments and I'll be able to approve.

In order to make use of this in a meaningful way I imagine we'll want to modify an existing driver to get all the driver features one would expect.
Do we have any plans for that?

Also, have you considered wrapping websockets such that they can be exposed as a tokio AsyncRead/AsyncWrite?
I imagine that kind of abstraction would let us share the websockets logic between shotover and the driver and would make for an easier drop in replacement of TCP for shotover or any driver that uses AsyncRead/AsyncWrite.
I imagine it would also come at a performance cost though.

Copy link
Member

@rukai rukai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm!

@conorbros
Copy link
Member Author

conorbros commented Jun 29, 2023

I did look into that a bit before starting but the changes would need a fair bit of work to tungstenite so I decided to just move ahead with a working solution in Shotover first.

snapview/tokio-tungstenite#32

Copy link
Member

@benbromhead benbromhead left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!!

@conorbros conorbros enabled auto-merge (squash) June 30, 2023 01:14
@conorbros conorbros merged commit 13cb5f8 into shotover:main Jun 30, 2023
10 checks passed
@conorbros conorbros deleted the websocket branch June 30, 2023 01:33
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.

None yet

3 participants