Routing SPI: Default forwarding to the assigned upstream broker

[hrishabhg]

Revises the Router plugin API (proposal 070) so a request on a broker-bound connection forwards to the broker that endpoint represents by default, and a router declares only the API keys it must intercept. Replaces staticRoutes().

Addresses the staticRoutes() gaps in kroxylicious#4177: the lost forward-to-assigned-broker default, the VC-level vs per-connection scope mismatch, and the unpopulatable key->route map.

SPI: intercepts(apiKey, ctx) gate (default = bootstrap connections only), shouldDecodeRequest(apiKey, ctx) (default false), and onRequest(apiKey, RequestFrame, ctx). Response decode is lazy via ResponseFrame.body(); no shouldDecodeResponse and no onResponse callback. Route binding is established at connection-creation time, which keeps bound-connection forwarding unambiguous and permits multiple routes to one cluster.

Relates-to: kroxylicious/kroxylicious#4177
Assisted-by: Claude Opus 4.8 noreply@anthropic.com

[SamBarker]

Thanks for writing this up — and for pushing on staticRoutes() in #4177. The problems you identified are real and the core of this proposal is the right answer.

I want to share a direction I've been thinking about as context. I haven't fully worked it out, so this isn't a counter-proposal — it's a lens I'd like us to evaluate the proposal through.

The default destination for every request is the broker the connection already represents. That's the foundation I think we should be building on. I've been calling this "identity routing" in my head — every request has a default destination (the assigned broker), and routing is about when and why you deviate from that. Whether the identity router ends up as a concrete class, a runtime behaviour, or something else, I don't know yet. But the principle feels right: the assigned broker is always the answer unless something says otherwise.

Under that lens, intercepts() is well-aligned. The default gate (ctx.virtualNode().isEmpty()) says exactly the right thing: on a bound connection there's a default destination, so don't bother the router. On bootstrap there isn't, so the router must participate. The three shapes table and the four example routers all work cleanly against this. This is the strongest part of the proposal.

Where I'd like to push is whether the decode depth mechanics need to be part of this proposal or whether they're a follow-on. Thinking through each one against the same lens:

• shouldDecodeRequest — a router that intercepts but only needs the header (client-id, subject) shouldn't force a full decode. That's a real need. But it's a need of the interception path, not of the default destination. Rob made the point in #4177 that deserialization is a runtime decision, not a router decision. Could this be separated?

• RequestFrame/ResponseFrame and lazy response decoding — these widen 070's sendRequest return type and introduce new abstractions. They're well thought through, but they're about optimising what happens after a router intercepts. The default destination doesn't need frames — the request just goes where it was going. Could these follow once intercepts() is in place?

I'm not saying these are wrong. I'm asking whether they're separable from intercepts(), because if they are, this proposal gets smaller and lands faster — and the decode depth work gets a proposal that can focus on it properly.

Is there a dependency I'm missing that ties the decode depth to the routing default?

I've run out of time for a deeper look right now but will come back to this properly.

[hrishabhg]

@SamBarker

Is there a dependency I'm missing that ties the decode depth to the routing default?

No dependency — I've slimmed the proposal to just the gate and moved decode depth to follow-on work.

[SamBarker]

One more thought on the decode depth side, specifically lazy response decoding.

Given the work in kroxylicious#4257, shouldDecodeResponse in DelegatingDecodePredicate is already symmetric with shouldDecodeRequest — and the router already drives response decoding declaratively for node ID translation. Metadata, FindCoordinator, DescribeTopicPartitions, Fetch (preferredReadReplica, currentLeader), and the other responses that carry node references are decoded regardless of what the router asks for, because virtual node ID mapping requires it.

Fetch is the primary response by volume and size, and it's already decoded. So lazy ResponseFrame.body() would only save decode cost for a response that a router intercepts but doesn't need to read, AND that nothing else in the pipeline (node ID translation, filters) has already decoded. Is that case real enough to justify widening sendRequest's return type and introducing an asymmetry between the request and response decode paths?

[hrishabhg]

One more thought on the decode depth side, specifically lazy response decoding.

Given the work in kroxylicious#4257, shouldDecodeResponse in DelegatingDecodePredicate is already symmetric with shouldDecodeRequest — and the router already drives response decoding declaratively for node ID translation. Metadata, FindCoordinator, DescribeTopicPartitions, Fetch (preferredReadReplica, currentLeader), and the other responses that carry node references are decoded regardless of what the router asks for, because virtual node ID mapping requires it.

