tombentley commented on a change in pull request #10743:
URL: https://github.com/apache/kafka/pull/10743#discussion_r649872433
##########
File path:
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractCoordinator.java
##########
@@ -858,6 +885,12 @@ public void onSuccess(ClientResponse resp,
RequestFuture<Void> future) {
public void onFailure(RuntimeException e, RequestFuture<Void> future) {
log.debug("FindCoordinator request failed due to {}",
e.toString());
+ if (e instanceof UnsupportedBatchLookupException) {
Review comment:
This `AdminApiDriver` abstraction is pretty new to me, so I might be
wide of the mark, but it doesn't appear to handle this very well. The lookup
strategy has to build a request without knowing either the broker or the API
versions. It would be possible to pass the `ApiVersions` to the
`CoordinatorStrategy`, which should let you do the right thing based on the
minimum of the FindCoordinator API version supported in the whole cluster.
(That's not completely perfect, since really you'd want to decide on a
per-broker basis, but I think it would be good enough). Sadly it's not quite
enough to pass just `ApiVersions`, since it doesn't really know about the nodes
in the cluster, so you'd need to pass `Metadata` too, which is quite a lot of
work. So I can understand why it makes sense to do it like this, since it lets
you benefit from the existing logic for figuring out request versions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
