hachikuji commented on a change in pull request #9600:
URL: https://github.com/apache/kafka/pull/9600#discussion_r553701160
##########
File path: core/src/main/scala/kafka/server/KafkaApis.scala
##########
@@ -1790,17 +1790,41 @@ class KafkaApis(val requestChannel: RequestChannel,
else {
val supportedFeatures = brokerFeatures.supportedFeatures
val finalizedFeaturesOpt = finalizedFeatureCache.get
- finalizedFeaturesOpt match {
- case Some(finalizedFeatures) => ApiVersion.apiVersionsResponse(
- requestThrottleMs,
- config.interBrokerProtocolVersion.recordVersion.value,
- supportedFeatures,
- finalizedFeatures.features,
- finalizedFeatures.epoch)
- case None => ApiVersion.apiVersionsResponse(
- requestThrottleMs,
- config.interBrokerProtocolVersion.recordVersion.value,
- supportedFeatures)
+ val controllerApiVersions = if (isForwardingEnabled(request)) {
+ forwardingManager.controllerApiVersions()
+ } else
+ None
+
+ if (isForwardingEnabled(request) && controllerApiVersions.isEmpty) {
Review comment:
This seems a little severe. Not all apis need to be coordinated with the
controller. We have to handle the case when we receive a version that the
current controller cannot handle anyway, so I think it's ok to make this a
best-effort intersection and return just the broker APIs if the controller APIs
are not yet known.
The tricky case for this PR is when the controller changed or we learned
about new API version support after a client had already connected to the
broker and sent `ApiVersions`. In this case, we have to detect version
incompatibility dynamically when we try to forward the request. I might be
missing something, but the current patch doesn't seem to handle this. Maybe the
simplest option is to let the controller return UNSUPPORTED_VERSION in the
envelope response if the header indicates an api or version that it does not
support. Then the question is whether this error should be sent back to the
client, but that would be a little surprising.
Consider this sequence:
1. Client connects and sends ApiVersions request
2. Current controller supports AlterConfig v0-4, so that is what the broker
indicates in the ApiVersions response
3. New controller is elected and only supports AlterConfig v0-3
4. Client sends AlterConfig v4
Now what happens? An unsupported version error here would be treated as
fatal by the client. Instead, I think we agreed that instead of sending back
the error, the broker would just disconnect. This would force a reconnect and a
refresh of the API version support.
----------------------------------------------------------------
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
