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

CassandraSinkCluster replace recv with try_recv #1553

Merged
merged 4 commits into from
Apr 18, 2024

Conversation

rukai
Copy link
Member

@rukai rukai commented Mar 26, 2024

This PR swaps CassandraSinkCluster from recv to try_recv for the performance improvements it brings:

  • use try_recv instead of awaiting on recv. - this gives us the benchmark improvement shown below
  • when the MessageRewriter invents a new request out of thin air it needs to assign it a unique stream_id that is unique for its destination node. This is a difficult problem. The simplest approach is just to return a BatchMode, allowing the rewriter to dictate whether the current batch of requests is allowed to pipeline the requests for efficiency or whether the current batch must be isolated allowing the rewriter to generate stream_id's as long as they are unique within the current request batch.

Looks like a large win for connection_count=1
image

And when comparing against benches without shotover in the loop we can see that shotover is now very close!
connection_count = 1 has shotover lagging behind by about 20-30%
image

connection_count = 10 has slightly higher throughput for shotover (in reality thats probably just noise)
image

connection_count = 100 is also pretty much identical.
image

@rukai rukai changed the title CassandraSinkCluster port to Connection CassandraSinkCluster port to SinkConnection Mar 26, 2024
@rukai rukai force-pushed the cassandra_sink_cluster_port_to_connection branch 3 times, most recently from 54bdacc to b4e1e0c Compare April 2, 2024 23:05
@rukai rukai force-pushed the cassandra_sink_cluster_port_to_connection branch 3 times, most recently from a5e1525 to 0c3caab Compare April 3, 2024 05:15
@rukai rukai force-pushed the cassandra_sink_cluster_port_to_connection branch 4 times, most recently from 9c4fe8a to ed4e668 Compare April 4, 2024 02:00
@rukai rukai force-pushed the cassandra_sink_cluster_port_to_connection branch 4 times, most recently from 5f27e17 to 2217c08 Compare April 5, 2024 00:17
@rukai rukai changed the title CassandraSinkCluster port to SinkConnection CassandraSinkCluster replace recv with try_recv Apr 5, 2024
@rukai rukai force-pushed the cassandra_sink_cluster_port_to_connection branch 4 times, most recently from 6670751 to 28f1457 Compare April 5, 2024 05:08
@rukai rukai force-pushed the cassandra_sink_cluster_port_to_connection branch 3 times, most recently from 2284eea to da19bea Compare April 16, 2024 23:18
@rukai rukai force-pushed the cassandra_sink_cluster_port_to_connection branch from da19bea to a609834 Compare April 17, 2024 21:55
@rukai rukai marked this pull request as ready for review April 17, 2024 23:22
@rukai rukai requested a review from conorbros April 18, 2024 01:49
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!

@rukai rukai enabled auto-merge (squash) April 18, 2024 21:09
@rukai rukai merged commit 9e3e2aa into shotover:main Apr 18, 2024
40 checks passed
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