dajac commented on code in PR #17411:
URL: https://github.com/apache/kafka/pull/17411#discussion_r1796499895
##########
core/src/main/scala/kafka/server/KafkaApis.scala:
##########
@@ -3796,7 +3796,15 @@ class KafkaApis(val requestChannel: RequestChannel,
if (!isConsumerGroupProtocolEnabled()) {
// The API is not supported by the "old" group coordinator (the
default). If the
// new one is not enabled, we fail directly here.
- requestHelper.sendMaybeThrottle(request,
consumerGroupHeartbeatRequest.getErrorResponse(Errors.UNSUPPORTED_VERSION.exception))
+ requestHelper.sendMaybeThrottle(request, new
ConsumerGroupHeartbeatResponse(
+ new ConsumerGroupHeartbeatResponseData()
+ .setErrorCode(Errors.UNSUPPORTED_VERSION.code())
+ .setErrorMessage("An error occurred handling the consumer group
heartbeat request. " +
+ s"To enable support for the ${Group.GroupType.CONSUMER} group
protocol, update the broker " +
+ s"configuration for
\"${GroupCoordinatorConfig.NEW_GROUP_COORDINATOR_ENABLE_CONFIG}\" to \"true\"
and " +
+
s"\"${GroupCoordinatorConfig.GROUP_COORDINATOR_REBALANCE_PROTOCOLS_CONFIG}\" to
" +
+ s"\"${Group.GroupType.CLASSIC}, ${Group.GroupType.CONSUMER}\".
Alternatively, update the client " +
+ s"configuration for \"group.protocol\" to
\"${Group.GroupType.CLASSIC}\".")))
Review Comment:
@FrankYang0529 Thanks for working on this. I have a high level comment about
the approach. In my opinion, having the error message defined here is incorrect
because the consumer could also get an UNSUPPORTED_VERSION if the API is not
available at all. Hence, I think that we should catch the UNSUPPORTED_VERSION
in the membership manager and put the error message there. It will ensure that
we cover all the possible paths.
Moreover, I suggest to not talk about the server side configs at all in the
error message. Consumer's users usually don't have access to the servers so it
would help them. Instead, I would rather say something like this:
> The cluster doesn't yet support the new consumer group protocol. Set
group.protocol=classic to revert to the classic protocol until the cluster is
upgraded.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
