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

warn when shotover doesnt know how to route a request #1779

Merged
merged 2 commits into from
Oct 21, 2024

Conversation

rukai
Copy link
Member

@rukai rukai commented Oct 18, 2024

1

When shotover encounters a request type it does not know how to route it will route it to a random node.
This is a reasonable guess and in the worst case the broker will respond with a routing error which we return to the client.

However this does make it hard to track down issues caused by this random routing which can occur when

  • an existing message type hasnt had routing implemented yet
  • new message types are added to kafka

To make it easier to diagnose these issues this PR does two things:

  • Adds explicit routing handling for all message types we currently cover in our integration tests
  • Adds a warning if a request is unhandled and therefore routed to a random node.

2

Unfortunately things are never that simple.
The OffsetForLeaderEpoch request is being sent to shotover as part of our test suite and requires routing to partition leader but has no routing implemented yet.
It was failing due to incorrect routing but then eventually succeeds when the random routing arrives at the partition leader node.
So I decided to implement routing for this node rather than just adding explicit random node routing and leaving a TODO.

OffsetForLeaderEpoch requires request splitting for the same reasons that fetch and produce requests need to be split.
So this PR also implements routing, splitting and joining of OffsetForLeaderEpoch requests.

Copy link

codspeed-hq bot commented Oct 18, 2024

CodSpeed Performance Report

Merging #1779 will not alter performance

Comparing rukai:warn_on_unrouted_request (bb266ea) with main (26550b8)

Summary

✅ 38 untouched benchmarks

@rukai rukai force-pushed the warn_on_unrouted_request branch 2 times, most recently from af3402a to 2a3fc04 Compare October 18, 2024 02:02
@rukai rukai marked this pull request as ready for review October 18, 2024 02:49
@rukai rukai merged commit f327c96 into shotover:main Oct 21, 2024
41 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