Fetch is the primary response by volume and size, and it's already decoded. So lazy ResponseFrame.body() would only save decode cost for a response that a router intercepts but doesn't need to read, AND that nothing else in the pipeline (node ID translation, filters) has already decoded. Is that case real enough to justify widening sendRequest's return type and introducing an asymmetry between the request and response decode paths?

@SamBarker Not much gain in the decode path due to nodeIdTranslation, agreed and removed from the proposal.

Two version notes:

• NodeEndpoints/currentLeader arrive only in Fetch v16+ (Kafka 3.7).
• PreferredReadReplica (v11+, Kafka 2.3) forces Fetch response decoding anyway — and Kroxylicious currently does not observe whether a deployment actually uses rack-aware follower fetching, so the decode can't be skipped where it's off.

[SamBarker]

What was your thinking behind changing the name from the question the runtime asks of filters? From where I'm sitting, this gate is asking the same question as shouldDecodeRequest on the filter API — "should the runtime decode this request rather than passing it through as raw bytes?" The proposal already shows it being OR'd into the DecodePredicate, which reinforces that it's the same question asked of a different participant.

[SamBarker]

I think the proposal is good, but we need a second maintainer to have a review before we can merge.

[robobario]

Thanks for the work @hrishabhg , I'm generally supportive of this.

I wonder if we could make this more open to extension than a boolean decision and am wondering if the language in the proposal should be changed to talk less about "broker" and more about "virtual node" because future topologies may not have the "1 virtualNodeId: 1 upstreamNode" relationship.

[robobario]

we should include short apiVersion in the signature, allowing for Routers that only need to intercept specific version ranges

[robobario]

One idea we had in 070 was to support a richer set of actions here than intercept or forward-to-implicit virtualnode, for example you had your own ideas about forwarding on without implying a decode was required. Tom also speculated about broadcast/multicast possibilities, forwarding the same bytes to different upstreams. I wonder if we should leave the API open for extension towards that. Something like:

sealed interface RouteAction {
  ...
}

default RouteAction route(ApiKeys apiKey, short apiVersion, RouterContext ctx) {
    if (!ctx.virtualNode().isEmpty()) {
        return RouteAction.FORWARD_TO_IMPLICIT_NODE;
    } else {
        return RouteAction.INTERCEPT;
    }
}

In one way I like that this brings the implicit node target language into the API. Currently I don't think it's clear in the Router API javadoc where messages "forwarded to a route" are sent. One hesitation though is that future topologies might not have an implied node target, so adding the language might not extend well to those situations.

In future we could come up with further RouteActions like forwardToNode(VirtualNode node) implying we don't need to decode the message if we don't need to modify it or use it's contents for routing. Or multiCastToNodes and so on.

What do you think of this vs the boolean?

[robobario]

Suggested change

	instances are per-connection. A route name cannot express "forward to the broker _this connection_
	represents" — that target (`virtualNode()`) is only known per connection.
	instances are per-connection. A route name cannot express "forward to the implied target virtual node" — that target (`virtualNode()`) is derived initially from the connection.

I think we have to be careful with the language since we can have a DAG of Routers which will see different virtual node id mappings, so Router A may know that virtualNodeId 2 goes to route x and then Router B may see that the request is destined for virtualNodeId 0. A Router knows contextually which virtualNode() is associated with the request.

And also we don't want to suggest that there will always be a 1:1 mapping between a virtual node id and some physical upstream broker, leaving room for future topologies.

[robobario]

generally for this and the following section I wonder if we should talk about assigned virtual node vs broker.

From each Router's point of view it is aware of VirtualNodes, the connection implies a virtual node, then it's a runtime detail that we can happen to map it 1:1 to a broker. So I wonder if the proposal should use language throughout, rather than baking in this assumption. It's pedantic I know but it might help keep the concepts clear for future readers.

[robobario]

not quite sure what this section is here for, is it to justify solidifying inclusion of bootstrap and broker-bound endpoints as a Proxy neccessity? Something that should be visible in our APIs, rather than just a convenience. That seems like a good thing to have noted, I don't get the relationship to the plane types.

[tombentley]

@hrishabhg thanks for this!

Do you have a POC of this, or is it purely design currently? If you had a POC I'd like to take it for a spin and try it out.

[tombentley]

We have shouldHandle*Request() and shouldHandle*Response() in the filter API, so I think a better name would be shouldIntercept or shouldHandle.

[tombentley]

I find the idea appealing, but I'm not sure how feasible it would be given the RequestData and ResponseData APIs we have. I guess if the root object had a reference to the ByteBuf we could, in theory, deserialize progressively only when a called tried to access a struct which hadn't been decoded yet.