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

Fix kafka benches to actually bench shotover #1372

Merged
merged 3 commits into from
Nov 23, 2023

Conversation

rukai
Copy link
Member

@rukai rukai commented Nov 21, 2023

Before this PR our shotover benches were actually benching directly with kafka completely bypassing shotover!

The KafkaSinkSingle transform is designed to run only on the same machine as the kafka broker.
However the bench currently sets up kafka and shotover on different instances.

One would expect this to result in an error somewhere and the bench to fail.
But what actually happens is that shotover fails to rewrite the metadata query containing the kafka nodes in the cluster and as a result once the bencher receives the metadata query back from shotover it proceeds to send all the benchmark queries directly to kafka.

In more detail:
Shotover will overwrite the port of any brokers returned by the metadata requests.
When shotover and kafka are on different instances they both have ports of 9092.
So we are just overwriting 9092 with 9092, an unintentional noop, and so the client just connects directly to kafka without any error.

This silent failure is really bad and I've raised the following to prevent similar issues from occurring again:

This PR fixes the benchmark by moving shotover and kafka onto the same machine for the kafka benchmarks.

kafka windsock results with this PR:
image

@rukai rukai force-pushed the fix_kafka_benches branch 3 times, most recently from 64e52f6 to c4b9c4e Compare November 21, 2023 02:57
@rukai rukai marked this pull request as draft November 21, 2023 02:59
@rukai rukai force-pushed the fix_kafka_benches branch from c4b9c4e to 3bb3dc8 Compare November 21, 2023 03:11
@rukai rukai force-pushed the fix_kafka_benches branch from 3bb3dc8 to 644ea72 Compare November 21, 2023 03:14
@rukai rukai marked this pull request as ready for review November 21, 2023 05:45
@rukai rukai requested a review from conorbros November 21, 2023 05:45
@rukai rukai enabled auto-merge (squash) November 23, 2023 21:53
@rukai rukai merged commit 9e58a0a into shotover:main Nov 23, 2023
39 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.

3 participants