feat: Adds Scalable Push Query Routing #7587
Conversation
There was a problem hiding this comment.
Thanks for this, @AlanConfluent ! Within my limitations, this change looks good. I was puzzled by one thing, though (inline).
| @@ -66,6 +68,7 @@ | |||
| private static final String STATUS_PATH = "/status"; | |||
| private static final String KSQL_PATH = "/ksql"; | |||
| private static final String QUERY_PATH = "/query"; | |||
| private static final String QUERY_STREAM_PATH = "/query-stream"; | |||
There was a problem hiding this comment.
What's up with this? We have had to handle transient queries for a long time, so I'm not seeing how this could have been missing.
There was a problem hiding this comment.
/query is the http 1.1 path, handled by StreamedQueryResource. This is the endpoint we want to eventually deprecate. It's what's currently handling queries for the other methods of KsqlTarget. I've added a new method which uses the new endpoint /query-stream which has the benefit of not using the threadpool and being fully async.
There was a problem hiding this comment.
Thanks, yeah, I was just surprised that we're not already using the async endpoint.
Description
The next step of scalable push queries, this adds the routing logic in
PushRouting.At the moment, it attempts to determine the set of hosts at start time to connect to and just sticks with them. A followup change will fail the query if there's a rebalance.
PushRoutingdoes everything async, meaning that it uses the Vertx Context and CompletableFutures to handle everything, so it doesn't require a thread pool.This also introduces a http2 client to
KsqlClientsince this is required to issue http2 requests, which this now does for the/query-streamrequests between cluster instances.Testing done
Ran unit tests.
Reviewer checklist