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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to